[PATCH v1] Fix propagation of indimmediate flag in index_create_copy

On 2026-Jul-20, Nitin Motiani wrote:

It was reported in [1] that Reindex Concurrently for indexes with
deferrable constraints doesn't honour the deferrable constraints while
the operations is running. And the transactions can fail because the
constraint is enforced before commit.

I'm proposing a patch here which correctly copies the flag from the
old index to the new one. Please take a look.

Hah, that was quick, thanks! This fails the isolation tests for me
though, with this backtrace:

#4 0x00005618d76672d6 in ExceptionalCondition (
conditionName=conditionName@entry=0x5618d77741a8 "(constr_flags == 0) || ((flags & INDEX_CREATE_ADD_CONSTRAINT) != 0)",
fileName=fileName@entry=0x5618d7773e58 "../../pgsql/source/master/src/backend/catalog/index.c", lineNumber=lineNumber@entry=773)
at ../../pgsql/source/master/src/backend/utils/error/assert.c:65
#5 0x00005618d7271fd5 in index_create (heapRelation=heapRelation@entry=0x7f13e9ce85d0,
indexRelationName=indexRelationName@entry=0x5618fe816e58 "uq_val_ccnew", indexRelationId=indexRelationId@entry=0,
parentIndexRelid=parentIndexRelid@entry=0, parentConstraintId=parentConstraintId@entry=0, relFileNumber=relFileNumber@entry=0,
indexInfo=0x5618fe816aa8, indexColNames=0x5618fe6c8d40, accessMethodId=403, tableSpaceId=0, collationIds=0x5618fe73e2f8,
opclassIds=0x7f13e9cead80, opclassOptions=0x5618fe6c8ea0, coloptions=0x7f13e9cead9c, stattargets=0x5618fe80d748, reloptions=0, flags=140,
constr_flags=2, allow_system_table_mods=true, is_internal=false, constraintId=0x0) at ../../pgsql/source/master/src/backend/catalog/index.c:773
#6 0x00005618d727248d in index_create_copy (heapRelation=heapRelation@entry=0x7f13e9ce85d0, flags=flags@entry=140, oldIndexId=33725,
tablespaceOid=0, newName=0x5618fe816e58 "uq_val_ccnew") at ../../pgsql/source/master/src/backend/catalog/index.c:1468
#7 0x00005618d72cbd46 in ReindexRelationConcurrently (stmt=stmt@entry=0x5618fe712228, relationOid=relationOid@entry=33722,
params=params@entry=0x7ffcf8ab62f8) at ../../pgsql/source/master/src/backend/commands/indexcmds.c:4117
#8 0x00005618d72cd892 in ReindexTable (stmt=0x5618fe712228, params=0x7ffcf8ab62f8, isTopLevel=true)
at ../../pgsql/source/master/src/backend/commands/indexcmds.c:3227
#9 ExecReindex (pstate=pstate@entry=0x5618fe6c8c30, stmt=stmt@entry=0x5618fe712228, isTopLevel=isTopLevel@entry=true)
at ../../pgsql/source/master/src/backend/commands/indexcmds.c:3042

This is during the
REINDEX TABLE CONCURRENTLY reind_deferred;
execution.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/
"The important things in the world are problems with society that we don't
understand at all. The machines will become more complicated but they won't
be more complicated than the societies that run them." (Freeman Dyson)

On Mon, Jul 20, 2026 at 11:47 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:

Hah, that was quick, thanks! This fails the isolation tests for me
though, with this backtrace:

I drafted the change a couple of weeks ago but had not sent it out as
I was trying to see if I missed something. When I saw your response on
the thread, I sent it out as is.

