Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 77e7218

Browse files
michail-nikolaevCommitfest Bot
authored and
Commitfest Bot
committed
Modify the infer_arbiter_indexes function to also look for indexes that match the specified named constraint to be used as arbiters. This ensures that the same set of arbiter indexes is used for all concurrent transactions in cases where REINDEX CONCURRENTLY processes an index used as a named constraint.
The patch resolves the issues in the following specs: * reindex_concurrently_upsert_on_constraint Despite the patch, the following specs are still affected: * reindex_concurrently_upsert_partitioned
1 parent 11035e7 commit 77e7218

File tree

1 file changed

+88
-33
lines changed

1 file changed

+88
-33
lines changed

src/backend/optimizer/util/plancat.c

Lines changed: 88 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -716,9 +716,10 @@ infer_arbiter_indexes(PlannerInfo *root)
716716
List *indexList;
717717
ListCell *l;
718718

719-
/* Normalized inference attributes and inference expressions: */
720-
Bitmapset *inferAttrs = NULL;
721-
List *inferElems = NIL;
719+
/* Normalized required attributes and expressions: */
720+
Bitmapset *requiredArbiterAttrs = NULL;
721+
List *requiredArbiterElems = NIL;
722+
List *requiredIndexPredExprs = (List *) onconflict->arbiterWhere;
722723

723724
/* Results */
724725
List *results = NIL;
@@ -757,8 +758,8 @@ infer_arbiter_indexes(PlannerInfo *root)
757758

758759
if (!IsA(elem->expr, Var))
759760
{
760-
/* If not a plain Var, just shove it in inferElems for now */
761-
inferElems = lappend(inferElems, elem->expr);
761+
/* If not a plain Var, just shove it in requiredArbiterElems for now */
762+
requiredArbiterElems = lappend(requiredArbiterElems, elem->expr);
762763
continue;
763764
}
764765

@@ -770,30 +771,76 @@ infer_arbiter_indexes(PlannerInfo *root)
770771
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
771772
errmsg("whole row unique index inference specifications are not supported")));
772773

773-
inferAttrs = bms_add_member(inferAttrs,
774+
requiredArbiterAttrs = bms_add_member(requiredArbiterAttrs,
774775
attno - FirstLowInvalidHeapAttributeNumber);
775776
}
776777

778+
indexList = RelationGetIndexList(relation);
779+
777780
/*
778781
* Lookup named constraint's index. This is not immediately returned
779-
* because some additional sanity checks are required.
782+
* because some additional sanity checks are required. Additionally, we
783+
* need to process other indexes as potential arbiters to account for
784+
* cases where REINDEX CONCURRENTLY is processing an index used as a
785+
* named constraint.
780786
*/
781787
if (onconflict->constraint != InvalidOid)
782788
{
783789
indexOidFromConstraint = get_constraint_index(onconflict->constraint);
784790

785791
if (indexOidFromConstraint == InvalidOid)
792+
{
786793
ereport(ERROR,
787794
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
788-
errmsg("constraint in ON CONFLICT clause has no associated index")));
795+
errmsg("constraint in ON CONFLICT clause has no associated index")));
796+
}
797+
798+
/*
799+
* Find the named constraint index to extract its attributes and predicates.
800+
* We open all indexes in the loop to avoid deadlock of changed order of locks.
801+
* */
802+
foreach(l, indexList)
803+
{
804+
Oid indexoid = lfirst_oid(l);
805+
Relation idxRel;
806+
Form_pg_index idxForm;
807+
AttrNumber natt;
808+
809+
idxRel = index_open(indexoid, rte->rellockmode);
810+
idxForm = idxRel->rd_index;
811+
812+
if (idxForm->indisready)
813+
{
814+
if (indexOidFromConstraint == idxForm->indexrelid)
815+
{
816+
/*
817+
* Prepare requirements for other indexes to be used as arbiter together
818+
* with indexOidFromConstraint. It is required to involve both equals indexes
819+
* in case of REINDEX CONCURRENTLY.
820+
*/
821+
for (natt = 0; natt < idxForm->indnkeyatts; natt++)
822+
{
823+
int attno = idxRel->rd_index->indkey.values[natt];
824+
825+
if (attno != 0)
826+
requiredArbiterAttrs = bms_add_member(requiredArbiterAttrs,
827+
attno - FirstLowInvalidHeapAttributeNumber);
828+
}
829+
requiredArbiterElems = RelationGetIndexExpressions(idxRel);
830+
requiredIndexPredExprs = RelationGetIndexPredicate(idxRel);
831+
/* We are done, so, quite the loop. */
832+
index_close(idxRel, NoLock);
833+
break;
834+
}
835+
}
836+
index_close(idxRel, NoLock);
837+
}
789838
}
790839

