Re: Error position support for ComputeIndexAttrs

On Wed, 31 Dec 2025 at 13:14, jian he <jian(dot)universality(at)gmail(dot)com> wrote:
>
> On Tue, Dec 16, 2025 at 9:01 PM Amul Sul <sulamul(at)gmail(dot)com> wrote:
> > >
> >
> > +1, patch looks quite straightforward and pretty much reasonable to me.
> >
> > Regards,
> > Amul
>
> hi.
>
> some failures because I didn't adjust collate.linux.utf8.out and
> collate.windows.win1252.out:
> https://cirrus-ci.com/build/6690791764000768
> https://api.cirrus-ci.com/v1/artifact/task/5658162642026496/testrun/build/testrun/regress/regress/regression.diffs
> https://api.cirrus-ci.com/v1/artifact/task/5605386083893248/log/src/test/regress/regression.diffs
>
> The attached patch only adjusts collate.linux.utf8.out and
> collate.windows.win1252.out, with no other changes.
>
>
> --
> jian
> https://www.enterprisedb.com

Hi!
Nice catch, I see this patch is indeed an improvement.

For v2-0002, I was wondering if there is any further improvements
possible. Namely, if we can pass parse state in cases where v-0002
passes NULL.

1) So, DefineIndex in bootstrap (src/backend/bootstrap/bootparse.y) -
obviously there is no need to support error position for bootstrap
2) DefineIndex in commands/indexcmds.c L1524 (inside DefineIndex
actually) - We do not need to pass parse state here, because if any
trouble, we will elog(ERROR) earlier.
3) DefineIndex in commands/tablecmds.c L 1305 - also executed in
child partition cases.
4) DefineIndex inside ATAddIndex. It is triggered for patterns like
"alter table t add constraint c unique (ii);" - current patch set
missing support for them. I tried to pass pstate here, but no success,
because exprLocation returns -1 in ComputeIndexAttrs. Please see my
attempt attached. I guess this can be completed to get location
support, but I do not have any time today.
5) DefineIndex inside AttachPartitionEnsureIndexes - looks like we
do not need pstate here.

--
Best regards,
Kirill Reshke