#4 0x00005618d76672d6 in ExceptionalCondition (
conditionName=conditionName@entry=0x5618d77741a8 "(constr_flags == 0) || ((flags & INDEX_CREATE_ADD_CONSTRAINT) != 0)",
fileName=fileName@entry=0x5618d7773e58 "../../pgsql/source/master/src/backend/catalog/index.c", lineNumber=lineNumber@entry=773)
at ../../pgsql/source/master/src/backend/utils/error/assert.c:65
#5 0x00005618d7271fd5 in index_create (heapRelation=heapRelation@entry=0x7f13e9ce85d0,
indexRelationName=indexRelationName@entry=0x5618fe816e58 "uq_val_ccnew", indexRelationId=indexRelationId@entry=0,
parentIndexRelid=parentIndexRelid@entry=0, parentConstraintId=parentConstraintId@entry=0, relFileNumber=relFileNumber@entry=0,
indexInfo=0x5618fe816aa8, indexColNames=0x5618fe6c8d40, accessMethodId=403, tableSpaceId=0, collationIds=0x5618fe73e2f8,
opclassIds=0x7f13e9cead80, opclassOptions=0x5618fe6c8ea0, coloptions=0x7f13e9cead9c, stattargets=0x5618fe80d748, reloptions=0, flags=140,
constr_flags=2, allow_system_table_mods=true, is_internal=false, constraintId=0x0) at ../../pgsql/source/master/src/backend/catalog/index.c:773
#6 0x00005618d727248d in index_create_copy (heapRelation=heapRelation@entry=0x7f13e9ce85d0, flags=flags@entry=140, oldIndexId=33725,
tablespaceOid=0, newName=0x5618fe816e58 "uq_val_ccnew") at ../../pgsql/source/master/src/backend/catalog/index.c:1468
#7 0x00005618d72cbd46 in ReindexRelationConcurrently (stmt=stmt@entry=0x5618fe712228, relationOid=relationOid@entry=33722,
params=params@entry=0x7ffcf8ab62f8) at ../../pgsql/source/master/src/backend/commands/indexcmds.c:4117
#8 0x00005618d72cd892 in ReindexTable (stmt=0x5618fe712228, params=0x7ffcf8ab62f8, isTopLevel=true)
at ../../pgsql/source/master/src/backend/commands/indexcmds.c:3227
#9 ExecReindex (pstate=pstate@entry=0x5618fe6c8c30, stmt=stmt@entry=0x5618fe712228, isTopLevel=isTopLevel@entry=true)
at ../../pgsql/source/master/src/backend/commands/indexcmds.c:3042

This is during the
REINDEX TABLE CONCURRENTLY reind_deferred;
execution.

Thanks. I will look into this.

Regards,
Nitin Motiani
Google

#4 0x00005618d76672d6 in ExceptionalCondition (
conditionName=conditionName@entry=0x5618d77741a8 "(constr_flags == 0) || ((flags & INDEX_CREATE_ADD_CONSTRAINT) != 0)",
fileName=fileName@entry=0x5618d7773e58 "../../pgsql/source/master/src/backend/catalog/index.c", lineNumber=lineNumber@entry=773)
at ../../pgsql/source/master/src/backend/utils/error/assert.c:65
#5 0x00005618d7271fd5 in index_create (heapRelation=heapRelation@entry=0x7f13e9ce85d0,
indexRelationName=indexRelationName@entry=0x5618fe816e58 "uq_val_ccnew", indexRelationId=indexRelationId@entry=0,
parentIndexRelid=parentIndexRelid@entry=0, parentConstraintId=parentConstraintId@entry=0, relFileNumber=relFileNumber@entry=0,
indexInfo=0x5618fe816aa8, indexColNames=0x5618fe6c8d40, accessMethodId=403, tableSpaceId=0, collationIds=0x5618fe73e2f8,
opclassIds=0x7f13e9cead80, opclassOptions=0x5618fe6c8ea0, coloptions=0x7f13e9cead9c, stattargets=0x5618fe80d748, reloptions=0, flags=140,
constr_flags=2, allow_system_table_mods=true, is_internal=false, constraintId=0x0) at ../../pgsql/source/master/src/backend/catalog/index.c:773
#6 0x00005618d727248d in index_create_copy (heapRelation=heapRelation@entry=0x7f13e9ce85d0, flags=flags@entry=140, oldIndexId=33725,
tablespaceOid=0, newName=0x5618fe816e58 "uq_val_ccnew") at ../../pgsql/source/master/src/backend/catalog/index.c:1468
#7 0x00005618d72cbd46 in ReindexRelationConcurrently (stmt=stmt@entry=0x5618fe712228, relationOid=relationOid@entry=33722,
params=params@entry=0x7ffcf8ab62f8) at ../../pgsql/source/master/src/backend/commands/indexcmds.c:4117
#8 0x00005618d72cd892 in ReindexTable (stmt=0x5618fe712228, params=0x7ffcf8ab62f8, isTopLevel=true)
at ../../pgsql/source/master/src/backend/commands/indexcmds.c:3227
#9 ExecReindex (pstate=pstate@entry=0x5618fe6c8c30, stmt=stmt@entry=0x5618fe712228, isTopLevel=isTopLevel@entry=true)
at ../../pgsql/source/master/src/backend/commands/indexcmds.c:3042

