On Wed, 31 Dec 2025 at 13:14, jian he <[email protected]> wrote: > > On Tue, Dec 16, 2025 at 9:01 PM Amul Sul <[email protected]> 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
patch.diff
Description: Binary data
