Re: Add column name to error description

On 2024-03-31 15:22 +0200, Marcos Pegoraro wrote:
> This is my first patch, so sorry if I miss something.

Please make sure that tests are passing by running make check:
https://www.postgresql.org/docs/current/regress-run.html#REGRESS-RUN-TEMP-INST

The patch breaks src/test/regress/sql/plpgsql.sql at:

    -- this does not work currently (no implicit casting)
    create or replace function compos() returns compostype as $$
    begin
      return (1, 'hello');
    end;
    $$ language plpgsql;
    select compos();
    server closed the connection unexpectedly
        This probably means the server terminated abnormally
        before or while processing the request.
    connection to server was lost

> If I have a function which returns lots of columns and any of these columns
> returns a wrong type it's not easy to see which one is that column because
> it points me only to its position, not its name. So, this patch only adds
> that column name, just that.

+1 for this improvement.

> create function my_f(a integer, b integer) returns table(first_col integer,
> lots_of_cols_later numeric) language plpgsql as $function$
> begin
>   return query select a, b;
> end;$function$;
> 
> select * from my_f(1,1);
> --ERROR: structure of query does not match function result type
> --Returned type integer does not match expected type numeric in column 2.
> 
> For a function which has just 2 columns is easy but if it returns a hundred
> of columns, which one is that 66th column ?
> 
> My patch just adds column name to that description message.
> --ERROR: structure of query does not match function result type
> --Returned type integer does not match expected type numeric in column 2-
> lots_of_cols_later.

> diff --git a/src/backend/access/common/attmap.c b/src/backend/access/common/attmap.c
> index b0fe27ef57..85f7c0cb8c 100644
> --- a/src/backend/access/common/attmap.c
> +++ b/src/backend/access/common/attmap.c
> @@ -118,12 +118,13 @@ build_attrmap_by_position(TupleDesc indesc,
>                  ereport(ERROR,
>                          (errcode(ERRCODE_DATATYPE_MISMATCH),
>                           errmsg_internal("%s", _(msg)),
> -                         errdetail("Returned type %s does not match expected type %s in column %d.",
> +                         errdetail("Returned type %s does not match expected type %s in column %d-%s.",

The format "%d-%s" is not ideal.  I suggesst "%d (%s)".

>                                     format_type_with_typemod(att->atttypid,
>                                                              att->atttypmod),
>                                     format_type_with_typemod(atttypid,
>                                                              atttypmod),
> -                                   noutcols)));
> +                                   noutcols,
> +                                   att->attname)));

Must be NameStr(att->attname) for use with printf's %s.  GCC even prints
a warning:

    attmap.c:121:60: warning: format ‘%s’ expects argument of type ‘char *’, but argument 5 has type ‘NameData’ {aka
‘structnameData’} [-Wformat=]
 

-- 
Erik