On 07/29/2015 02:41 AM, Dean Rasheed wrote: > I don't think there is any point in adding the new function > transformPolicyClause(), which is identical to transformWhereClause(). > You can just use transformWhereClause() with EXPR_KIND_POLICY. It's > already used for lots of other expression kinds.
Ok -- I went back to using transformWhereClause. I'd still prefer to change the name -- more than half the uses of the function are for other than EXPR_KIND_WHERE -- but I don't feel that strongly about it. > There are a couple of places in test_rls_hooks.c that also need updating. done > I think that check_agglevels_and_constraints() and > transformWindowFuncCall() could be made to emit more targeted error > messages for EXPR_KIND_POLICY, for example "aggregate functions are > not allowed in policy USING and WITH CHECK expressions". done Unless there are other comments/objections, will commit this later today. Joe
diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c index d8b4390..bcf4a8f 100644 *** a/src/backend/commands/policy.c --- b/src/backend/commands/policy.c *************** CreatePolicy(CreatePolicyStmt *stmt) *** 534,545 **** qual = transformWhereClause(qual_pstate, copyObject(stmt->qual), ! EXPR_KIND_WHERE, "POLICY"); with_check_qual = transformWhereClause(with_check_pstate, copyObject(stmt->with_check), ! EXPR_KIND_WHERE, "POLICY"); /* Fix up collation information */ --- 534,545 ---- qual = transformWhereClause(qual_pstate, copyObject(stmt->qual), ! EXPR_KIND_POLICY, "POLICY"); with_check_qual = transformWhereClause(with_check_pstate, copyObject(stmt->with_check), ! EXPR_KIND_POLICY, "POLICY"); /* Fix up collation information */ *************** AlterPolicy(AlterPolicyStmt *stmt) *** 707,713 **** addRTEtoQuery(qual_pstate, rte, false, true, true); qual = transformWhereClause(qual_pstate, copyObject(stmt->qual), ! EXPR_KIND_WHERE, "POLICY"); /* Fix up collation information */ --- 707,713 ---- addRTEtoQuery(qual_pstate, rte, false, true, true); qual = transformWhereClause(qual_pstate, copyObject(stmt->qual), ! EXPR_KIND_POLICY, "POLICY"); /* Fix up collation information */ *************** AlterPolicy(AlterPolicyStmt *stmt) *** 730,736 **** with_check_qual = transformWhereClause(with_check_pstate, copyObject(stmt->with_check), ! EXPR_KIND_WHERE, "POLICY"); /* Fix up collation information */ --- 730,736 ---- with_check_qual = transformWhereClause(with_check_pstate, copyObject(stmt->with_check), ! EXPR_KIND_POLICY, "POLICY"); /* Fix up collation information */ diff --git a/src/backend/parser/parse_agg.c b/src/backend/parser/parse_agg.c index 478d8ca..e33adf5 100644 *** a/src/backend/parser/parse_agg.c --- b/src/backend/parser/parse_agg.c *************** check_agglevels_and_constraints(ParseSta *** 373,378 **** --- 373,385 ---- case EXPR_KIND_WHERE: errkind = true; break; + case EXPR_KIND_POLICY: + if (isAgg) + err = _("aggregate functions are not allowed in POLICY USING and WITH CHECK expressions"); + else + err = _("grouping operations are not allowed in POLICY USING and WITH CHECK expressions"); + + break; case EXPR_KIND_HAVING: /* okay */ break; *************** transformWindowFuncCall(ParseState *psta *** 770,775 **** --- 777,785 ---- case EXPR_KIND_WHERE: errkind = true; break; + case EXPR_KIND_POLICY: + err = _("aggregate functions are not allowed in POLICY USING and WITH CHECK expressions"); + break; case EXPR_KIND_HAVING: errkind = true; break; diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 0ff46dd..fa77ef1 100644 *** a/src/backend/parser/parse_expr.c --- b/src/backend/parser/parse_expr.c *************** transformSubLink(ParseState *pstate, Sub *** 1672,1677 **** --- 1672,1678 ---- case EXPR_KIND_FROM_SUBSELECT: case EXPR_KIND_FROM_FUNCTION: case EXPR_KIND_WHERE: + case EXPR_KIND_POLICY: case EXPR_KIND_HAVING: case EXPR_KIND_FILTER: case EXPR_KIND_WINDOW_PARTITION: *************** ParseExprKindName(ParseExprKind exprKind *** 3173,3178 **** --- 3174,3181 ---- return "function in FROM"; case EXPR_KIND_WHERE: return "WHERE"; + case EXPR_KIND_POLICY: + return "POLICY"; case EXPR_KIND_HAVING: return "HAVING"; case EXPR_KIND_FILTER: diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h index 7ecaffc..5249945 100644 *** a/src/include/parser/parse_node.h --- b/src/include/parser/parse_node.h *************** typedef enum ParseExprKind *** 63,69 **** EXPR_KIND_INDEX_PREDICATE, /* index predicate */ EXPR_KIND_ALTER_COL_TRANSFORM, /* transform expr in ALTER COLUMN TYPE */ EXPR_KIND_EXECUTE_PARAMETER, /* parameter value in EXECUTE */ ! EXPR_KIND_TRIGGER_WHEN /* WHEN condition in CREATE TRIGGER */ } ParseExprKind; --- 63,70 ---- EXPR_KIND_INDEX_PREDICATE, /* index predicate */ EXPR_KIND_ALTER_COL_TRANSFORM, /* transform expr in ALTER COLUMN TYPE */ EXPR_KIND_EXECUTE_PARAMETER, /* parameter value in EXECUTE */ ! EXPR_KIND_TRIGGER_WHEN, /* WHEN condition in CREATE TRIGGER */ ! EXPR_KIND_POLICY /* USING or WITH CHECK expr in policy */ } ParseExprKind; diff --git a/src/test/modules/test_rls_hooks/test_rls_hooks.c b/src/test/modules/test_rls_hooks/test_rls_hooks.c index 61b62d5..d76b17a 100644 *** a/src/test/modules/test_rls_hooks/test_rls_hooks.c --- b/src/test/modules/test_rls_hooks/test_rls_hooks.c *************** test_rls_hooks_permissive(CmdType cmdtyp *** 106,112 **** e = (Node *) makeSimpleA_Expr(AEXPR_OP, "=", (Node *) n, (Node *) c, 0); policy->qual = (Expr *) transformWhereClause(qual_pstate, copyObject(e), ! EXPR_KIND_WHERE, "POLICY"); policy->with_check_qual = copyObject(policy->qual); --- 106,112 ---- e = (Node *) makeSimpleA_Expr(AEXPR_OP, "=", (Node *) n, (Node *) c, 0); policy->qual = (Expr *) transformWhereClause(qual_pstate, copyObject(e), ! EXPR_KIND_POLICY, "POLICY"); policy->with_check_qual = copyObject(policy->qual); *************** test_rls_hooks_restrictive(CmdType cmdty *** 160,166 **** e = (Node *) makeSimpleA_Expr(AEXPR_OP, "=", (Node *) n, (Node *) c, 0); policy->qual = (Expr *) transformWhereClause(qual_pstate, copyObject(e), ! EXPR_KIND_WHERE, "POLICY"); policy->with_check_qual = copyObject(policy->qual); --- 160,166 ---- e = (Node *) makeSimpleA_Expr(AEXPR_OP, "=", (Node *) n, (Node *) c, 0); policy->qual = (Expr *) transformWhereClause(qual_pstate, copyObject(e), ! EXPR_KIND_POLICY, "POLICY"); policy->with_check_qual = copyObject(policy->qual); diff --git a/src/test/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index b146da3..22e89bc 100644 *** a/src/test/regress/expected/rowsecurity.out --- b/src/test/regress/expected/rowsecurity.out *************** CREATE RULE "_RETURN" AS ON SELECT TO t *** 3024,3029 **** --- 3024,3038 ---- SELECT * FROM generate_series(1,5) t0(c); -- succeeds ROLLBACK; -- + -- Policy expression handling + -- + BEGIN; + SET row_security = FORCE; + CREATE TABLE t (c) AS VALUES ('bar'::text); + CREATE POLICY p ON t USING (max(c)); -- fails: aggregate functions are not allowed in POLICY + ERROR: aggregate functions are not allowed in POLICY USING and WITH CHECK expressions + ROLLBACK; + -- -- Clean up objects -- RESET SESSION AUTHORIZATION; diff --git a/src/test/regress/sql/rowsecurity.sql b/src/test/regress/sql/rowsecurity.sql index 54f2c89..47a2703 100644 *** a/src/test/regress/sql/rowsecurity.sql --- b/src/test/regress/sql/rowsecurity.sql *************** CREATE RULE "_RETURN" AS ON SELECT TO t *** 1290,1295 **** --- 1290,1304 ---- ROLLBACK; -- + -- Policy expression handling + -- + BEGIN; + SET row_security = FORCE; + CREATE TABLE t (c) AS VALUES ('bar'::text); + CREATE POLICY p ON t USING (max(c)); -- fails: aggregate functions are not allowed in POLICY + ROLLBACK; + + -- -- Clean up objects -- RESET SESSION AUTHORIZATION;
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers