diff --git a/src/backend/statistics/extended_stats.c b/src/backend/statistics/extended_stats.c index ee8f2efb3a..5922909fe6 100644 --- a/src/backend/statistics/extended_stats.c +++ b/src/backend/statistics/extended_stats.c @@ -1316,10 +1316,38 @@ choose_best_statistics(List *stats, char requiredkind, * statext_is_compatible_clause_internal * Determines if the clause is compatible with MCV lists. * - * Does the heavy lifting of actually inspecting the clauses for - * statext_is_compatible_clause. It needs to be split like this because - * of recursion. The attnums bitmap is an input/output parameter collecting - * attribute numbers from all compatible clauses (recursively). + * To be compatible, the given clause must be a combination of supported + * clauses built from Vars or sub-expressions (where a sub-expression is + * something that exactly matches an expression found in statistics objects). + * This function recursively examines the clause and extracts any + * sub-expressions that will need to be matched against statistics. + * + * Currently, we only support the following types of clauses: + * + * (a) OpExprs of the form (Var/Expr op Const), or (Const op Var/Expr), where + * the op is one of ("=", "<", ">", ">=", "<=") + * + * (b) (Var/Expr IS [NOT] NULL) + * + * (c) combinations using AND/OR/NOT + * + * (d) ScalarArrayOpExprs of the form (Var/Expr op ANY (array)) or (Var/Expr + * op ALL (array)) + * + * In the future, the range of supported clauses may be expanded to more + * complex cases, for example (Var op Var). + * + * Arguments: + * clause: (sub)clause to be inspected (bare clause, not a RestrictInfo) + * relid: rel that all Vars in clause must belong to + * *attnums: input/output parameter collecting attribute numbers of all + * mentioned Vars. Note that we do not offset the attribute numbers, + * so we can't cope with system columns. + * *exprs: input/output parameter collecting primitive subclauses within + * the clause tree + * + * Returns false if there is something we definitively can't handle. + * On true return, we can proceed to match the *exprs against statistics. */ static bool statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause, @@ -1343,10 +1371,14 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause, if (var->varlevelsup > 0) return false; - /* Also skip system attributes (we don't allow stats on those). */ + /* + * Also reject system attributes and whole-row Vars (we don't allow + * stats on those). + */ if (!AttrNumberIsForUserDefinedAttr(var->varattno)) return false; + /* OK, record the attnum for later permissions checks. */ *attnums = bms_add_member(*attnums, var->varattno); return true; @@ -1501,7 +1533,7 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause, foreach(lc, expr->args) { /* - * Had we found incompatible clause in the arguments, treat the + * If we find an incompatible clause in the arguments, treat the * whole clause as incompatible. */ if (!statext_is_compatible_clause_internal(root, @@ -1540,27 +1572,28 @@ statext_is_compatible_clause_internal(PlannerInfo *root, Node *clause, * statext_is_compatible_clause * Determines if the clause is compatible with MCV lists. * - * Currently, we only support the following types of clauses: + * See statext_is_compatible_clause_internal, above, for the basic rules. + * This layer deals with RestrictInfo superstructure and applies permissions + * checks to verify that it's okay to examine all mentioned Vars. * - * (a) OpExprs of the form (Var/Expr op Const), or (Const op Var/Expr), where - * the op is one of ("=", "<", ">", ">=", "<=") + * Arguments: + * clause: clause to be inspected (in RestrictInfo form) + * relid: rel that all Vars in clause must belong to + * *attnums: input/output parameter collecting attribute numbers of all + * mentioned Vars. Note that we do not offset the attribute numbers, + * so we can't cope with system columns. + * *exprs: input/output parameter collecting primitive subclauses within + * the clause tree * - * (b) (Var/Expr IS [NOT] NULL) - * - * (c) combinations using AND/OR/NOT - * - * (d) ScalarArrayOpExprs of the form (Var/Expr op ANY (array)) or (Var/Expr - * op ALL (array)) - * - * In the future, the range of supported clauses may be expanded to more - * complex cases, for example (Var op Var). + * Returns false if there is something we definitively can't handle. + * On true return, we can proceed to match the *exprs against statistics. */ static bool statext_is_compatible_clause(PlannerInfo *root, Node *clause, Index relid, Bitmapset **attnums, List **exprs) { RangeTblEntry *rte = root->simple_rte_array[relid]; - RestrictInfo *rinfo = (RestrictInfo *) clause; + RestrictInfo *rinfo; int clause_relid; Oid userid; @@ -1589,8 +1622,9 @@ statext_is_compatible_clause(PlannerInfo *root, Node *clause, Index relid, } /* Otherwise it must be a RestrictInfo. */ - if (!IsA(rinfo, RestrictInfo)) + if (!IsA(clause, RestrictInfo)) return false; + rinfo = (RestrictInfo *) clause; /* Pseudoconstants are not really interesting here. */ if (rinfo->pseudoconstant) @@ -1612,34 +1646,48 @@ statext_is_compatible_clause(PlannerInfo *root, Node *clause, Index relid, */ userid = rte->checkAsUser ? rte->checkAsUser : GetUserId(); + /* Table-level SELECT privilege is sufficient for all columns */ if (pg_class_aclcheck(rte->relid, userid, ACL_SELECT) != ACLCHECK_OK) { Bitmapset *clause_attnums = NULL; + int attnum = -1; - /* Don't have table privilege, must check individual columns */ + /* + * We have to check per-column privileges. *attnums has the attnums + * for individual Vars we saw, but there may also be Vars within + * subexpressions in *exprs. We can use pull_varattnos() to extract + * those, but there's an impedance mismatch: attnums returned by + * pull_varattnos() are offset by FirstLowInvalidHeapAttributeNumber, + * while attnums within *attnums aren't. Convert *attnums to the + * offset style so we can combine the results. + */ + while ((attnum = bms_next_member(*attnums, attnum)) >= 0) + { + clause_attnums = + bms_add_member(clause_attnums, + attnum - FirstLowInvalidHeapAttributeNumber); + } + + /* Now merge attnums from *exprs into clause_attnums */ if (*exprs != NIL) - { - pull_varattnos((Node *) exprs, relid, &clause_attnums); - clause_attnums = bms_add_members(clause_attnums, *attnums); - } - else - clause_attnums = *attnums; + pull_varattnos((Node *) *exprs, relid, &clause_attnums); - if (bms_is_member(InvalidAttrNumber, clause_attnums)) + attnum = -1; + while ((attnum = bms_next_member(clause_attnums, attnum)) >= 0) { - /* Have a whole-row reference, must have access to all columns */ - if (pg_attribute_aclcheck_all(rte->relid, userid, ACL_SELECT, - ACLMASK_ALL) != ACLCHECK_OK) - return false; - } - else - { - /* Check the columns referenced by the clause */ - int attnum = -1; + /* Undo the offset */ + AttrNumber attno = attnum + FirstLowInvalidHeapAttributeNumber; - while ((attnum = bms_next_member(clause_attnums, attnum)) >= 0) + if (attno == InvalidAttrNumber) { - if (pg_attribute_aclcheck(rte->relid, attnum, userid, + /* Whole-row reference, so must have access to all columns */ + if (pg_attribute_aclcheck_all(rte->relid, userid, ACL_SELECT, + ACLMASK_ALL) != ACLCHECK_OK) + return false; + } + else + { + if (pg_attribute_aclcheck(rte->relid, attno, userid, ACL_SELECT) != ACLCHECK_OK) return false; } diff --git a/src/test/regress/expected/stats_ext.out b/src/test/regress/expected/stats_ext.out index b0548108f0..b494411074 100644 --- a/src/test/regress/expected/stats_ext.out +++ b/src/test/regress/expected/stats_ext.out @@ -3183,6 +3183,10 @@ GRANT USAGE ON SCHEMA tststats TO regress_stats_user1; SET SESSION AUTHORIZATION regress_stats_user1; SELECT * FROM tststats.priv_test_tbl; -- Permission denied ERROR: permission denied for table priv_test_tbl +-- Check individual columns if we don't have table privilege +SELECT * FROM tststats.priv_test_tbl + WHERE a = 1 and tststats.priv_test_tbl.* > (1, 1) is not null; +ERROR: permission denied for table priv_test_tbl -- Attempt to gain access using a leaky operator CREATE FUNCTION op_leak(int, int) RETURNS bool AS 'BEGIN RAISE NOTICE ''op_leak => %, %'', $1, $2; RETURN $1 < $2; END' diff --git a/src/test/regress/sql/stats_ext.sql b/src/test/regress/sql/stats_ext.sql index d2c59b0a8a..953c7acfc8 100644 --- a/src/test/regress/sql/stats_ext.sql +++ b/src/test/regress/sql/stats_ext.sql @@ -1600,6 +1600,10 @@ GRANT USAGE ON SCHEMA tststats TO regress_stats_user1; SET SESSION AUTHORIZATION regress_stats_user1; SELECT * FROM tststats.priv_test_tbl; -- Permission denied +-- Check individual columns if we don't have table privilege +SELECT * FROM tststats.priv_test_tbl + WHERE a = 1 and tststats.priv_test_tbl.* > (1, 1) is not null; + -- Attempt to gain access using a leaky operator CREATE FUNCTION op_leak(int, int) RETURNS bool AS 'BEGIN RAISE NOTICE ''op_leak => %, %'', $1, $2; RETURN $1 < $2; END'