From 25b1a6a215434881effc5a3355397a446d2dcaf8 Mon Sep 17 00:00:00 2001 From: nkey Date: Thu, 20 Feb 2025 14:51:19 +0300 Subject: [PATCH v9 3/4] 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 --- src/backend/optimizer/util/plancat.c | 121 +++++++++++++++++++-------- 1 file changed, 88 insertions(+), 33 deletions(-) diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c index 63f2c90340a..0c720e450e9 100644 --- a/src/backend/optimizer/util/plancat.c +++ b/src/backend/optimizer/util/plancat.c @@ -716,9 +716,10 @@ infer_arbiter_indexes(PlannerInfo *root) List *indexList; ListCell *l; - /* Normalized inference attributes and inference expressions: */ - Bitmapset *inferAttrs = NULL; - List *inferElems = NIL; + /* Normalized required attributes and expressions: */ + Bitmapset *requiredArbiterAttrs = NULL; + List *requiredArbiterElems = NIL; + List *requiredIndexPredExprs = (List *) onconflict->arbiterWhere; /* Results */ List *results = NIL; @@ -757,8 +758,8 @@ infer_arbiter_indexes(PlannerInfo *root) if (!IsA(elem->expr, Var)) { - /* If not a plain Var, just shove it in inferElems for now */ - inferElems = lappend(inferElems, elem->expr); + /* If not a plain Var, just shove it in requiredArbiterElems for now */ + requiredArbiterElems = lappend(requiredArbiterElems, elem->expr); continue; } @@ -770,30 +771,76 @@ infer_arbiter_indexes(PlannerInfo *root) (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("whole row unique index inference specifications are not supported"))); - inferAttrs = bms_add_member(inferAttrs, + requiredArbiterAttrs = bms_add_member(requiredArbiterAttrs, attno - FirstLowInvalidHeapAttributeNumber); } + indexList = RelationGetIndexList(relation); + /* * Lookup named constraint's index. This is not immediately returned - * because some additional sanity checks are required. + * because some additional sanity checks are required. Additionally, we + * need to process other indexes as potential arbiters to account for + * cases where REINDEX CONCURRENTLY is processing an index used as a + * named constraint. */ if (onconflict->constraint != InvalidOid) { indexOidFromConstraint = get_constraint_index(onconflict->constraint); if (indexOidFromConstraint == InvalidOid) + { ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("constraint in ON CONFLICT clause has no associated index"))); + errmsg("constraint in ON CONFLICT clause has no associated index"))); + } + + /* + * Find the named constraint index to extract its attributes and predicates. + * We open all indexes in the loop to avoid deadlock of changed order of locks. + * */ + foreach(l, indexList) + { + Oid indexoid = lfirst_oid(l); + Relation idxRel; + Form_pg_index idxForm; + AttrNumber natt; + + idxRel = index_open(indexoid, rte->rellockmode); + idxForm = idxRel->rd_index; + + if (idxForm->indisready) + { + if (indexOidFromConstraint == idxForm->indexrelid) + { + /* + * Prepare requirements for other indexes to be used as arbiter together + * with indexOidFromConstraint. It is required to involve both equals indexes + * in case of REINDEX CONCURRENTLY. + */ + for (natt = 0; natt < idxForm->indnkeyatts; natt++) + { + int attno = idxRel->rd_index->indkey.values[natt]; + + if (attno != 0) + requiredArbiterAttrs = bms_add_member(requiredArbiterAttrs, + attno - FirstLowInvalidHeapAttributeNumber); + } + requiredArbiterElems = RelationGetIndexExpressions(idxRel); + requiredIndexPredExprs = RelationGetIndexPredicate(idxRel); + /* We are done, so, quite the loop. */ + index_close(idxRel, NoLock); + break; + } + } + index_close(idxRel, NoLock); + } } /* * Using that representation, iterate through the list of indexes on the * target relation to try and find a match */ - indexList = RelationGetIndexList(relation); - foreach(l, indexList) { Oid indexoid = lfirst_oid(l); @@ -842,26 +889,23 @@ infer_arbiter_indexes(PlannerInfo *root) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("ON CONFLICT DO UPDATE not supported with exclusion constraints"))); - - results = lappend_oid(results, idxForm->indexrelid); - foundValid |= idxForm->indisvalid; - index_close(idxRel, NoLock); - break; + goto found; } else if (indexOidFromConstraint != InvalidOid) { - /* No point in further work for index in named constraint case */ - goto next; + /* In the case of "ON constraint_name DO UPDATE" we need to skip non-unique candidates. */ + if (!idxForm->indisunique && onconflict->action == ONCONFLICT_UPDATE) + goto next; + } else { + /* + * Only considering conventional inference at this point (not named + * constraints), so index under consideration can be immediately + * skipped if it's not unique + */ + if (!idxForm->indisunique) + goto next; } - /* - * Only considering conventional inference at this point (not named - * constraints), so index under consideration can be immediately - * skipped if it's not unique - */ - if (!idxForm->indisunique) - goto next; - /* * So-called unique constraints with WITHOUT OVERLAPS are really * exclusion constraints, so skip those too. @@ -881,7 +925,7 @@ infer_arbiter_indexes(PlannerInfo *root) } /* Non-expression attributes (if any) must match */ - if (!bms_equal(indexedAttrs, inferAttrs)) + if (!bms_equal(indexedAttrs, requiredArbiterAttrs)) goto next; /* Expression attributes (if any) must match */ @@ -889,6 +933,10 @@ infer_arbiter_indexes(PlannerInfo *root) if (idxExprs && varno != 1) ChangeVarNodes((Node *) idxExprs, 1, varno, 0); + /* + * If arbiterElems are present, check them. If name >constraint is + * present arbiterElems == NIL. + */ foreach(el, onconflict->arbiterElems) { InferenceElem *elem = (InferenceElem *) lfirst(el); @@ -926,26 +974,33 @@ infer_arbiter_indexes(PlannerInfo *root) } /* - * Now that all inference elements were matched, ensure that the + * In case of the conventional inference involved ensure that the * expression elements from inference clause are not missing any * cataloged expressions. This does the right thing when unique * indexes redundantly repeat the same attribute, or if attributes * redundantly appear multiple times within an inference clause. + * + * In the case of named constraint ensure candidate has equal set + * of expressions as the named constraint index. */ - if (list_difference(idxExprs, inferElems) != NIL) + if (list_difference(idxExprs, requiredArbiterElems) != NIL) goto next; - /* - * If it's a partial index, its predicate must be implied by the ON - * CONFLICT's WHERE clause. - */ predExprs = RelationGetIndexPredicate(idxRel); if (predExprs && varno != 1) ChangeVarNodes((Node *) predExprs, 1, varno, 0); - if (!predicate_implied_by(predExprs, (List *) onconflict->arbiterWhere, false)) + /* + * If it's a partial index and conventional inference, its predicate must be implied + * by the ON CONFLICT's WHERE clause. + */ + if (indexOidFromConstraint == InvalidOid && !predicate_implied_by(predExprs, requiredIndexPredExprs, false)) + goto next; + /* If it's a partial index and named constraint predicates must be equal. */ + if (indexOidFromConstraint != InvalidOid && list_difference(predExprs, requiredIndexPredExprs) != NIL) goto next; +found: results = lappend_oid(results, idxForm->indexrelid); foundValid |= idxForm->indisvalid; next: -- 2.43.0