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

Commit 3d2b31e

Browse files
committed
Fix brin_summarize_new_values() to check index type and ownership.
brin_summarize_new_values() did not check that the passed OID was for an index at all, much less that it was a BRIN index, and would fail in obscure ways if it wasn't (possibly damaging data first?). It also lacked any permissions test; by analogy to VACUUM, we should only allow the table's owner to summarize. Noted by Jeff Janes, fix by Michael Paquier and me
1 parent 8014c44 commit 3d2b31e

File tree

3 files changed

+55
-3
lines changed

3 files changed

+55
-3
lines changed

src/backend/access/brin/brin.c

+39-3
Original file line numberDiff line numberDiff line change
@@ -787,14 +787,50 @@ Datum
787787
brin_summarize_new_values(PG_FUNCTION_ARGS)
788788
{
789789
Oid indexoid = PG_GETARG_OID(0);
790+
Oid heapoid;
790791
Relation indexRel;
791792
Relation heapRel;
792793
double numSummarized = 0;
793794

794-
heapRel = heap_open(IndexGetRelation(indexoid, false),
795-
ShareUpdateExclusiveLock);
795+
/*
796+
* We must lock table before index to avoid deadlocks. However, if the
797+
* passed indexoid isn't an index then IndexGetRelation() will fail.
798+
* Rather than emitting a not-very-helpful error message, postpone
799+
* complaining, expecting that the is-it-an-index test below will fail.
800+
*/
801+
heapoid = IndexGetRelation(indexoid, true);
802+
if (OidIsValid(heapoid))
803+
heapRel = heap_open(heapoid, ShareUpdateExclusiveLock);
804+
else
805+
heapRel = NULL;
806+
796807
indexRel = index_open(indexoid, ShareUpdateExclusiveLock);
797808

809+
/* Must be a BRIN index */
810+
if (indexRel->rd_rel->relkind != RELKIND_INDEX ||
811+
indexRel->rd_rel->relam != BRIN_AM_OID)
812+
ereport(ERROR,
813+
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
814+
errmsg("\"%s\" is not a BRIN index",
815+
RelationGetRelationName(indexRel))));
816+
817+
/* User must own the index (comparable to privileges needed for VACUUM) */
818+
if (!pg_class_ownercheck(indexoid, GetUserId()))
819+
aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_CLASS,
820+
RelationGetRelationName(indexRel));
821+
822+
/*
823+
* Since we did the IndexGetRelation call above without any lock, it's
824+
* barely possible that a race against an index drop/recreation could have
825+
* netted us the wrong table. Recheck.
826+
*/
827+
if (heapRel == NULL || heapoid != IndexGetRelation(indexoid, false))
828+
ereport(ERROR,
829+
(errcode(ERRCODE_UNDEFINED_TABLE),
830+
errmsg("could not open parent table of index %s",
831+
RelationGetRelationName(indexRel))));
832+
833+
/* OK, do it */
798834
brinsummarize(indexRel, heapRel, &numSummarized, NULL);
799835

800836
relation_close(indexRel, ShareUpdateExclusiveLock);
@@ -962,7 +998,7 @@ summarize_range(IndexInfo *indexInfo, BrinBuildState *state, Relation heapRel,
962998
*/
963999
state->bs_currRangeStart = heapBlk;
9641000
scanNumBlks = heapBlk + state->bs_pagesPerRange <= heapNumBlks ?
965-
state->bs_pagesPerRange : heapNumBlks - heapBlk;
1001+
state->bs_pagesPerRange : heapNumBlks - heapBlk;
9661002
IndexBuildHeapRangeScan(heapRel, state->bs_irel, indexInfo, false, true,
9671003
heapBlk, scanNumBlks,
9681004
brinbuildCallback, (void *) state);

src/test/regress/expected/brin.out

+11
Original file line numberDiff line numberDiff line change
@@ -407,3 +407,14 @@ FROM tenk1 ORDER BY unique2 LIMIT 5 OFFSET 5;
407407
VACUUM brintest; -- force a summarization cycle in brinidx
408408
UPDATE brintest SET int8col = int8col * int4col;
409409
UPDATE brintest SET textcol = '' WHERE textcol IS NOT NULL;
410+
-- Tests for brin_summarize_new_values
411+
SELECT brin_summarize_new_values('brintest'); -- error, not an index
412+
ERROR: "brintest" is not an index
413+
SELECT brin_summarize_new_values('tenk1_unique1'); -- error, not a BRIN index
414+
ERROR: "tenk1_unique1" is not a BRIN index
415+
SELECT brin_summarize_new_values('brinidx'); -- ok, no change expected
416+
brin_summarize_new_values
417+
---------------------------
418+
0
419+
(1 row)
420+

src/test/regress/sql/brin.sql

+5
Original file line numberDiff line numberDiff line change
@@ -416,3 +416,8 @@ VACUUM brintest; -- force a summarization cycle in brinidx
416416

417417
UPDATE brintest SET int8col = int8col * int4col;
418418
UPDATE brintest SET textcol = '' WHERE textcol IS NOT NULL;
419+
420+
-- Tests for brin_summarize_new_values
421+
SELECT brin_summarize_new_values('brintest'); -- error, not an index
422+
SELECT brin_summarize_new_values('tenk1_unique1'); -- error, not a BRIN index
423+
SELECT brin_summarize_new_values('brinidx'); -- ok, no change expected

0 commit comments

Comments
 (0)