Horton

Thread

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add parse location to IndexElem.

  2. Include error location in errors from ComputeIndexAttrs().

  3. Error position support for partition specifications

  1. Error position support for ComputeIndexAttrs

    jian he <jian.universality@gmail.com> — 2025-12-16T07:21:04Z

    hi.

Following the addition of error position support to ComputePartitionAttrs in
[0], we can extend this feature to ComputeIndexAttrs.

Both partition keys and indexes support expressions and share a 32-column
limit, CREATE INDEX can be as complicated as PARTITION BY expression, and given
that ComputeIndexAttrs already contains 14 calls to ereport(ERROR, ...).
Adding error position support for ComputeIndexAttrs seems to make sense.

To achieve this, ComputeIndexAttrs must receive a ParseState. Since
ComputeIndexAttrs is nested under DefineIndex , DefineIndex must also have a
ParseState.

v1-0001: almost the same as [1], the only difference is after
makeNode(IndexElem),
we should set the location to -1.
v1-0002: Error position support for ComputeIndexAttrs


[0]: https://git.postgresql.org/cgit/postgresql.git/commit/?id=1e5e4efd02b614908cae62d9452528462d307224
[1]: https://postgr.es/m/202512121327.f2zimsr6guso@alvherre.pgsql



--
jian
https://www.enterprisedb.com/

  2. Re: Error position support for ComputeIndexAttrs

    amul sul <sulamul@gmail.com> — 2025-12-16T13:00:23Z

    On Tue, Dec 16, 2025 at 12:51 PM jian he <jian.universality@gmail.com> wrote:
>
> hi.
>
> Following the addition of error position support to ComputePartitionAttrs in
> [0], we can extend this feature to ComputeIndexAttrs.
>
> Both partition keys and indexes support expressions and share a 32-column
> limit, CREATE INDEX can be as complicated as PARTITION BY expression, and given
> that ComputeIndexAttrs already contains 14 calls to ereport(ERROR, ...).
> Adding error position support for ComputeIndexAttrs seems to make sense.
>
> To achieve this, ComputeIndexAttrs must receive a ParseState. Since
> ComputeIndexAttrs is nested under DefineIndex , DefineIndex must also have a
> ParseState.
>
> v1-0001: almost the same as [1], the only difference is after
> makeNode(IndexElem),
> we should set the location to -1.
> v1-0002: Error position support for ComputeIndexAttrs
>

+1, patch looks quite straightforward and pretty much reasonable to me.

Regards,
Amul

  3. Re: Error position support for ComputeIndexAttrs

    jian he <jian.universality@gmail.com> — 2025-12-31T08:13:47Z

    On Tue, Dec 16, 2025 at 9:01 PM Amul Sul <sulamul@gmail.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

  4. Re: Error position support for ComputeIndexAttrs

    Kirill Reshke <reshkekirill@gmail.com> — 2025-12-31T14:37:13Z

    On Wed, 31 Dec 2025 at 13:14, jian he <jian.universality@gmail.com> wrote:
>
> On Tue, Dec 16, 2025 at 9:01 PM Amul Sul <sulamul@gmail.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