On 07/03/2015 10:03 AM, Noah Misch wrote: > (7) Using an aggregate function in a policy predicate elicits an inapposite > error message due to use of EXPR_KIND_WHERE for parse analysis. Need a new > ParseExprKind. Test case:
Patch attached. Comments? Joe
diff --git a/src/backend/commands/policy.c b/src/backend/commands/policy.c index 4642d7c..d8ef496 100644 *** a/src/backend/commands/policy.c --- b/src/backend/commands/policy.c *************** CreatePolicy(CreatePolicyStmt *stmt) *** 532,545 **** NULL, false, false); addRTEtoQuery(with_check_pstate, rte, false, true, true); ! 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 */ --- 532,545 ---- NULL, false, false); addRTEtoQuery(with_check_pstate, rte, false, true, true); ! qual = transformPolicyClause(qual_pstate, copyObject(stmt->qual), ! EXPR_KIND_POLICY, "POLICY"); ! with_check_qual = transformPolicyClause(with_check_pstate, copyObject(stmt->with_check), ! EXPR_KIND_POLICY, "POLICY"); /* Fix up collation information */ *************** AlterPolicy(AlterPolicyStmt *stmt) *** 704,711 **** addRTEtoQuery(qual_pstate, rte, false, true, true); ! qual = transformWhereClause(qual_pstate, copyObject(stmt->qual), ! EXPR_KIND_WHERE, "POLICY"); /* Fix up collation information */ --- 704,711 ---- addRTEtoQuery(qual_pstate, rte, false, true, true); ! qual = transformPolicyClause(qual_pstate, copyObject(stmt->qual), ! EXPR_KIND_POLICY, "POLICY"); /* Fix up collation information */ *************** AlterPolicy(AlterPolicyStmt *stmt) *** 726,734 **** addRTEtoQuery(with_check_pstate, rte, false, true, true); ! with_check_qual = transformWhereClause(with_check_pstate, copyObject(stmt->with_check), ! EXPR_KIND_WHERE, "POLICY"); /* Fix up collation information */ --- 726,734 ---- addRTEtoQuery(with_check_pstate, rte, false, true, true); ! with_check_qual = transformPolicyClause(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..b79e73d 100644 *** a/src/backend/parser/parse_agg.c --- b/src/backend/parser/parse_agg.c *************** check_agglevels_and_constraints(ParseSta *** 373,378 **** --- 373,381 ---- case EXPR_KIND_WHERE: errkind = true; break; + case EXPR_KIND_POLICY: + errkind = true; + break; case EXPR_KIND_HAVING: /* okay */ break; *************** transformWindowFuncCall(ParseState *psta *** 770,775 **** --- 773,781 ---- case EXPR_KIND_WHERE: errkind = true; break; + case EXPR_KIND_POLICY: + errkind = true; + break; case EXPR_KIND_HAVING: errkind = true; break; diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index 5980856..7063e5f 100644 *** a/src/backend/parser/parse_clause.c --- b/src/backend/parser/parse_clause.c *************** transformWhereClause(ParseState *pstate, *** 1465,1470 **** --- 1465,1491 ---- return qual; } + /* + * transformPolicyClause - + * Transform USING and WITH CHECK expr to make sure it is of type boolean. + * + * constructName does not affect the semantics, but is used in error messages + */ + Node * + transformPolicyClause(ParseState *pstate, Node *clause, + ParseExprKind exprKind, const char *constructName) + { + Node *qual; + + if (clause == NULL) + return NULL; + + qual = transformExpr(pstate, clause, exprKind); + + qual = coerce_to_boolean(pstate, qual, constructName); + + return qual; + } /* * transformLimitClause - 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_clause.h b/src/include/parser/parse_clause.h index 77619e3..63e83e1 100644 *** a/src/include/parser/parse_clause.h --- b/src/include/parser/parse_clause.h *************** extern bool interpretOidsOption(List *de *** 24,29 **** --- 24,31 ---- extern Node *transformWhereClause(ParseState *pstate, Node *clause, ParseExprKind exprKind, const char *constructName); + extern Node *transformPolicyClause(ParseState *pstate, Node *clause, + ParseExprKind exprKind, const char *constructName); extern Node *transformLimitClause(ParseState *pstate, Node *clause, ParseExprKind exprKind, const char *constructName); extern List *transformGroupClause(ParseState *pstate, List *grouplist, 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/regress/expected/rowsecurity.out b/src/test/regress/expected/rowsecurity.out index b146da3..23e9a55 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 + 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