791840
/*
792841
* Using that representation, iterate through the list of indexes on the
793842
* target relation to try and find a match
794843
*/
795-
indexList = RelationGetIndexList(relation);
796-
797844
foreach(l, indexList)
798845
{
799846
Oid indexoid = lfirst_oid(l);
@@ -842,26 +889,23 @@ infer_arbiter_indexes(PlannerInfo *root)
842889
ereport(ERROR,
843890
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
844891
errmsg("ON CONFLICT DO UPDATE not supported with exclusion constraints")));
845-
846-
results = lappend_oid(results, idxForm->indexrelid);
847-
foundValid |= idxForm->indisvalid;
848-
index_close(idxRel, NoLock);
849-
break;
892+
goto found;
850893
}
851894
else if (indexOidFromConstraint != InvalidOid)
852895
{
853-
/* No point in further work for index in named constraint case */
854-
goto next;
896+
/* In the case of "ON constraint_name DO UPDATE" we need to skip non-unique candidates. */
897+
if (!idxForm->indisunique && onconflict->action == ONCONFLICT_UPDATE)
898+
goto next;
899+
} else {
900+
/*
901+
* Only considering conventional inference at this point (not named
902+
* constraints), so index under consideration can be immediately
903+
* skipped if it's not unique
904+
*/
905+
if (!idxForm->indisunique)
906+
goto next;
855907
}
856908

857-
/*
858-
* Only considering conventional inference at this point (not named
859-
* constraints), so index under consideration can be immediately
860-
* skipped if it's not unique
861-
*/
862-
if (!idxForm->indisunique)
863-
goto next;
864-
865909
/*
866910
* So-called unique constraints with WITHOUT OVERLAPS are really
867911
* exclusion constraints, so skip those too.
@@ -881,14 +925,18 @@ infer_arbiter_indexes(PlannerInfo *root)
881925
}
882926

883927
/* Non-expression attributes (if any) must match */
884-
if (!bms_equal(indexedAttrs, inferAttrs))
928+
if (!bms_equal(indexedAttrs, requiredArbiterAttrs))
885929
goto next;
886930

887931
/* Expression attributes (if any) must match */
888932
idxExprs = RelationGetIndexExpressions(idxRel);
889933
if (idxExprs && varno != 1)
890934
ChangeVarNodes((Node *) idxExprs, 1, varno, 0);
891935

936+
/*
937+
* If arbiterElems are present, check them. If name >constraint is
938+
* present arbiterElems == NIL.
939+
*/
892940
foreach(el, onconflict->arbiterElems)
893941
{
894942
InferenceElem *elem = (InferenceElem *) lfirst(el);
@@ -926,26 +974,33 @@ infer_arbiter_indexes(PlannerInfo *root)
926974
}
927975

928976
/*
929-
* Now that all inference elements were matched, ensure that the
977+
* In case of the conventional inference involved ensure that the
930978
* expression elements from inference clause are not missing any
931979
* cataloged expressions. This does the right thing when unique
932980
* indexes redundantly repeat the same attribute, or if attributes
933981
* redundantly appear multiple times within an inference clause.
982+
*
983+
* In the case of named constraint ensure candidate has equal set
984+
* of expressions as the named constraint index.
934985
*/
935-
if (list_difference(idxExprs, inferElems) != NIL)
986+
if (list_difference(idxExprs, requiredArbiterElems) != NIL)
936987
goto next;
937988

938-
/*
939-
* If it's a partial index, its predicate must be implied by the ON
940-
* CONFLICT's WHERE clause.
941-
*/
942989
predExprs = RelationGetIndexPredicate(idxRel);
943990
if (predExprs && varno != 1)
944991
ChangeVarNodes((Node *) predExprs, 1, varno, 0);
945992

946-
if (!predicate_implied_by(predExprs, (List *) onconflict->arbiterWhere, false))
993+
/*
994+
* If it's a partial index and conventional inference, its predicate must be implied
995+
* by the ON CONFLICT's WHERE clause.
996+
*/
997+
if (indexOidFromConstraint == InvalidOid && !predicate_implied_by(predExprs, requiredIndexPredExprs, false))
998+
goto next;
999+
/* If it's a partial index and named constraint predicates must be equal. */
1000+
if (indexOidFromConstraint != InvalidOid && list_difference(predExprs, requiredIndexPredExprs) != NIL)
9471001
goto next;
9481002

1003+
found:
9491004
results = lappend_oid(results, idxForm->indexrelid);
9501005
foundValid |= idxForm->indisvalid;
9511006
next:

0 commit comments

Comments
 (0)