This is during the
REINDEX TABLE CONCURRENTLY reind_deferred;
execution.

I missed this as I had not tried with asserts enabled. I considered
the following approaches to fix this :

1. By changing the check to allow constr_flag &
INDEX_CONSTR_CREATE_DEFERRABLE to be non-zero even if
INDEX_CREATE_ADD_CONSTRAINT was not set. This seemed unsafe.
2. Adding a new flag to the function to specially allow
INDEX_CONSTR_CREATE_DEFERRABLE for the case when the index was being
copied.
3. Adding a new flag but using that to determine whether to set
indimmediate to true or not. This way constr_flags still follows the
contract that it will only be used when a constraint is being created.

I ended up picking the 3rd approach and created a new index creation
flag INDEX_CREATE_IS_DEFERRABLE_COPY. Now indimmediate is set if
either INDEX_CONSTR_CREATE_DEFERRABLE is set in constr_flags or
INDEX_CREATE_IS_DEFERRABLE_COPY is set in flags. This looked a little
extra code but I think it's cleaner as it allows constr_flags to be
only used when a new constraint is created. And a reader can see that
there are two possible reasons for setting indimmediate to false.

I'm attaching the v2 patch with this change. Please let me know what you think.

Thanks

Nitin Motiani
Google

On Fri, Jul 24, 2026 at 05:20:03PM +0530, Nitin Motiani wrote:

I ended up picking the 3rd approach and created a new index creation
flag INDEX_CREATE_IS_DEFERRABLE_COPY. Now indimmediate is set if
either INDEX_CONSTR_CREATE_DEFERRABLE is set in constr_flags or
INDEX_CREATE_IS_DEFERRABLE_COPY is set in flags. This looked a little
extra code but I think it's cleaner as it allows constr_flags to be
only used when a new constraint is created. And a reader can see that
there are two possible reasons for setting indimmediate to false.

I'm attaching the v2 patch with this change. Please let me know what you think.

As this is in my realm of things.. I have caught up with what you
have here.

And here I thought that we somewhat missed to set indimmediate on the
new index definition in index_concurrently_swap(), but your issue is
before even that. It is a sensible thing to do to ensure that the
initial phase of the copied index satisfies the deferred constraint of
the underlying table, limiting the reliability of the operation, as we
want the index to be reused for the constraint.

The addition of the new INDEX_ option would be at least backpatchable.
I was wondering why you needed the extra flag to begin with, but I
don't immediately see how we could enforce indimmediate properly in
the definition copy without it. Having three flags related to
deferred constraints feels a bit annoying long-term, but that really
comes down to the grammar representation, what we allow in the table
definition based on the standard, and how much space we give the
backend internals to deal with the CREATE TABLE grammar for
constraints in an equivalent way.

Side note: your patch has some useless whitespaces in index.h. I'll
study all that a bit more next week, booking a few hours.
--
Michael

