Re: Pass ParseState as down to utility functions.

On Wed, Dec 25, 2024 at 5:29 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Mon, Dec 16, 2024 at 05:25:45PM +0800, jian he wrote:
> > i've split into 3 patches, feel free to merge them in any way.
> > v12-0001:  add error position for ATPrepAlterColumnType.
>
> For this one, why don't you do the same for undefined columns and
> USING with generated columns at least?  This looks half-baked.
>

I think I understand what you mean.
please check attached for changes within ATPrepAlterColumnType


> > v12-0002:  add error position for these 3 functions:
> > transformColumnDefinition, transformAlterTableStmt, transformTableConstraint.
>
>  ERROR:  column "c" of relation "itest4" does not exist
> +LINE 1: ALTER TABLE itest4 ALTER COLUMN c ADD GENERATED ALWAYS AS ID...
> +                                              ^
>
> This one is kind of confusing?  The part that matters for the error is
> the column that does not exist, not the ADD GENERATED.
>
I agree this is confusing.

while looking at 0002:
errmsg("conflicting NO INHERIT declarations for not-null constraints
on column \"%s\"", column->colname)
add parser_errposition
will not be very helpful. i think we need an errhint.

for example:
create table notnull_tbl_fail (a int primary key constraint foo not
null no inherit);
ERROR:  conflicting NO INHERIT declarations for not-null constraints
on column "a"
the error message didn't explicitly say that the primary key imply a
not-null inherit constraint.

Maybe we can change to
errmsg("conflicting NO INHERIT declarations for not-null constraints
on column \"%s\"", column->colname),
errhint("specified primary key or identity sequence imply an inherited
not-null constraint will be created")

what do you think?