On Sat, Dec 26, 2015 at 7:10 AM, Jeff Janes <jeff.ja...@gmail.com> wrote:
> brin_summarize_new_values() does not check that it is called on the
> correct type of index, nor does it check permissions.

Yeah, it should have those sanity checks... On top of that this is not
a really user-friendly message:
=# select brin_summarize_new_values('brin_example'::regclass);
ERROR:  XX000: cache lookup failed for index 16391
LOCATION:  IndexGetRelation, index.c:3279

What do you think about the attached? This should definitely be fixed
by 9.5.0...
-- 
Michael
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 99337b0..19478d1 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -789,8 +789,30 @@ brin_summarize_new_values(PG_FUNCTION_ARGS)
 	Oid			indexoid = PG_GETARG_OID(0);
 	Relation	indexRel;
 	Relation	heapRel;
+	Relation	relation;
 	double		numSummarized = 0;
 
+	relation = relation_open(indexoid, ShareUpdateExclusiveLock);
+	if (relation->rd_rel->relkind != RELKIND_INDEX)
+		ereport(ERROR,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("\"%s\" is not an index",
+						RelationGetRelationName(relation))));
+
+	if (relation->rd_rel->relam != BRIN_AM_OID)
+		ereport(ERROR,
+				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+				 errmsg("\"%s\" is not a BRIN index",
+						RelationGetRelationName(relation))));
+
+	if (pg_class_aclcheck(indexoid, GetUserId(), ACL_SELECT) != ACLCHECK_OK)
+		ereport(ERROR,
+				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+				 errmsg("permission denied for index %s",
+						RelationGetRelationName(relation))));
+
+	relation_close(relation, ShareUpdateExclusiveLock);
+
 	heapRel = heap_open(IndexGetRelation(indexoid, false),
 						ShareUpdateExclusiveLock);
 	indexRel = index_open(indexoid, ShareUpdateExclusiveLock);
diff --git a/src/test/regress/expected/brin.out b/src/test/regress/expected/brin.out
index 0937b63..9dd5e6e 100644
--- a/src/test/regress/expected/brin.out
+++ b/src/test/regress/expected/brin.out
@@ -407,3 +407,26 @@ 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
+CREATE TABLE brin_example (id int);
+CREATE INDEX btree_index ON brin_example(id);
+CREATE INDEX brin_index ON brin_example USING brin(id);
+-- Checks on relation type
+SELECT brin_summarize_new_values('brin_example'::regclass); -- error
+ERROR:  "brin_example" is not an index
+SELECT brin_summarize_new_values('btree_index'::regclass); -- error
+ERROR:  "btree_index" is not a BRIN index
+SELECT brin_summarize_new_values('brin_index'::regclass); -- ok
+ brin_summarize_new_values 
+---------------------------
+                         0
+(1 row)
+
+-- Permission checks
+CREATE ROLE brin_role;
+SET SESSION AUTHORIZATION brin_role;
+SELECT brin_summarize_new_values('brin_index'::regclass); -- error
+ERROR:  permission denied for index brin_index
+RESET SESSION AUTHORIZATION;
+DROP ROLE brin_role;
+DROP TABLE brin_example;
diff --git a/src/test/regress/sql/brin.sql b/src/test/regress/sql/brin.sql
index db78d05..69a3ba8 100644
--- a/src/test/regress/sql/brin.sql
+++ b/src/test/regress/sql/brin.sql
@@ -416,3 +416,19 @@ 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
+CREATE TABLE brin_example (id int);
+CREATE INDEX btree_index ON brin_example(id);
+CREATE INDEX brin_index ON brin_example USING brin(id);
+-- Checks on relation type
+SELECT brin_summarize_new_values('brin_example'::regclass); -- error
+SELECT brin_summarize_new_values('btree_index'::regclass); -- error
+SELECT brin_summarize_new_values('brin_index'::regclass); -- ok
+-- Permission checks
+CREATE ROLE brin_role;
+SET SESSION AUTHORIZATION brin_role;
+SELECT brin_summarize_new_values('brin_index'::regclass); -- error
+RESET SESSION AUTHORIZATION;
+DROP ROLE brin_role;
+DROP TABLE brin_example;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to