On Sun, Jul 26, 2026 at 06:12:27PM +0900, Michael Paquier wrote:

The addition of the new INDEX_ option would be at least backpatchable.
I was wondering why you needed the extra flag to begin with, but I
don't immediately see how we could enforce indimmediate properly in
the definition copy without it. Having three flags related to
deferred constraints feels a bit annoying long-term, but that really
comes down to the grammar representation, what we allow in the table
definition based on the standard, and how much space we give the
backend internals to deal with the CREATE TABLE grammar for
constraints in an equivalent way.

Side note: your patch has some useless whitespaces in index.h. I'll
study all that a bit more next week, booking a few hours.

I have looked at the patch, and here are some comments/fixes:
- IS_DEFERRABLE_COPY feels confusing, and it is inconsistent with the
other option names. I have settled to a simpler CREATE_DEFERRABLE,
meaning indimmediate=false for the copy when the option is set.
- Let's avoid a hardcoded sleep in the isolation test. With 10s, I
suspect that this is going to be unstable in the buildfarm. 0.1s
should not be noticeable on a fast machine, but let's just make that
faster and reliable with an injection point. We just need one after
the phase 2, waiting while we perform some writes that break the
constraint (result would be to not break the constraint, of course,
due to the indimmediate).
- We can do the same test in less sessions and less steps. I was able
to get down to 2 sessions. Without the fix, we report immediately a
diff with the constraint breaking.

Attached is my result, that I intend to backpatch. Test is for v17~.
No time to edit the commit message yet, sorry, I get that it is
confusing with the new flag name. :p

Thanks,
--
Michael

On Mon, Jul 27, 2026 at 04:47:00PM +0900, Michael Paquier wrote:

Attached is my result, that I intend to backpatch. Test is for v17~.
No time to edit the commit message yet, sorry, I get that it is
confusing with the new flag name. :p

And done as 74276e685dd0.

You may note that I have used the word "tentatively" regarding the
backport of the test. In v17~v19, injection point waits depend on
condition variables, that depend on DSMs, that have a dependency with
cleanup actions not happening correctly outside of transaction
contexts as far as I know. We're dealing with a wait/wakeup and not
an ERROR, which should be fine at the end, just mentioning in
passing.. Injpoint waits depend on atomics on HEAD, which is more
portable, hence not an issue if one has the idea to do waits outside
of a transaction.
--
Michael

You may note that I have used the word "tentatively" regarding the
backport of the test. In v17~v19, injection point waits depend on
condition variables, that depend on DSMs, that have a dependency with
cleanup actions not happening correctly outside of transaction
contexts as far as I know. We're dealing with a wait/wakeup and not
an ERROR, which should be fine at the end, just mentioning in
passing.. Injpoint waits depend on atomics on HEAD, which is more
portable, hence not an issue if one has the idea to do waits outside
of a transaction.
--

Thanks a lot, Michael, for taking care of this and for the explanation.

The original report in [1]/messages/by-id/cc0470801d4ee46bd85f94c2516fd31b@nimrod.no also had this closely related problem.

Note how it is currently not possible to safely add a UNIQUE
DEFERRED constraint following the example in
https://www.postgresql.org/docs/18/sql-altertable.html

CREATE UNIQUE INDEX CONCURRENTLY dist_id_temp_idx ON distributors (dist_id);
ALTER TABLE distributors DROP CONSTRAINT distributors_pkey,
ADD CONSTRAINT distributors_pkey PRIMARY KEY USING INDEX
dist_id_temp_idx;

For this to work safely with UNIQUE DEFERRED constraints, I assume it would
be necessary to add an option to CREATE INDEX to make an index DEFERRED.

So is that something worth looking into? I'm not sure if a deferred
index makes sense in general but is there something that can be done
for this specific scenario?

[1]: /messages/by-id/cc0470801d4ee46bd85f94c2516fd31b@nimrod.no

Regards,
Nitin Motiani
Google