diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index 99337b0f0c..2622a7efb1 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -787,14 +787,50 @@ Datum brin_summarize_new_values(PG_FUNCTION_ARGS) { Oid indexoid = PG_GETARG_OID(0); + Oid heapoid; Relation indexRel; Relation heapRel; double numSummarized = 0; - heapRel = heap_open(IndexGetRelation(indexoid, false), - ShareUpdateExclusiveLock); + /* + * We must lock table before index to avoid deadlocks. However, if the + * passed indexoid isn't an index then IndexGetRelation() will fail. + * Rather than emitting a not-very-helpful error message, postpone + * complaining, expecting that the is-it-an-index test below will fail. + */ + heapoid = IndexGetRelation(indexoid, true); + if (OidIsValid(heapoid)) + heapRel = heap_open(heapoid, ShareUpdateExclusiveLock); + else + heapRel = NULL; + indexRel = index_open(indexoid, ShareUpdateExclusiveLock); + /* Must be a BRIN index */ + if (indexRel->rd_rel->relkind != RELKIND_INDEX || + indexRel->rd_rel->relam != BRIN_AM_OID) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("\"%s\" is not a BRIN index", + RelationGetRelationName(indexRel)))); + + /* User must own the index (comparable to privileges needed for VACUUM) */ + if (!pg_class_ownercheck(indexoid, GetUserId())) + aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS, + RelationGetRelationName(indexRel)); + + /* + * Since we did the IndexGetRelation call above without any lock, it's + * barely possible that a race against an index drop/recreation could have + * netted us the wrong table. Recheck. + */ + if (heapRel == NULL || heapoid != IndexGetRelation(indexoid, false)) + ereport(ERROR, + (errcode(ERRCODE_UNDEFINED_TABLE), + errmsg("could not open parent table of index %s", + RelationGetRelationName(indexRel)))); + + /* OK, do it */ brinsummarize(indexRel, heapRel, &numSummarized, NULL); relation_close(indexRel, ShareUpdateExclusiveLock); @@ -962,7 +998,7 @@ summarize_range(IndexInfo *indexInfo, BrinBuildState *state, Relation heapRel, */ state->bs_currRangeStart = heapBlk; scanNumBlks = heapBlk + state->bs_pagesPerRange <= heapNumBlks ? - state->bs_pagesPerRange : heapNumBlks - heapBlk; + state->bs_pagesPerRange : heapNumBlks - heapBlk; IndexBuildHeapRangeScan(heapRel, state->bs_irel, indexInfo, false, true, heapBlk, scanNumBlks, brinbuildCallback, (void *) state); diff --git a/src/test/regress/expected/brin.out b/src/test/regress/expected/brin.out index 0937b63667..475525912f 100644 --- a/src/test/regress/expected/brin.out +++ b/src/test/regress/expected/brin.out @@ -407,3 +407,14 @@ FROM tenk1 ORDER BY unique2 LIMIT 5 OFFSET 5; VACUUM brintest; -- force a summarization cycle in brinidx UPDATE brintest SET int8col = int8col * int4col; UPDATE brintest SET textcol = '' WHERE textcol IS NOT NULL; +-- Tests for brin_summarize_new_values +SELECT brin_summarize_new_values('brintest'); -- error, not an index +ERROR: "brintest" is not an index +SELECT brin_summarize_new_values('tenk1_unique1'); -- error, not a BRIN index +ERROR: "tenk1_unique1" is not a BRIN index +SELECT brin_summarize_new_values('brinidx'); -- ok, no change expected + brin_summarize_new_values +--------------------------- + 0 +(1 row) + diff --git a/src/test/regress/sql/brin.sql b/src/test/regress/sql/brin.sql index db78d05ff3..9e4836e17e 100644 --- a/src/test/regress/sql/brin.sql +++ b/src/test/regress/sql/brin.sql @@ -416,3 +416,8 @@ VACUUM brintest; -- force a summarization cycle in brinidx UPDATE brintest SET int8col = int8col * int4col; UPDATE brintest SET textcol = '' WHERE textcol IS NOT NULL; + +-- Tests for brin_summarize_new_values +SELECT brin_summarize_new_values('brintest'); -- error, not an index +SELECT brin_summarize_new_values('tenk1_unique1'); -- error, not a BRIN index +SELECT brin_summarize_new_values('brinidx'); -- ok, no change expected