pgsql: RLS refactoring
RLS refactoring
This refactors rewrite/rowsecurity.c to simplify the handling of the
default deny case (reducing the number of places where we check for and
add the default deny policy from three to one) by splitting up the
retrival of the policies from the application of them.
This also allowed us to do away with the policy_id field. A policy_name
field was added for WithCheckOption policies and is used in error
reporting, when available.
Patch by Dean Rasheed, with various mostly cosmetic changes by me.
Back-patch to 9.5 where RLS was introduced to avoid unnecessary
differences, since we're still in alpha, per discussion with Robert.
Branch
------
master
Details
-------
http://git.postgresql.org/pg/commitdiff/22eaf35c1d247407b7cf1fffb310a26cd9b9ceb1
Modified Files
--------------
src/backend/commands/policy.c | 41 -
src/backend/executor/execMain.c | 20 +-
src/backend/nodes/copyfuncs.c | 1 +
src/backend/nodes/equalfuncs.c | 1 +
src/backend/nodes/outfuncs.c | 1 +
src/backend/nodes/readfuncs.c | 1 +
src/backend/rewrite/rewriteHandler.c | 5 +-
src/backend/rewrite/rowsecurity.c | 816 ++++++++++----------
src/backend/utils/cache/relcache.c | 2 -
src/include/nodes/parsenodes.h | 1 +
src/include/rewrite/rowsecurity.h | 3 +-
.../test_rls_hooks/expected/test_rls_hooks.out | 10 +-
src/test/modules/test_rls_hooks/test_rls_hooks.c | 2 -
13 files changed, 447 insertions(+), 457 deletions(-)
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Stephen Frost <sfrost@snowman.net> writes:
RLS refactoring
It looks to me like this changed the representation of stored rules, so it
should have included a catversion bump. This is particularly relevant to
the 9.5 branch where people already have alpha installations.
regards, tom lane
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
RLS refactoring
It looks to me like this changed the representation of stored rules, so it
should have included a catversion bump. This is particularly relevant to
the 9.5 branch where people already have alpha installations.
I had considererd if a bump was needed and figured it wasn't.
The WithCheckOption node which was changed doesn't ever end up in the
catalog, I don't believe; certainly not in pg_policy which just stores
the expressions which come from transformWhereClause, which haven't
changed.
I don't mind doing a bump if we feel it's necessary and maybe I'm
missing that there's a way to cause that node type to end up in the
catalog, but I don't think so, as we only ever build WithCheckOption
nodes in the rewriter.
Thanks!
Stephen
Stephen Frost wrote:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
It looks to me like this changed the representation of stored rules, so it
should have included a catversion bump. This is particularly relevant to
the 9.5 branch where people already have alpha installations.I had considererd if a bump was needed and figured it wasn't.
The WithCheckOption node which was changed doesn't ever end up in the
catalog, I don't believe; certainly not in pg_policy which just stores
the expressions which come from transformWhereClause, which haven't
changed.
Uhm, so why is it in readfuncs.c? If you create a view "WITH CHECK
OPTION", the pg_rewrite row says ":withCheckOptions <>". Does that not
change with your commit?
--
�lvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Stephen Frost <sfrost@snowman.net> writes:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
It looks to me like this changed the representation of stored rules, so it
should have included a catversion bump. This is particularly relevant to
the 9.5 branch where people already have alpha installations.
I had considererd if a bump was needed and figured it wasn't.
I don't mind doing a bump if we feel it's necessary and maybe I'm
missing that there's a way to cause that node type to end up in the
catalog, but I don't think so, as we only ever build WithCheckOption
nodes in the rewriter.
Oh, I see. In that case you should remove WithCheckOption from the set of
node types supported by readfuncs.c, both because it's dead code and to
clarify that the node is not meant to ever end up on disk.
(outfuncs.c support is useful for debugging though, so keep that.)
regards, tom lane
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
It looks to me like this changed the representation of stored rules, so it
should have included a catversion bump. This is particularly relevant to
the 9.5 branch where people already have alpha installations.I had considererd if a bump was needed and figured it wasn't.
I don't mind doing a bump if we feel it's necessary and maybe I'm
missing that there's a way to cause that node type to end up in the
catalog, but I don't think so, as we only ever build WithCheckOption
nodes in the rewriter.Oh, I see. In that case you should remove WithCheckOption from the set of
node types supported by readfuncs.c, both because it's dead code and to
clarify that the node is not meant to ever end up on disk.
Yeah, I was just thinking the same.
(outfuncs.c support is useful for debugging though, so keep that.)
Right, makes sense.
I should be able to get to that tomorrow afternoon, til then I'm pretty
tied up with PostgresOpen.
Thanks!
Stephen
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
Stephen Frost wrote:
The WithCheckOption node which was changed doesn't ever end up in the
catalog, I don't believe; certainly not in pg_policy which just stores
the expressions which come from transformWhereClause, which haven't
changed.
Uhm, so why is it in readfuncs.c? If you create a view "WITH CHECK
OPTION", the pg_rewrite row says ":withCheckOptions <>". Does that not
change with your commit?
That field would always be NIL in a query produced by the parser;
it's only ever filled by the rewriter. But if this is documented
anywhere, I couldn't find it, and the placement of the field in struct
Query seems designed to be as confusing as possible about that. I'd have
put it down near the end myself, and certainly have documented that it is
NOT the parse-time representation of a WITH CHECK OPTION clause. For that
matter I don't even find it to be named very well, because it's impossible
to avoid that impression with the name as-is. Perhaps something like
insertedCheckClauses would have been better.
regards, tom lane
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Alvaro Herrera <alvherre@2ndquadrant.com> writes:
Stephen Frost wrote:
The WithCheckOption node which was changed doesn't ever end up in the
catalog, I don't believe; certainly not in pg_policy which just stores
the expressions which come from transformWhereClause, which haven't
changed.Uhm, so why is it in readfuncs.c? If you create a view "WITH CHECK
OPTION", the pg_rewrite row says ":withCheckOptions <>". Does that not
change with your commit?That field would always be NIL in a query produced by the parser;
Right.
it's only ever filled by the rewriter. But if this is documented
anywhere, I couldn't find it, and the placement of the field in struct
Query seems designed to be as confusing as possible about that. I'd have
put it down near the end myself, and certainly have documented that it is
NOT the parse-time representation of a WITH CHECK OPTION clause. For that
matter I don't even find it to be named very well, because it's impossible
to avoid that impression with the name as-is. Perhaps something like
insertedCheckClauses would have been better.
Agreed. I will see about improving on that situation with at least
documentation changes. If we want to remove it completely then we'd
need to bump catversion.. Not against doing that if we want to though.
Might be better that way.
Thanks!
Stephen
Stephen Frost <sfrost@snowman.net> writes:
Agreed. I will see about improving on that situation with at least
documentation changes. If we want to remove it completely then we'd
need to bump catversion.. Not against doing that if we want to though.
Might be better that way.
readfuncs.c doesn't actually stop to verify that the field name in stored
rules is what it expects. So I believe (but you'd better check) that
renaming the field could be done without initdb. If we wanted to change
its placement, we'd need initdb --- unless we wanted to move it in the
struct definition but not in _outQuery/_readQuery, which I wouldn't do
in HEAD but it might be acceptable in back branches.
So the limiting factor here is not initdb but avoiding an API/ABI break
for extensions that look at struct Query. It's clearly too late to do any
such thing in 9.4, but we still could accept API breaks for 9.5 I think.
My vote would be to rename and reposition the field in HEAD and 9.5
and accept the corresponding initdb. We already forced an initdb
since alpha2, so it's basically free as far as testers are concerned,
and keeping 9.5 in sync with HEAD in this area seems like a really
good idea for awhile to come.
regards, tom lane
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
On 2015-09-15 19:16:06 -0400, Tom Lane wrote:
readfuncs.c doesn't actually stop to verify that the field name in stored
rules is what it expects.
This reminds me: Is there a reason for that? ISTM that adding checks for
the field names would make error messages about borked stored trees much
easier to understand?
Greetings,
Andres Freund
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Andres Freund <andres@anarazel.de> writes:
On 2015-09-15 19:16:06 -0400, Tom Lane wrote:
readfuncs.c doesn't actually stop to verify that the field name in stored
rules is what it expects.
This reminds me: Is there a reason for that? ISTM that adding checks for
the field names would make error messages about borked stored trees much
easier to understand?
Dunno, how often have you seen such an error message, and how much would
it help anyway? I'm not sure it'd be worth the code and cycles to check.
regards, tom lane
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
Tom, all,
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
My vote would be to rename and reposition the field in HEAD and 9.5
and accept the corresponding initdb. We already forced an initdb
since alpha2, so it's basically free as far as testers are concerned,
and keeping 9.5 in sync with HEAD in this area seems like a really
good idea for awhile to come.
Alright, attached is an attempt at doing these renames. I went a bit
farther since it seemed to make sense to me (at least at the time, I'm
wondering a bit about it now), so, please provide any thoughts or
feedback you have regarding these changes.
Thanks!
Stephen
Attachments:
wco-rename.v1.patchtext/x-diff; charset=us-asciiDownload
From 579dcfb80ffb922cabf10590219de221bb12267a Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Thu, 24 Sep 2015 10:21:42 -0400
Subject: [PATCH] Rename withCheckOptions to insertedCheckClauses
CHECK clauses which are generated from the WITH CHECK OPTION being
specified on a view or from RLS WITH CHECK policies are always inserted
by the rewriter and therefore don't ever need to be persisted out on
disk for a given query.
This modifies outfuncs to no longer output the rewriter-added nodes for
a Query or a ModifyTable, and readfuncs to no longer read them in.
Further, rename 'withCheckOptions' in Query and ModifyTable to
'insertedCheckClauses', ExecWithCheckOption to ExecCheckClauses,
ri_WithCheckOptions and ri_WithCheckOptionExprs to
ri_InsertedCheckClauses and ri_InsertedCheckExprs, respectively, all to
make it clear that these are the check clauses which were added by the
rewriter, not the actual WITH CHECK OPTION indication for a view (which
is stored in reloptions for the view) nor the WITH CHECK expressions for
a policy (which are stored in pg_policy).
Requires a catversion bump.
Per discussion with Tom and Alvaro.
Backpatch to 9.5 since we're still in alpha.
---
src/backend/executor/execMain.c | 16 +++++------
src/backend/executor/nodeModifyTable.c | 45 +++++++++++++++----------------
src/backend/nodes/copyfuncs.c | 4 +--
src/backend/nodes/equalfuncs.c | 2 +-
src/backend/nodes/nodeFuncs.c | 4 +--
src/backend/nodes/outfuncs.c | 2 --
src/backend/nodes/readfuncs.c | 21 ---------------
src/backend/optimizer/plan/createplan.c | 8 +++---
src/backend/optimizer/plan/planner.c | 42 +++++++++++++++++------------
src/backend/optimizer/plan/setrefs.c | 4 +--
src/backend/optimizer/prep/prepsecurity.c | 2 +-
src/backend/rewrite/rewriteHandler.c | 37 ++++++++++++-------------
src/backend/rewrite/rowsecurity.c | 28 +++++++++----------
src/include/executor/executor.h | 2 +-
src/include/nodes/execnodes.h | 8 +++---
src/include/nodes/parsenodes.h | 8 ++++--
src/include/nodes/plannodes.h | 2 +-
src/include/optimizer/planmain.h | 2 +-
src/include/rewrite/rowsecurity.h | 2 +-
19 files changed, 114 insertions(+), 125 deletions(-)
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 85ff46b..d344b09 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1738,17 +1738,17 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
}
/*
- * ExecWithCheckOptions -- check that tuple satisfies any WITH CHECK OPTIONs
+ * ExecCheckClauses -- check that tuple satisfies any inserted CHECK clauses,
* of the specified kind.
*
* Note that this needs to be called multiple times to ensure that all kinds of
- * WITH CHECK OPTIONs are handled (both those from views which have the WITH
- * CHECK OPTION set and from row level security policies). See ExecInsert()
- * and ExecUpdate().
+ * CHECK clauses are handled (both those from views which have the WITH CHECK
+ * OPTION set and from row level security policies). See ExecInsert() and
+ * ExecUpdate().
*/
void
-ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
- TupleTableSlot *slot, EState *estate)
+ExecCheckClauses(WCOKind kind, ResultRelInfo *resultRelInfo,
+ TupleTableSlot *slot, EState *estate)
{
Relation rel = resultRelInfo->ri_RelationDesc;
TupleDesc tupdesc = RelationGetDescr(rel);
@@ -1766,8 +1766,8 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
econtext->ecxt_scantuple = slot;
/* Check each of the constraints */
- forboth(l1, resultRelInfo->ri_WithCheckOptions,
- l2, resultRelInfo->ri_WithCheckOptionExprs)
+ forboth(l1, resultRelInfo->ri_InsertedCheckClauses,
+ l2, resultRelInfo->ri_InsertedCheckClauseExprs)
{
WithCheckOption *wco = (WithCheckOption *) lfirst(l1);
ExprState *wcoExpr = (ExprState *) lfirst(l2);
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 1ef76d0..a310299 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -321,12 +321,11 @@ ExecInsert(ModifyTableState *mtstate,
/*
* Check any RLS INSERT WITH CHECK policies
*
- * ExecWithCheckOptions() will skip any WCOs which are not of the kind
+ * ExecCheckClauses() will skip any WCOs which are not of the kind
* we are looking for at this point.
*/
- if (resultRelInfo->ri_WithCheckOptions != NIL)
- ExecWithCheckOptions(WCO_RLS_INSERT_CHECK,
- resultRelInfo, slot, estate);
+ if (resultRelInfo->ri_InsertedCheckClauses != NIL)
+ ExecCheckClauses(WCO_RLS_INSERT_CHECK, resultRelInfo, slot, estate);
/*
* Check the constraints of the tuple
@@ -479,14 +478,14 @@ ExecInsert(ModifyTableState *mtstate,
* violations per the SQL spec, so we do it after actually inserting the
* record into the heap and all indexes.
*
- * ExecWithCheckOptions will elog(ERROR) if a violation is found, so the
+ * ExecCheckClauses will elog(ERROR) if a violation is found, so the
* tuple will never be seen, if it violates the WITH CHECK OPTION.
*
- * ExecWithCheckOptions() will skip any WCOs which are not of the kind we
+ * ExecCheckClauses() will skip any WCOs which are not of the kind we
* are looking for at this point.
*/
- if (resultRelInfo->ri_WithCheckOptions != NIL)
- ExecWithCheckOptions(WCO_VIEW_CHECK, resultRelInfo, slot, estate);
+ if (resultRelInfo->ri_InsertedCheckClauses != NIL)
+ ExecCheckClauses(WCO_VIEW_CHECK, resultRelInfo, slot, estate);
/* Process RETURNING if present */
if (resultRelInfo->ri_projectReturning)
@@ -858,13 +857,12 @@ ExecUpdate(ItemPointer tupleid,
* triggers then trigger.c will have done heap_lock_tuple to lock the
* correct tuple, so there's no need to do them again.)
*
- * ExecWithCheckOptions() will skip any WCOs which are not of the kind
+ * ExecCheckClauses() will skip any WCOs which are not of the kind
* we are looking for at this point.
*/
lreplace:;
- if (resultRelInfo->ri_WithCheckOptions != NIL)
- ExecWithCheckOptions(WCO_RLS_UPDATE_CHECK,
- resultRelInfo, slot, estate);
+ if (resultRelInfo->ri_InsertedCheckClauses != NIL)
+ ExecCheckClauses(WCO_RLS_UPDATE_CHECK, resultRelInfo, slot, estate);
/*
* Check the constraints of the tuple
@@ -993,11 +991,11 @@ lreplace:;
* violations per the SQL spec, so we do it after actually updating the
* record in the heap and all indexes.
*
- * ExecWithCheckOptions() will skip any WCOs which are not of the kind we
+ * ExecCheckClauses() will skip any WCOs which are not of the kind we
* are looking for at this point.
*/
- if (resultRelInfo->ri_WithCheckOptions != NIL)
- ExecWithCheckOptions(WCO_VIEW_CHECK, resultRelInfo, slot, estate);
+ if (resultRelInfo->ri_InsertedCheckClauses != NIL)
+ ExecCheckClauses(WCO_VIEW_CHECK, resultRelInfo, slot, estate);
/* Process RETURNING if present */
if (resultRelInfo->ri_projectReturning)
@@ -1156,7 +1154,7 @@ ExecOnConflictUpdate(ModifyTableState *mtstate,
return true; /* done with the tuple */
}
- if (resultRelInfo->ri_WithCheckOptions != NIL)
+ if (resultRelInfo->ri_InsertedCheckClauses != NIL)
{
/*
* Check target's existing tuple against UPDATE-applicable USING
@@ -1173,9 +1171,9 @@ ExecOnConflictUpdate(ModifyTableState *mtstate,
* kinds, so there is no danger of spurious over-enforcement in the
* INSERT or UPDATE path.
*/
- ExecWithCheckOptions(WCO_RLS_CONFLICT_CHECK, resultRelInfo,
- mtstate->mt_existing,
- mtstate->ps.state);
+ ExecCheckClauses(WCO_RLS_CONFLICT_CHECK, resultRelInfo,
+ mtstate->mt_existing,
+ mtstate->ps.state);
}
/* Project the new tuple version */
@@ -1570,11 +1568,12 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
estate->es_result_relation_info = saved_resultRelInfo;
/*
- * Initialize any WITH CHECK OPTION constraints if needed.
+ * Initialize any inserted CHECK clauses (which would have come from
+ * view WITH CHECK OPTION constraints or RLS), if needed.
*/
resultRelInfo = mtstate->resultRelInfo;
i = 0;
- foreach(l, node->withCheckOptionLists)
+ foreach(l, node->insertedCheckClauses)
{
List *wcoList = (List *) lfirst(l);
List *wcoExprs = NIL;
@@ -1589,8 +1588,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
wcoExprs = lappend(wcoExprs, wcoExpr);
}
- resultRelInfo->ri_WithCheckOptions = wcoList;
- resultRelInfo->ri_WithCheckOptionExprs = wcoExprs;
+ resultRelInfo->ri_InsertedCheckClauses = wcoList;
+ resultRelInfo->ri_InsertedCheckClauseExprs = wcoExprs;
resultRelInfo++;
i++;
}
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 62355aa..3fbf21a 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -181,7 +181,6 @@ _copyModifyTable(const ModifyTable *from)
COPY_NODE_FIELD(resultRelations);
COPY_SCALAR_FIELD(resultRelIndex);
COPY_NODE_FIELD(plans);
- COPY_NODE_FIELD(withCheckOptionLists);
COPY_NODE_FIELD(returningLists);
COPY_NODE_FIELD(fdwPrivLists);
COPY_NODE_FIELD(rowMarks);
@@ -192,6 +191,7 @@ _copyModifyTable(const ModifyTable *from)
COPY_NODE_FIELD(onConflictWhere);
COPY_SCALAR_FIELD(exclRelRTI);
COPY_NODE_FIELD(exclRelTlist);
+ COPY_NODE_FIELD(insertedCheckClauses);
return newnode;
}
@@ -2702,7 +2702,6 @@ _copyQuery(const Query *from)
COPY_NODE_FIELD(rtable);
COPY_NODE_FIELD(jointree);
COPY_NODE_FIELD(targetList);
- COPY_NODE_FIELD(withCheckOptions);
COPY_NODE_FIELD(onConflict);
COPY_NODE_FIELD(returningList);
COPY_NODE_FIELD(groupClause);
@@ -2716,6 +2715,7 @@ _copyQuery(const Query *from)
COPY_NODE_FIELD(rowMarks);
COPY_NODE_FIELD(setOperations);
COPY_NODE_FIELD(constraintDeps);
+ COPY_NODE_FIELD(insertedCheckClauses);
return newnode;
}
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 8f16833..fd1a074 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -906,7 +906,6 @@ _equalQuery(const Query *a, const Query *b)
COMPARE_NODE_FIELD(rtable);
COMPARE_NODE_FIELD(jointree);
COMPARE_NODE_FIELD(targetList);
- COMPARE_NODE_FIELD(withCheckOptions);
COMPARE_NODE_FIELD(onConflict);
COMPARE_NODE_FIELD(returningList);
COMPARE_NODE_FIELD(groupClause);
@@ -920,6 +919,7 @@ _equalQuery(const Query *a, const Query *b)
COMPARE_NODE_FIELD(rowMarks);
COMPARE_NODE_FIELD(setOperations);
COMPARE_NODE_FIELD(constraintDeps);
+ COMPARE_NODE_FIELD(insertedCheckClauses);
return true;
}
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index a11cb9f..521af70 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -2047,7 +2047,7 @@ query_tree_walker(Query *query,
if (walker((Node *) query->targetList, context))
return true;
- if (walker((Node *) query->withCheckOptions, context))
+ if (walker((Node *) query->insertedCheckClauses, context))
return true;
if (walker((Node *) query->onConflict, context))
return true;
@@ -2853,7 +2853,7 @@ query_tree_mutator(Query *query,
}
MUTATE(query->targetList, query->targetList, List *);
- MUTATE(query->withCheckOptions, query->withCheckOptions, List *);
+ MUTATE(query->insertedCheckClauses, query->insertedCheckClauses, List *);
MUTATE(query->onConflict, query->onConflict, OnConflictExpr *);
MUTATE(query->returningList, query->returningList, List *);
MUTATE(query->jointree, query->jointree, FromExpr *);
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index c91273c..9a9855e 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -335,7 +335,6 @@ _outModifyTable(StringInfo str, const ModifyTable *node)
WRITE_NODE_FIELD(resultRelations);
WRITE_INT_FIELD(resultRelIndex);
WRITE_NODE_FIELD(plans);
- WRITE_NODE_FIELD(withCheckOptionLists);
WRITE_NODE_FIELD(returningLists);
WRITE_NODE_FIELD(fdwPrivLists);
WRITE_NODE_FIELD(rowMarks);
@@ -2385,7 +2384,6 @@ _outQuery(StringInfo str, const Query *node)
WRITE_NODE_FIELD(rtable);
WRITE_NODE_FIELD(jointree);
WRITE_NODE_FIELD(targetList);
- WRITE_NODE_FIELD(withCheckOptions);
WRITE_NODE_FIELD(onConflict);
WRITE_NODE_FIELD(returningList);
WRITE_NODE_FIELD(groupClause);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 08519ed..5c82c42 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -244,7 +244,6 @@ _readQuery(void)
READ_NODE_FIELD(rtable);
READ_NODE_FIELD(jointree);
READ_NODE_FIELD(targetList);
- READ_NODE_FIELD(withCheckOptions);
READ_NODE_FIELD(onConflict);
READ_NODE_FIELD(returningList);
READ_NODE_FIELD(groupClause);
@@ -292,23 +291,6 @@ _readDeclareCursorStmt(void)
}
/*
- * _readWithCheckOption
- */
-static WithCheckOption *
-_readWithCheckOption(void)
-{
- READ_LOCALS(WithCheckOption);
-
- READ_ENUM_FIELD(kind, WCOKind);
- READ_STRING_FIELD(relname);
- READ_STRING_FIELD(polname);
- READ_NODE_FIELD(qual);
- READ_BOOL_FIELD(cascaded);
-
- READ_DONE();
-}
-
-/*
* _readSortGroupClause
*/
static SortGroupClause *
@@ -1466,7 +1448,6 @@ _readModifyTable(void)
READ_NODE_FIELD(resultRelations);
READ_INT_FIELD(resultRelIndex);
READ_NODE_FIELD(plans);
- READ_NODE_FIELD(withCheckOptionLists);
READ_NODE_FIELD(returningLists);
READ_NODE_FIELD(fdwPrivLists);
READ_NODE_FIELD(rowMarks);
@@ -2180,8 +2161,6 @@ parseNodeString(void)
if (MATCH("QUERY", 5))
return_value = _readQuery();
- else if (MATCH("WITHCHECKOPTION", 15))
- return_value = _readWithCheckOption();
else if (MATCH("SORTGROUPCLAUSE", 15))
return_value = _readSortGroupClause();
else if (MATCH("GROUPINGSET", 11))
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 404c6f5..1fc52de 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -4959,7 +4959,7 @@ make_modifytable(PlannerInfo *root,
CmdType operation, bool canSetTag,
Index nominalRelation,
List *resultRelations, List *subplans,
- List *withCheckOptionLists, List *returningLists,
+ List *insertedCheckClauses, List *returningLists,
List *rowMarks, OnConflictExpr *onconflict, int epqParam)
{
ModifyTable *node = makeNode(ModifyTable);
@@ -4971,8 +4971,8 @@ make_modifytable(PlannerInfo *root,
int i;
Assert(list_length(resultRelations) == list_length(subplans));
- Assert(withCheckOptionLists == NIL ||
- list_length(resultRelations) == list_length(withCheckOptionLists));
+ Assert(insertedCheckClauses == NIL ||
+ list_length(resultRelations) == list_length(insertedCheckClauses));
Assert(returningLists == NIL ||
list_length(resultRelations) == list_length(returningLists));
@@ -5036,7 +5036,7 @@ make_modifytable(PlannerInfo *root,
node->exclRelRTI = onconflict->exclRelIndex;
node->exclRelTlist = onconflict->exclRelTlist;
}
- node->withCheckOptionLists = withCheckOptionLists;
+ node->insertedCheckClauses = insertedCheckClauses;
node->returningLists = returningLists;
node->rowMarks = rowMarks;
node->epqParam = epqParam;
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 06be922..f04b02e 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -378,7 +378,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
{
PlannerInfo *root;
Plan *plan;
- List *newWithCheckOptions;
+ List *newCheckClauses;
List *newHaving;
bool hasOuterJoins;
ListCell *l;
@@ -508,17 +508,21 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
preprocess_expression(root, (Node *) parse->targetList,
EXPRKIND_TARGET);
- newWithCheckOptions = NIL;
- foreach(l, parse->withCheckOptions)
+ /*
+ * Process any inserted CHECK clauses (built when WITH CHECK OPTION is
+ * specified or RLS is used).
+ */
+ newCheckClauses = NIL;
+ foreach(l, parse->insertedCheckClauses)
{
WithCheckOption *wco = (WithCheckOption *) lfirst(l);
wco->qual = preprocess_expression(root, wco->qual,
EXPRKIND_QUAL);
if (wco->qual != NULL)
- newWithCheckOptions = lappend(newWithCheckOptions, wco);
+ newCheckClauses = lappend(newCheckClauses, wco);
}
- parse->withCheckOptions = newWithCheckOptions;
+ parse->insertedCheckClauses = newCheckClauses;
parse->returningList = (List *)
preprocess_expression(root, (Node *) parse->returningList,
@@ -677,18 +681,19 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
/* If it's not SELECT, we need a ModifyTable node */
if (parse->commandType != CMD_SELECT)
{
- List *withCheckOptionLists;
+ List *newCheckClausesList;
List *returningLists;
List *rowMarks;
/*
- * Set up the WITH CHECK OPTION and RETURNING lists-of-lists, if
+ * Set up the inserted CHECK clauses (for when a view has WITH CHECK
+ * specified or RLS is being used), and RETURNING lists-of-lists, if
* needed.
*/
- if (parse->withCheckOptions)
- withCheckOptionLists = list_make1(parse->withCheckOptions);
+ if (parse->insertedCheckClauses)
+ newCheckClausesList = list_make1(parse->insertedCheckClauses);
else
- withCheckOptionLists = NIL;
+ newCheckClausesList = NIL;
if (parse->returningList)
returningLists = list_make1(parse->returningList);
@@ -711,7 +716,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
parse->resultRelation,
list_make1_int(parse->resultRelation),
list_make1(plan),
- withCheckOptionLists,
+ newCheckClausesList,
returningLists,
rowMarks,
parse->onConflict,
@@ -909,7 +914,7 @@ inheritance_planner(PlannerInfo *root)
RelOptInfo **save_rel_array = NULL;
List *subplans = NIL;
List *resultRelations = NIL;
- List *withCheckOptionLists = NIL;
+ List *insertedCheckClauses = NIL;
List *returningLists = NIL;
List *rowMarks;
ListCell *lc;
@@ -1228,10 +1233,13 @@ inheritance_planner(PlannerInfo *root)
/* Build list of target-relation RT indexes */
resultRelations = lappend_int(resultRelations, appinfo->child_relid);
- /* Build lists of per-relation WCO and RETURNING targetlists */
- if (parse->withCheckOptions)
- withCheckOptionLists = lappend(withCheckOptionLists,
- subroot.parse->withCheckOptions);
+ /*
+ * Build lists of per-relation CHECK clauses (from views which have
+ * WITH CHECK specified or if RLS is used) and RETURNING targetlists
+ */
+ if (parse->insertedCheckClauses)
+ insertedCheckClauses = lappend(insertedCheckClauses,
+ subroot.parse->insertedCheckClauses);
if (parse->returningList)
returningLists = lappend(returningLists,
subroot.parse->returningList);
@@ -1283,7 +1291,7 @@ inheritance_planner(PlannerInfo *root)
nominalRelation,
resultRelations,
subplans,
- withCheckOptionLists,
+ insertedCheckClauses,
returningLists,
rowMarks,
NULL,
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index daeb584..b33cc96 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -710,8 +710,8 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
Assert(splan->plan.targetlist == NIL);
Assert(splan->plan.qual == NIL);
- splan->withCheckOptionLists =
- fix_scan_list(root, splan->withCheckOptionLists, rtoffset);
+ splan->insertedCheckClauses =
+ fix_scan_list(root, splan->insertedCheckClauses, rtoffset);
if (splan->returningLists)
{
diff --git a/src/backend/optimizer/prep/prepsecurity.c b/src/backend/optimizer/prep/prepsecurity.c
index c4b61df..ba65ebb 100644
--- a/src/backend/optimizer/prep/prepsecurity.c
+++ b/src/backend/optimizer/prep/prepsecurity.c
@@ -138,7 +138,7 @@ expand_security_quals(PlannerInfo *root, List *tlist)
ChangeVarNodes((Node *) parse->returningList, rt_index,
parse->resultRelation, 0);
- ChangeVarNodes((Node *) parse->withCheckOptions, rt_index,
+ ChangeVarNodes((Node *) parse->insertedCheckClauses, rt_index,
parse->resultRelation, 0);
}
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 1b8e7b0..6867372 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -1770,7 +1770,7 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc);
Relation rel;
List *securityQuals;
- List *withCheckOptions;
+ List *insertedCheckClauses;
bool hasRowSecurity;
bool hasSubLinks;
@@ -1787,10 +1787,10 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
* Fetch any new security quals that must be applied to this RTE.
*/
get_row_security_policies(parsetree, rte, rt_index,
- &securityQuals, &withCheckOptions,
+ &securityQuals, &insertedCheckClauses,
&hasRowSecurity, &hasSubLinks);
- if (securityQuals != NIL || withCheckOptions != NIL)
+ if (securityQuals != NIL || insertedCheckClauses != NIL)
{
if (hasSubLinks)
{
@@ -1810,15 +1810,16 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
/*
* get_row_security_policies just passed back securityQuals
- * and/or withCheckOptions, and there were SubLinks, make sure
- * we lock any relations which are referenced.
+ * and/or insertedCheckClauses, and there were SubLinks, make
+ * sure we lock any relations which are referenced.
*
* These locks would normally be acquired by the parser, but
- * securityQuals and withCheckOptions are added post-parsing.
+ * securityQuals and insertedCheckClauses are added
+ * post-parsing.
*/
context.for_execute = true;
(void) acquireLocksOnSubLinks((Node *) securityQuals, &context);
- (void) acquireLocksOnSubLinks((Node *) withCheckOptions,
+ (void) acquireLocksOnSubLinks((Node *) insertedCheckClauses,
&context);
/*
@@ -1828,7 +1829,7 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
expression_tree_walker((Node *) securityQuals,
fireRIRonSubLink, (void *) activeRIRs);
- expression_tree_walker((Node *) withCheckOptions,
+ expression_tree_walker((Node *) insertedCheckClauses,
fireRIRonSubLink, (void *) activeRIRs);
activeRIRs = list_delete_first(activeRIRs);
@@ -1843,8 +1844,8 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
rte->securityQuals = list_concat(securityQuals,
rte->securityQuals);
- parsetree->withCheckOptions = list_concat(withCheckOptions,
- parsetree->withCheckOptions);
+ parsetree->insertedCheckClauses = list_concat(insertedCheckClauses,
+ parsetree->insertedCheckClauses);
}
/*
@@ -2983,7 +2984,7 @@ rewriteTargetView(Query *parsetree, Relation view)
/*
* For INSERT/UPDATE, if the view has the WITH CHECK OPTION, or any parent
* view specified WITH CASCADED CHECK OPTION, add the quals from the view
- * to the query's withCheckOptions list.
+ * to the query's insertedCheckClauses list.
*/
if (parsetree->commandType != CMD_DELETE)
{
@@ -2994,14 +2995,14 @@ rewriteTargetView(Query *parsetree, Relation view)
* If the parent view has a cascaded check option, treat this view as
* if it also had a cascaded check option.
*
- * New WithCheckOptions are added to the start of the list, so if
- * there is a cascaded check option, it will be the first item in the
- * list.
+ * New WithCheckOptions are added to the start of the
+ * insertedCheckClauses list, so if there is a cascaded check option,
+ * it will be the first item in the list.
*/
- if (parsetree->withCheckOptions != NIL)
+ if (parsetree->insertedCheckClauses != NIL)
{
WithCheckOption *parent_wco =
- (WithCheckOption *) linitial(parsetree->withCheckOptions);
+ (WithCheckOption *) linitial(parsetree->insertedCheckClauses);
if (parent_wco->cascaded)
{
@@ -3030,8 +3031,8 @@ rewriteTargetView(Query *parsetree, Relation view)
wco->qual = NULL;
wco->cascaded = cascaded;
- parsetree->withCheckOptions = lcons(wco,
- parsetree->withCheckOptions);
+ parsetree->insertedCheckClauses = lcons(wco,
+ parsetree->insertedCheckClauses);
if (viewquery->jointree->quals != NULL)
{
diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
index c20a178..42108af 100644
--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -77,7 +77,7 @@ static void add_with_check_options(Relation rel,
WCOKind kind,
List *permissive_policies,
List *restrictive_policies,
- List **withCheckOptions,
+ List **insertedCheckClauses,
bool *hasSubLinks);
static bool check_role_for_policy(ArrayType *policy_roles, Oid user_id);
@@ -104,7 +104,7 @@ row_security_policy_hook_type row_security_policy_hook_restrictive = NULL;
*/
void
get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
- List **securityQuals, List **withCheckOptions,
+ List **securityQuals, List **insertedCheckClauses,
bool *hasRowSecurity, bool *hasSubLinks)
{
Oid user_id;
@@ -116,7 +116,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
/* Defaults for the return values */
*securityQuals = NIL;
- *withCheckOptions = NIL;
+ *insertedCheckClauses = NIL;
*hasRowSecurity = false;
*hasSubLinks = false;
@@ -214,7 +214,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
}
/*
- * For INSERT and UPDATE, add withCheckOptions to verify that any new
+ * For INSERT and UPDATE, add insertedCheckClauses to verify that any new
* records added are consistent with the security policies. This will use
* each policy's WITH CHECK clause, or its USING clause if no explicit
* WITH CHECK clause is defined.
@@ -229,7 +229,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
WCO_RLS_INSERT_CHECK : WCO_RLS_UPDATE_CHECK,
permissive_policies,
restrictive_policies,
- withCheckOptions,
+ insertedCheckClauses,
hasSubLinks);
/*
@@ -257,7 +257,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
WCO_RLS_CONFLICT_CHECK,
conflict_permissive_policies,
conflict_restrictive_policies,
- withCheckOptions,
+ insertedCheckClauses,
hasSubLinks);
/*
@@ -276,7 +276,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
WCO_RLS_CONFLICT_CHECK,
conflict_returning_permissive_policies,
conflict_returning_restrictive_policies,
- withCheckOptions,
+ insertedCheckClauses,
hasSubLinks);
}
@@ -285,7 +285,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
WCO_RLS_UPDATE_CHECK,
conflict_permissive_policies,
conflict_restrictive_policies,
- withCheckOptions,
+ insertedCheckClauses,
hasSubLinks);
}
}
@@ -565,8 +565,8 @@ add_security_quals(int rt_index,
* a WCO_RLS_CONFLICT_CHECK WithCheckOption, which always uses the USING
* clauses from RLS policies.
*
- * New WCOs are added to withCheckOptions, and hasSubLinks is set to true if
- * any of the check clauses added contain sublink subqueries.
+ * New WCOs are added to insertedCheckClauses, and hasSubLinks is set to true
+ * if any of the check clauses added contain sublink subqueries.
*/
static void
add_with_check_options(Relation rel,
@@ -574,7 +574,7 @@ add_with_check_options(Relation rel,
WCOKind kind,
List *permissive_policies,
List *restrictive_policies,
- List **withCheckOptions,
+ List **insertedCheckClauses,
bool *hasSubLinks)
{
ListCell *item;
@@ -631,7 +631,7 @@ add_with_check_options(Relation rel,
ChangeVarNodes(wco->qual, 1, rt_index, 0);
- *withCheckOptions = lappend(*withCheckOptions, wco);
+ *insertedCheckClauses = lappend(*insertedCheckClauses, wco);
/*
* Now add WithCheckOptions for each of the restrictive policy clauses
@@ -657,7 +657,7 @@ add_with_check_options(Relation rel,
wco->qual = (Node *) qual;
wco->cascaded = false;
- *withCheckOptions = lappend(*withCheckOptions, wco);
+ *insertedCheckClauses = lappend(*insertedCheckClauses, wco);
*hasSubLinks |= policy->hassublinks;
}
}
@@ -679,7 +679,7 @@ add_with_check_options(Relation rel,
false, true);
wco->cascaded = false;
- *withCheckOptions = lappend(*withCheckOptions, wco);
+ *insertedCheckClauses = lappend(*insertedCheckClauses, wco);
}
}
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 226f905..2363b8a 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -193,7 +193,7 @@ extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids);
extern void ExecConstraints(ResultRelInfo *resultRelInfo,
TupleTableSlot *slot, EState *estate);
-extern void ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
+extern void ExecCheckClauses(WCOKind kind, ResultRelInfo *resultRelInfo,
TupleTableSlot *slot, EState *estate);
extern LockTupleMode ExecUpdateLockMode(EState *estate, ResultRelInfo *relinfo);
extern ExecRowMark *ExecFindRowMark(EState *estate, Index rti, bool missing_ok);
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 4ae2f3e..cbf4896 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -311,8 +311,8 @@ typedef struct JunkFilter
* TrigInstrument optional runtime measurements for triggers
* FdwRoutine FDW callback functions, if foreign table
* FdwState available to save private state of FDW
- * WithCheckOptions list of WithCheckOption's to be checked
- * WithCheckOptionExprs list of WithCheckOption expr states
+ * InsertedCheckClauses list of WithCheckOption's to be checked
+ * InsertedCheckClauseExprs list of WithCheckOption expr states
* ConstraintExprs array of constraint-checking expr states
* junkFilter for removing junk attributes from tuples
* projectReturning for computing a RETURNING list
@@ -334,8 +334,8 @@ typedef struct ResultRelInfo
Instrumentation *ri_TrigInstrument;
struct FdwRoutine *ri_FdwRoutine;
void *ri_FdwState;
- List *ri_WithCheckOptions;
- List *ri_WithCheckOptionExprs;
+ List *ri_InsertedCheckClauses;
+ List *ri_InsertedCheckClauseExprs;
List **ri_ConstraintExprs;
JunkFilter *ri_junkFilter;
ProjectionInfo *ri_projectReturning;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 770866a..fe8597e 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -130,8 +130,6 @@ typedef struct Query
List *targetList; /* target list (of TargetEntry) */
- List *withCheckOptions; /* a list of WithCheckOption's */
-
OnConflictExpr *onConflict; /* ON CONFLICT DO [NOTHING | UPDATE] */
List *returningList; /* return-values list (of TargetEntry) */
@@ -158,6 +156,12 @@ typedef struct Query
List *constraintDeps; /* a list of pg_constraint OIDs that the query
* depends on to be semantically valid */
+
+ List *insertedCheckClauses; /* a list of WithCheckOption's, which
+ * are added during rewrite, therefore
+ * not written out as part of Query by
+ * outfuncs or read in by readfuncs */
+
} Query;
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index cc259f1..6d03f8d 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -179,7 +179,6 @@ typedef struct ModifyTable
List *resultRelations; /* integer list of RT indexes */
int resultRelIndex; /* index of first resultRel in plan's list */
List *plans; /* plan(s) producing source data */
- List *withCheckOptionLists; /* per-target-table WCO lists */
List *returningLists; /* per-target-table RETURNING tlists */
List *fdwPrivLists; /* per-target-table FDW private data lists */
List *rowMarks; /* PlanRowMarks (non-locking only) */
@@ -190,6 +189,7 @@ typedef struct ModifyTable
Node *onConflictWhere; /* WHERE for ON CONFLICT UPDATE */
Index exclRelRTI; /* RTI of the EXCLUDED pseudo relation */
List *exclRelTlist; /* tlist of the EXCLUDED pseudo relation */
+ List *insertedCheckClauses; /* per-target-table WCO lists */
} ModifyTable;
/* ----------------
diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.h
index 52b077a..f8c9de0 100644
--- a/src/include/optimizer/planmain.h
+++ b/src/include/optimizer/planmain.h
@@ -86,7 +86,7 @@ extern ModifyTable *make_modifytable(PlannerInfo *root,
CmdType operation, bool canSetTag,
Index nominalRelation,
List *resultRelations, List *subplans,
- List *withCheckOptionLists, List *returningLists,
+ List *insertedCheckClauses, List *returningLists,
List *rowMarks, OnConflictExpr *onconflict, int epqParam);
extern bool is_projection_capable_plan(Plan *plan);
diff --git a/src/include/rewrite/rowsecurity.h b/src/include/rewrite/rowsecurity.h
index 4af244d..62823b7 100644
--- a/src/include/rewrite/rowsecurity.h
+++ b/src/include/rewrite/rowsecurity.h
@@ -42,7 +42,7 @@ extern PGDLLIMPORT row_security_policy_hook_type row_security_policy_hook_restri
extern void get_row_security_policies(Query *root,
RangeTblEntry *rte, int rt_index,
- List **securityQuals, List **withCheckOptions,
+ List **securityQuals, List **insertedCheckClauses,
bool *hasRowSecurity, bool *hasSubLinks);
#endif /* ROWSECURITY_H */
--
1.9.1
All,
and this totally should have gone to -hackers instead, sorry about that.
Please ignore the one to -committers, if possible.
Tom, all,
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
My vote would be to rename and reposition the field in HEAD and 9.5
and accept the corresponding initdb. We already forced an initdb
since alpha2, so it's basically free as far as testers are concerned,
and keeping 9.5 in sync with HEAD in this area seems like a really
good idea for awhile to come.
Alright, attached is an attempt at doing these renames. I went a bit
farther since it seemed to make sense to me (at least at the time, I'm
wondering a bit about it now), so, please provide any thoughts or
feedback you have regarding these changes.
Thanks!
Stephen
Attachments:
wco-rename.v1.patchtext/x-diff; charset=us-asciiDownload
From 579dcfb80ffb922cabf10590219de221bb12267a Mon Sep 17 00:00:00 2001
From: Stephen Frost <sfrost@snowman.net>
Date: Thu, 24 Sep 2015 10:21:42 -0400
Subject: [PATCH] Rename withCheckOptions to insertedCheckClauses
CHECK clauses which are generated from the WITH CHECK OPTION being
specified on a view or from RLS WITH CHECK policies are always inserted
by the rewriter and therefore don't ever need to be persisted out on
disk for a given query.
This modifies outfuncs to no longer output the rewriter-added nodes for
a Query or a ModifyTable, and readfuncs to no longer read them in.
Further, rename 'withCheckOptions' in Query and ModifyTable to
'insertedCheckClauses', ExecWithCheckOption to ExecCheckClauses,
ri_WithCheckOptions and ri_WithCheckOptionExprs to
ri_InsertedCheckClauses and ri_InsertedCheckExprs, respectively, all to
make it clear that these are the check clauses which were added by the
rewriter, not the actual WITH CHECK OPTION indication for a view (which
is stored in reloptions for the view) nor the WITH CHECK expressions for
a policy (which are stored in pg_policy).
Requires a catversion bump.
Per discussion with Tom and Alvaro.
Backpatch to 9.5 since we're still in alpha.
---
src/backend/executor/execMain.c | 16 +++++------
src/backend/executor/nodeModifyTable.c | 45 +++++++++++++++----------------
src/backend/nodes/copyfuncs.c | 4 +--
src/backend/nodes/equalfuncs.c | 2 +-
src/backend/nodes/nodeFuncs.c | 4 +--
src/backend/nodes/outfuncs.c | 2 --
src/backend/nodes/readfuncs.c | 21 ---------------
src/backend/optimizer/plan/createplan.c | 8 +++---
src/backend/optimizer/plan/planner.c | 42 +++++++++++++++++------------
src/backend/optimizer/plan/setrefs.c | 4 +--
src/backend/optimizer/prep/prepsecurity.c | 2 +-
src/backend/rewrite/rewriteHandler.c | 37 ++++++++++++-------------
src/backend/rewrite/rowsecurity.c | 28 +++++++++----------
src/include/executor/executor.h | 2 +-
src/include/nodes/execnodes.h | 8 +++---
src/include/nodes/parsenodes.h | 8 ++++--
src/include/nodes/plannodes.h | 2 +-
src/include/optimizer/planmain.h | 2 +-
src/include/rewrite/rowsecurity.h | 2 +-
19 files changed, 114 insertions(+), 125 deletions(-)
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 85ff46b..d344b09 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1738,17 +1738,17 @@ ExecConstraints(ResultRelInfo *resultRelInfo,
}
/*
- * ExecWithCheckOptions -- check that tuple satisfies any WITH CHECK OPTIONs
+ * ExecCheckClauses -- check that tuple satisfies any inserted CHECK clauses,
* of the specified kind.
*
* Note that this needs to be called multiple times to ensure that all kinds of
- * WITH CHECK OPTIONs are handled (both those from views which have the WITH
- * CHECK OPTION set and from row level security policies). See ExecInsert()
- * and ExecUpdate().
+ * CHECK clauses are handled (both those from views which have the WITH CHECK
+ * OPTION set and from row level security policies). See ExecInsert() and
+ * ExecUpdate().
*/
void
-ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
- TupleTableSlot *slot, EState *estate)
+ExecCheckClauses(WCOKind kind, ResultRelInfo *resultRelInfo,
+ TupleTableSlot *slot, EState *estate)
{
Relation rel = resultRelInfo->ri_RelationDesc;
TupleDesc tupdesc = RelationGetDescr(rel);
@@ -1766,8 +1766,8 @@ ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
econtext->ecxt_scantuple = slot;
/* Check each of the constraints */
- forboth(l1, resultRelInfo->ri_WithCheckOptions,
- l2, resultRelInfo->ri_WithCheckOptionExprs)
+ forboth(l1, resultRelInfo->ri_InsertedCheckClauses,
+ l2, resultRelInfo->ri_InsertedCheckClauseExprs)
{
WithCheckOption *wco = (WithCheckOption *) lfirst(l1);
ExprState *wcoExpr = (ExprState *) lfirst(l2);
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 1ef76d0..a310299 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -321,12 +321,11 @@ ExecInsert(ModifyTableState *mtstate,
/*
* Check any RLS INSERT WITH CHECK policies
*
- * ExecWithCheckOptions() will skip any WCOs which are not of the kind
+ * ExecCheckClauses() will skip any WCOs which are not of the kind
* we are looking for at this point.
*/
- if (resultRelInfo->ri_WithCheckOptions != NIL)
- ExecWithCheckOptions(WCO_RLS_INSERT_CHECK,
- resultRelInfo, slot, estate);
+ if (resultRelInfo->ri_InsertedCheckClauses != NIL)
+ ExecCheckClauses(WCO_RLS_INSERT_CHECK, resultRelInfo, slot, estate);
/*
* Check the constraints of the tuple
@@ -479,14 +478,14 @@ ExecInsert(ModifyTableState *mtstate,
* violations per the SQL spec, so we do it after actually inserting the
* record into the heap and all indexes.
*
- * ExecWithCheckOptions will elog(ERROR) if a violation is found, so the
+ * ExecCheckClauses will elog(ERROR) if a violation is found, so the
* tuple will never be seen, if it violates the WITH CHECK OPTION.
*
- * ExecWithCheckOptions() will skip any WCOs which are not of the kind we
+ * ExecCheckClauses() will skip any WCOs which are not of the kind we
* are looking for at this point.
*/
- if (resultRelInfo->ri_WithCheckOptions != NIL)
- ExecWithCheckOptions(WCO_VIEW_CHECK, resultRelInfo, slot, estate);
+ if (resultRelInfo->ri_InsertedCheckClauses != NIL)
+ ExecCheckClauses(WCO_VIEW_CHECK, resultRelInfo, slot, estate);
/* Process RETURNING if present */
if (resultRelInfo->ri_projectReturning)
@@ -858,13 +857,12 @@ ExecUpdate(ItemPointer tupleid,
* triggers then trigger.c will have done heap_lock_tuple to lock the
* correct tuple, so there's no need to do them again.)
*
- * ExecWithCheckOptions() will skip any WCOs which are not of the kind
+ * ExecCheckClauses() will skip any WCOs which are not of the kind
* we are looking for at this point.
*/
lreplace:;
- if (resultRelInfo->ri_WithCheckOptions != NIL)
- ExecWithCheckOptions(WCO_RLS_UPDATE_CHECK,
- resultRelInfo, slot, estate);
+ if (resultRelInfo->ri_InsertedCheckClauses != NIL)
+ ExecCheckClauses(WCO_RLS_UPDATE_CHECK, resultRelInfo, slot, estate);
/*
* Check the constraints of the tuple
@@ -993,11 +991,11 @@ lreplace:;
* violations per the SQL spec, so we do it after actually updating the
* record in the heap and all indexes.
*
- * ExecWithCheckOptions() will skip any WCOs which are not of the kind we
+ * ExecCheckClauses() will skip any WCOs which are not of the kind we
* are looking for at this point.
*/
- if (resultRelInfo->ri_WithCheckOptions != NIL)
- ExecWithCheckOptions(WCO_VIEW_CHECK, resultRelInfo, slot, estate);
+ if (resultRelInfo->ri_InsertedCheckClauses != NIL)
+ ExecCheckClauses(WCO_VIEW_CHECK, resultRelInfo, slot, estate);
/* Process RETURNING if present */
if (resultRelInfo->ri_projectReturning)
@@ -1156,7 +1154,7 @@ ExecOnConflictUpdate(ModifyTableState *mtstate,
return true; /* done with the tuple */
}
- if (resultRelInfo->ri_WithCheckOptions != NIL)
+ if (resultRelInfo->ri_InsertedCheckClauses != NIL)
{
/*
* Check target's existing tuple against UPDATE-applicable USING
@@ -1173,9 +1171,9 @@ ExecOnConflictUpdate(ModifyTableState *mtstate,
* kinds, so there is no danger of spurious over-enforcement in the
* INSERT or UPDATE path.
*/
- ExecWithCheckOptions(WCO_RLS_CONFLICT_CHECK, resultRelInfo,
- mtstate->mt_existing,
- mtstate->ps.state);
+ ExecCheckClauses(WCO_RLS_CONFLICT_CHECK, resultRelInfo,
+ mtstate->mt_existing,
+ mtstate->ps.state);
}
/* Project the new tuple version */
@@ -1570,11 +1568,12 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
estate->es_result_relation_info = saved_resultRelInfo;
/*
- * Initialize any WITH CHECK OPTION constraints if needed.
+ * Initialize any inserted CHECK clauses (which would have come from
+ * view WITH CHECK OPTION constraints or RLS), if needed.
*/
resultRelInfo = mtstate->resultRelInfo;
i = 0;
- foreach(l, node->withCheckOptionLists)
+ foreach(l, node->insertedCheckClauses)
{
List *wcoList = (List *) lfirst(l);
List *wcoExprs = NIL;
@@ -1589,8 +1588,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
wcoExprs = lappend(wcoExprs, wcoExpr);
}
- resultRelInfo->ri_WithCheckOptions = wcoList;
- resultRelInfo->ri_WithCheckOptionExprs = wcoExprs;
+ resultRelInfo->ri_InsertedCheckClauses = wcoList;
+ resultRelInfo->ri_InsertedCheckClauseExprs = wcoExprs;
resultRelInfo++;
i++;
}
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 62355aa..3fbf21a 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -181,7 +181,6 @@ _copyModifyTable(const ModifyTable *from)
COPY_NODE_FIELD(resultRelations);
COPY_SCALAR_FIELD(resultRelIndex);
COPY_NODE_FIELD(plans);
- COPY_NODE_FIELD(withCheckOptionLists);
COPY_NODE_FIELD(returningLists);
COPY_NODE_FIELD(fdwPrivLists);
COPY_NODE_FIELD(rowMarks);
@@ -192,6 +191,7 @@ _copyModifyTable(const ModifyTable *from)
COPY_NODE_FIELD(onConflictWhere);
COPY_SCALAR_FIELD(exclRelRTI);
COPY_NODE_FIELD(exclRelTlist);
+ COPY_NODE_FIELD(insertedCheckClauses);
return newnode;
}
@@ -2702,7 +2702,6 @@ _copyQuery(const Query *from)
COPY_NODE_FIELD(rtable);
COPY_NODE_FIELD(jointree);
COPY_NODE_FIELD(targetList);
- COPY_NODE_FIELD(withCheckOptions);
COPY_NODE_FIELD(onConflict);
COPY_NODE_FIELD(returningList);
COPY_NODE_FIELD(groupClause);
@@ -2716,6 +2715,7 @@ _copyQuery(const Query *from)
COPY_NODE_FIELD(rowMarks);
COPY_NODE_FIELD(setOperations);
COPY_NODE_FIELD(constraintDeps);
+ COPY_NODE_FIELD(insertedCheckClauses);
return newnode;
}
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 8f16833..fd1a074 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -906,7 +906,6 @@ _equalQuery(const Query *a, const Query *b)
COMPARE_NODE_FIELD(rtable);
COMPARE_NODE_FIELD(jointree);
COMPARE_NODE_FIELD(targetList);
- COMPARE_NODE_FIELD(withCheckOptions);
COMPARE_NODE_FIELD(onConflict);
COMPARE_NODE_FIELD(returningList);
COMPARE_NODE_FIELD(groupClause);
@@ -920,6 +919,7 @@ _equalQuery(const Query *a, const Query *b)
COMPARE_NODE_FIELD(rowMarks);
COMPARE_NODE_FIELD(setOperations);
COMPARE_NODE_FIELD(constraintDeps);
+ COMPARE_NODE_FIELD(insertedCheckClauses);
return true;
}
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index a11cb9f..521af70 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -2047,7 +2047,7 @@ query_tree_walker(Query *query,
if (walker((Node *) query->targetList, context))
return true;
- if (walker((Node *) query->withCheckOptions, context))
+ if (walker((Node *) query->insertedCheckClauses, context))
return true;
if (walker((Node *) query->onConflict, context))
return true;
@@ -2853,7 +2853,7 @@ query_tree_mutator(Query *query,
}
MUTATE(query->targetList, query->targetList, List *);
- MUTATE(query->withCheckOptions, query->withCheckOptions, List *);
+ MUTATE(query->insertedCheckClauses, query->insertedCheckClauses, List *);
MUTATE(query->onConflict, query->onConflict, OnConflictExpr *);
MUTATE(query->returningList, query->returningList, List *);
MUTATE(query->jointree, query->jointree, FromExpr *);
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index c91273c..9a9855e 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -335,7 +335,6 @@ _outModifyTable(StringInfo str, const ModifyTable *node)
WRITE_NODE_FIELD(resultRelations);
WRITE_INT_FIELD(resultRelIndex);
WRITE_NODE_FIELD(plans);
- WRITE_NODE_FIELD(withCheckOptionLists);
WRITE_NODE_FIELD(returningLists);
WRITE_NODE_FIELD(fdwPrivLists);
WRITE_NODE_FIELD(rowMarks);
@@ -2385,7 +2384,6 @@ _outQuery(StringInfo str, const Query *node)
WRITE_NODE_FIELD(rtable);
WRITE_NODE_FIELD(jointree);
WRITE_NODE_FIELD(targetList);
- WRITE_NODE_FIELD(withCheckOptions);
WRITE_NODE_FIELD(onConflict);
WRITE_NODE_FIELD(returningList);
WRITE_NODE_FIELD(groupClause);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 08519ed..5c82c42 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -244,7 +244,6 @@ _readQuery(void)
READ_NODE_FIELD(rtable);
READ_NODE_FIELD(jointree);
READ_NODE_FIELD(targetList);
- READ_NODE_FIELD(withCheckOptions);
READ_NODE_FIELD(onConflict);
READ_NODE_FIELD(returningList);
READ_NODE_FIELD(groupClause);
@@ -292,23 +291,6 @@ _readDeclareCursorStmt(void)
}
/*
- * _readWithCheckOption
- */
-static WithCheckOption *
-_readWithCheckOption(void)
-{
- READ_LOCALS(WithCheckOption);
-
- READ_ENUM_FIELD(kind, WCOKind);
- READ_STRING_FIELD(relname);
- READ_STRING_FIELD(polname);
- READ_NODE_FIELD(qual);
- READ_BOOL_FIELD(cascaded);
-
- READ_DONE();
-}
-
-/*
* _readSortGroupClause
*/
static SortGroupClause *
@@ -1466,7 +1448,6 @@ _readModifyTable(void)
READ_NODE_FIELD(resultRelations);
READ_INT_FIELD(resultRelIndex);
READ_NODE_FIELD(plans);
- READ_NODE_FIELD(withCheckOptionLists);
READ_NODE_FIELD(returningLists);
READ_NODE_FIELD(fdwPrivLists);
READ_NODE_FIELD(rowMarks);
@@ -2180,8 +2161,6 @@ parseNodeString(void)
if (MATCH("QUERY", 5))
return_value = _readQuery();
- else if (MATCH("WITHCHECKOPTION", 15))
- return_value = _readWithCheckOption();
else if (MATCH("SORTGROUPCLAUSE", 15))
return_value = _readSortGroupClause();
else if (MATCH("GROUPINGSET", 11))
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 404c6f5..1fc52de 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -4959,7 +4959,7 @@ make_modifytable(PlannerInfo *root,
CmdType operation, bool canSetTag,
Index nominalRelation,
List *resultRelations, List *subplans,
- List *withCheckOptionLists, List *returningLists,
+ List *insertedCheckClauses, List *returningLists,
List *rowMarks, OnConflictExpr *onconflict, int epqParam)
{
ModifyTable *node = makeNode(ModifyTable);
@@ -4971,8 +4971,8 @@ make_modifytable(PlannerInfo *root,
int i;
Assert(list_length(resultRelations) == list_length(subplans));
- Assert(withCheckOptionLists == NIL ||
- list_length(resultRelations) == list_length(withCheckOptionLists));
+ Assert(insertedCheckClauses == NIL ||
+ list_length(resultRelations) == list_length(insertedCheckClauses));
Assert(returningLists == NIL ||
list_length(resultRelations) == list_length(returningLists));
@@ -5036,7 +5036,7 @@ make_modifytable(PlannerInfo *root,
node->exclRelRTI = onconflict->exclRelIndex;
node->exclRelTlist = onconflict->exclRelTlist;
}
- node->withCheckOptionLists = withCheckOptionLists;
+ node->insertedCheckClauses = insertedCheckClauses;
node->returningLists = returningLists;
node->rowMarks = rowMarks;
node->epqParam = epqParam;
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 06be922..f04b02e 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -378,7 +378,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
{
PlannerInfo *root;
Plan *plan;
- List *newWithCheckOptions;
+ List *newCheckClauses;
List *newHaving;
bool hasOuterJoins;
ListCell *l;
@@ -508,17 +508,21 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
preprocess_expression(root, (Node *) parse->targetList,
EXPRKIND_TARGET);
- newWithCheckOptions = NIL;
- foreach(l, parse->withCheckOptions)
+ /*
+ * Process any inserted CHECK clauses (built when WITH CHECK OPTION is
+ * specified or RLS is used).
+ */
+ newCheckClauses = NIL;
+ foreach(l, parse->insertedCheckClauses)
{
WithCheckOption *wco = (WithCheckOption *) lfirst(l);
wco->qual = preprocess_expression(root, wco->qual,
EXPRKIND_QUAL);
if (wco->qual != NULL)
- newWithCheckOptions = lappend(newWithCheckOptions, wco);
+ newCheckClauses = lappend(newCheckClauses, wco);
}
- parse->withCheckOptions = newWithCheckOptions;
+ parse->insertedCheckClauses = newCheckClauses;
parse->returningList = (List *)
preprocess_expression(root, (Node *) parse->returningList,
@@ -677,18 +681,19 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
/* If it's not SELECT, we need a ModifyTable node */
if (parse->commandType != CMD_SELECT)
{
- List *withCheckOptionLists;
+ List *newCheckClausesList;
List *returningLists;
List *rowMarks;
/*
- * Set up the WITH CHECK OPTION and RETURNING lists-of-lists, if
+ * Set up the inserted CHECK clauses (for when a view has WITH CHECK
+ * specified or RLS is being used), and RETURNING lists-of-lists, if
* needed.
*/
- if (parse->withCheckOptions)
- withCheckOptionLists = list_make1(parse->withCheckOptions);
+ if (parse->insertedCheckClauses)
+ newCheckClausesList = list_make1(parse->insertedCheckClauses);
else
- withCheckOptionLists = NIL;
+ newCheckClausesList = NIL;
if (parse->returningList)
returningLists = list_make1(parse->returningList);
@@ -711,7 +716,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
parse->resultRelation,
list_make1_int(parse->resultRelation),
list_make1(plan),
- withCheckOptionLists,
+ newCheckClausesList,
returningLists,
rowMarks,
parse->onConflict,
@@ -909,7 +914,7 @@ inheritance_planner(PlannerInfo *root)
RelOptInfo **save_rel_array = NULL;
List *subplans = NIL;
List *resultRelations = NIL;
- List *withCheckOptionLists = NIL;
+ List *insertedCheckClauses = NIL;
List *returningLists = NIL;
List *rowMarks;
ListCell *lc;
@@ -1228,10 +1233,13 @@ inheritance_planner(PlannerInfo *root)
/* Build list of target-relation RT indexes */
resultRelations = lappend_int(resultRelations, appinfo->child_relid);
- /* Build lists of per-relation WCO and RETURNING targetlists */
- if (parse->withCheckOptions)
- withCheckOptionLists = lappend(withCheckOptionLists,
- subroot.parse->withCheckOptions);
+ /*
+ * Build lists of per-relation CHECK clauses (from views which have
+ * WITH CHECK specified or if RLS is used) and RETURNING targetlists
+ */
+ if (parse->insertedCheckClauses)
+ insertedCheckClauses = lappend(insertedCheckClauses,
+ subroot.parse->insertedCheckClauses);
if (parse->returningList)
returningLists = lappend(returningLists,
subroot.parse->returningList);
@@ -1283,7 +1291,7 @@ inheritance_planner(PlannerInfo *root)
nominalRelation,
resultRelations,
subplans,
- withCheckOptionLists,
+ insertedCheckClauses,
returningLists,
rowMarks,
NULL,
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index daeb584..b33cc96 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -710,8 +710,8 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
Assert(splan->plan.targetlist == NIL);
Assert(splan->plan.qual == NIL);
- splan->withCheckOptionLists =
- fix_scan_list(root, splan->withCheckOptionLists, rtoffset);
+ splan->insertedCheckClauses =
+ fix_scan_list(root, splan->insertedCheckClauses, rtoffset);
if (splan->returningLists)
{
diff --git a/src/backend/optimizer/prep/prepsecurity.c b/src/backend/optimizer/prep/prepsecurity.c
index c4b61df..ba65ebb 100644
--- a/src/backend/optimizer/prep/prepsecurity.c
+++ b/src/backend/optimizer/prep/prepsecurity.c
@@ -138,7 +138,7 @@ expand_security_quals(PlannerInfo *root, List *tlist)
ChangeVarNodes((Node *) parse->returningList, rt_index,
parse->resultRelation, 0);
- ChangeVarNodes((Node *) parse->withCheckOptions, rt_index,
+ ChangeVarNodes((Node *) parse->insertedCheckClauses, rt_index,
parse->resultRelation, 0);
}
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 1b8e7b0..6867372 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -1770,7 +1770,7 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc);
Relation rel;
List *securityQuals;
- List *withCheckOptions;
+ List *insertedCheckClauses;
bool hasRowSecurity;
bool hasSubLinks;
@@ -1787,10 +1787,10 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
* Fetch any new security quals that must be applied to this RTE.
*/
get_row_security_policies(parsetree, rte, rt_index,
- &securityQuals, &withCheckOptions,
+ &securityQuals, &insertedCheckClauses,
&hasRowSecurity, &hasSubLinks);
- if (securityQuals != NIL || withCheckOptions != NIL)
+ if (securityQuals != NIL || insertedCheckClauses != NIL)
{
if (hasSubLinks)
{
@@ -1810,15 +1810,16 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
/*
* get_row_security_policies just passed back securityQuals
- * and/or withCheckOptions, and there were SubLinks, make sure
- * we lock any relations which are referenced.
+ * and/or insertedCheckClauses, and there were SubLinks, make
+ * sure we lock any relations which are referenced.
*
* These locks would normally be acquired by the parser, but
- * securityQuals and withCheckOptions are added post-parsing.
+ * securityQuals and insertedCheckClauses are added
+ * post-parsing.
*/
context.for_execute = true;
(void) acquireLocksOnSubLinks((Node *) securityQuals, &context);
- (void) acquireLocksOnSubLinks((Node *) withCheckOptions,
+ (void) acquireLocksOnSubLinks((Node *) insertedCheckClauses,
&context);
/*
@@ -1828,7 +1829,7 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
expression_tree_walker((Node *) securityQuals,
fireRIRonSubLink, (void *) activeRIRs);
- expression_tree_walker((Node *) withCheckOptions,
+ expression_tree_walker((Node *) insertedCheckClauses,
fireRIRonSubLink, (void *) activeRIRs);
activeRIRs = list_delete_first(activeRIRs);
@@ -1843,8 +1844,8 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
rte->securityQuals = list_concat(securityQuals,
rte->securityQuals);
- parsetree->withCheckOptions = list_concat(withCheckOptions,
- parsetree->withCheckOptions);
+ parsetree->insertedCheckClauses = list_concat(insertedCheckClauses,
+ parsetree->insertedCheckClauses);
}
/*
@@ -2983,7 +2984,7 @@ rewriteTargetView(Query *parsetree, Relation view)
/*
* For INSERT/UPDATE, if the view has the WITH CHECK OPTION, or any parent
* view specified WITH CASCADED CHECK OPTION, add the quals from the view
- * to the query's withCheckOptions list.
+ * to the query's insertedCheckClauses list.
*/
if (parsetree->commandType != CMD_DELETE)
{
@@ -2994,14 +2995,14 @@ rewriteTargetView(Query *parsetree, Relation view)
* If the parent view has a cascaded check option, treat this view as
* if it also had a cascaded check option.
*
- * New WithCheckOptions are added to the start of the list, so if
- * there is a cascaded check option, it will be the first item in the
- * list.
+ * New WithCheckOptions are added to the start of the
+ * insertedCheckClauses list, so if there is a cascaded check option,
+ * it will be the first item in the list.
*/
- if (parsetree->withCheckOptions != NIL)
+ if (parsetree->insertedCheckClauses != NIL)
{
WithCheckOption *parent_wco =
- (WithCheckOption *) linitial(parsetree->withCheckOptions);
+ (WithCheckOption *) linitial(parsetree->insertedCheckClauses);
if (parent_wco->cascaded)
{
@@ -3030,8 +3031,8 @@ rewriteTargetView(Query *parsetree, Relation view)
wco->qual = NULL;
wco->cascaded = cascaded;
- parsetree->withCheckOptions = lcons(wco,
- parsetree->withCheckOptions);
+ parsetree->insertedCheckClauses = lcons(wco,
+ parsetree->insertedCheckClauses);
if (viewquery->jointree->quals != NULL)
{
diff --git a/src/backend/rewrite/rowsecurity.c b/src/backend/rewrite/rowsecurity.c
index c20a178..42108af 100644
--- a/src/backend/rewrite/rowsecurity.c
+++ b/src/backend/rewrite/rowsecurity.c
@@ -77,7 +77,7 @@ static void add_with_check_options(Relation rel,
WCOKind kind,
List *permissive_policies,
List *restrictive_policies,
- List **withCheckOptions,
+ List **insertedCheckClauses,
bool *hasSubLinks);
static bool check_role_for_policy(ArrayType *policy_roles, Oid user_id);
@@ -104,7 +104,7 @@ row_security_policy_hook_type row_security_policy_hook_restrictive = NULL;
*/
void
get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
- List **securityQuals, List **withCheckOptions,
+ List **securityQuals, List **insertedCheckClauses,
bool *hasRowSecurity, bool *hasSubLinks)
{
Oid user_id;
@@ -116,7 +116,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
/* Defaults for the return values */
*securityQuals = NIL;
- *withCheckOptions = NIL;
+ *insertedCheckClauses = NIL;
*hasRowSecurity = false;
*hasSubLinks = false;
@@ -214,7 +214,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
}
/*
- * For INSERT and UPDATE, add withCheckOptions to verify that any new
+ * For INSERT and UPDATE, add insertedCheckClauses to verify that any new
* records added are consistent with the security policies. This will use
* each policy's WITH CHECK clause, or its USING clause if no explicit
* WITH CHECK clause is defined.
@@ -229,7 +229,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
WCO_RLS_INSERT_CHECK : WCO_RLS_UPDATE_CHECK,
permissive_policies,
restrictive_policies,
- withCheckOptions,
+ insertedCheckClauses,
hasSubLinks);
/*
@@ -257,7 +257,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
WCO_RLS_CONFLICT_CHECK,
conflict_permissive_policies,
conflict_restrictive_policies,
- withCheckOptions,
+ insertedCheckClauses,
hasSubLinks);
/*
@@ -276,7 +276,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
WCO_RLS_CONFLICT_CHECK,
conflict_returning_permissive_policies,
conflict_returning_restrictive_policies,
- withCheckOptions,
+ insertedCheckClauses,
hasSubLinks);
}
@@ -285,7 +285,7 @@ get_row_security_policies(Query *root, RangeTblEntry *rte, int rt_index,
WCO_RLS_UPDATE_CHECK,
conflict_permissive_policies,
conflict_restrictive_policies,
- withCheckOptions,
+ insertedCheckClauses,
hasSubLinks);
}
}
@@ -565,8 +565,8 @@ add_security_quals(int rt_index,
* a WCO_RLS_CONFLICT_CHECK WithCheckOption, which always uses the USING
* clauses from RLS policies.
*
- * New WCOs are added to withCheckOptions, and hasSubLinks is set to true if
- * any of the check clauses added contain sublink subqueries.
+ * New WCOs are added to insertedCheckClauses, and hasSubLinks is set to true
+ * if any of the check clauses added contain sublink subqueries.
*/
static void
add_with_check_options(Relation rel,
@@ -574,7 +574,7 @@ add_with_check_options(Relation rel,
WCOKind kind,
List *permissive_policies,
List *restrictive_policies,
- List **withCheckOptions,
+ List **insertedCheckClauses,
bool *hasSubLinks)
{
ListCell *item;
@@ -631,7 +631,7 @@ add_with_check_options(Relation rel,
ChangeVarNodes(wco->qual, 1, rt_index, 0);
- *withCheckOptions = lappend(*withCheckOptions, wco);
+ *insertedCheckClauses = lappend(*insertedCheckClauses, wco);
/*
* Now add WithCheckOptions for each of the restrictive policy clauses
@@ -657,7 +657,7 @@ add_with_check_options(Relation rel,
wco->qual = (Node *) qual;
wco->cascaded = false;
- *withCheckOptions = lappend(*withCheckOptions, wco);
+ *insertedCheckClauses = lappend(*insertedCheckClauses, wco);
*hasSubLinks |= policy->hassublinks;
}
}
@@ -679,7 +679,7 @@ add_with_check_options(Relation rel,
false, true);
wco->cascaded = false;
- *withCheckOptions = lappend(*withCheckOptions, wco);
+ *insertedCheckClauses = lappend(*insertedCheckClauses, wco);
}
}
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 226f905..2363b8a 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -193,7 +193,7 @@ extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids);
extern void ExecConstraints(ResultRelInfo *resultRelInfo,
TupleTableSlot *slot, EState *estate);
-extern void ExecWithCheckOptions(WCOKind kind, ResultRelInfo *resultRelInfo,
+extern void ExecCheckClauses(WCOKind kind, ResultRelInfo *resultRelInfo,
TupleTableSlot *slot, EState *estate);
extern LockTupleMode ExecUpdateLockMode(EState *estate, ResultRelInfo *relinfo);
extern ExecRowMark *ExecFindRowMark(EState *estate, Index rti, bool missing_ok);
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 4ae2f3e..cbf4896 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -311,8 +311,8 @@ typedef struct JunkFilter
* TrigInstrument optional runtime measurements for triggers
* FdwRoutine FDW callback functions, if foreign table
* FdwState available to save private state of FDW
- * WithCheckOptions list of WithCheckOption's to be checked
- * WithCheckOptionExprs list of WithCheckOption expr states
+ * InsertedCheckClauses list of WithCheckOption's to be checked
+ * InsertedCheckClauseExprs list of WithCheckOption expr states
* ConstraintExprs array of constraint-checking expr states
* junkFilter for removing junk attributes from tuples
* projectReturning for computing a RETURNING list
@@ -334,8 +334,8 @@ typedef struct ResultRelInfo
Instrumentation *ri_TrigInstrument;
struct FdwRoutine *ri_FdwRoutine;
void *ri_FdwState;
- List *ri_WithCheckOptions;
- List *ri_WithCheckOptionExprs;
+ List *ri_InsertedCheckClauses;
+ List *ri_InsertedCheckClauseExprs;
List **ri_ConstraintExprs;
JunkFilter *ri_junkFilter;
ProjectionInfo *ri_projectReturning;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 770866a..fe8597e 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -130,8 +130,6 @@ typedef struct Query
List *targetList; /* target list (of TargetEntry) */
- List *withCheckOptions; /* a list of WithCheckOption's */
-
OnConflictExpr *onConflict; /* ON CONFLICT DO [NOTHING | UPDATE] */
List *returningList; /* return-values list (of TargetEntry) */
@@ -158,6 +156,12 @@ typedef struct Query
List *constraintDeps; /* a list of pg_constraint OIDs that the query
* depends on to be semantically valid */
+
+ List *insertedCheckClauses; /* a list of WithCheckOption's, which
+ * are added during rewrite, therefore
+ * not written out as part of Query by
+ * outfuncs or read in by readfuncs */
+
} Query;
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index cc259f1..6d03f8d 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -179,7 +179,6 @@ typedef struct ModifyTable
List *resultRelations; /* integer list of RT indexes */
int resultRelIndex; /* index of first resultRel in plan's list */
List *plans; /* plan(s) producing source data */
- List *withCheckOptionLists; /* per-target-table WCO lists */
List *returningLists; /* per-target-table RETURNING tlists */
List *fdwPrivLists; /* per-target-table FDW private data lists */
List *rowMarks; /* PlanRowMarks (non-locking only) */
@@ -190,6 +189,7 @@ typedef struct ModifyTable
Node *onConflictWhere; /* WHERE for ON CONFLICT UPDATE */
Index exclRelRTI; /* RTI of the EXCLUDED pseudo relation */
List *exclRelTlist; /* tlist of the EXCLUDED pseudo relation */
+ List *insertedCheckClauses; /* per-target-table WCO lists */
} ModifyTable;
/* ----------------
diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.h
index 52b077a..f8c9de0 100644
--- a/src/include/optimizer/planmain.h
+++ b/src/include/optimizer/planmain.h
@@ -86,7 +86,7 @@ extern ModifyTable *make_modifytable(PlannerInfo *root,
CmdType operation, bool canSetTag,
Index nominalRelation,
List *resultRelations, List *subplans,
- List *withCheckOptionLists, List *returningLists,
+ List *insertedCheckClauses, List *returningLists,
List *rowMarks, OnConflictExpr *onconflict, int epqParam);
extern bool is_projection_capable_plan(Plan *plan);
diff --git a/src/include/rewrite/rowsecurity.h b/src/include/rewrite/rowsecurity.h
index 4af244d..62823b7 100644
--- a/src/include/rewrite/rowsecurity.h
+++ b/src/include/rewrite/rowsecurity.h
@@ -42,7 +42,7 @@ extern PGDLLIMPORT row_security_policy_hook_type row_security_policy_hook_restri
extern void get_row_security_policies(Query *root,
RangeTblEntry *rte, int rt_index,
- List **securityQuals, List **withCheckOptions,
+ List **securityQuals, List **insertedCheckClauses,
bool *hasRowSecurity, bool *hasSubLinks);
#endif /* ROWSECURITY_H */
--
1.9.1
Stephen Frost <sfrost@snowman.net> writes:
Alright, attached is an attempt at doing these renames. I went a bit
farther since it seemed to make sense to me (at least at the time, I'm
wondering a bit about it now), so, please provide any thoughts or
feedback you have regarding these changes.
Further, rename 'withCheckOptions' in Query and ModifyTable to
'insertedCheckClauses', ExecWithCheckOption to ExecCheckClauses,
ri_WithCheckOptions and ri_WithCheckOptionExprs to
ri_InsertedCheckClauses and ri_InsertedCheckExprs, respectively, all to
make it clear that these are the check clauses which were added by the
rewriter, not the actual WITH CHECK OPTION indication for a view (which
is stored in reloptions for the view) nor the WITH CHECK expressions for
That commitlog entry doesn't seem to quite square with the patch,
wherein I see
- * WithCheckOptions list of WithCheckOption's to be checked
- * WithCheckOptionExprs list of WithCheckOption expr states
+ * InsertedCheckClauses list of WithCheckOption's to be checked
+ * InsertedCheckClauseExprs list of WithCheckOption expr states- List *ri_WithCheckOptions;
- List *ri_WithCheckOptionExprs;
+ List *ri_InsertedCheckClauses;
+ List *ri_InsertedCheckClauseExprs;The distinction between a "clause" and an "expr" is not very obvious,
and certainly most other places in the code use those terms pretty
interchangeably, so I find both the old and new names unclear here.
How about ri_InsertedCheckClauseStates instead for the second list?
And similarly if you're using "Expr" to mean ExprState anywhere else.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
I wrote:
- List *ri_WithCheckOptions; - List *ri_WithCheckOptionExprs; + List *ri_InsertedCheckClauses; + List *ri_InsertedCheckClauseExprs;
The distinction between a "clause" and an "expr" is not very obvious,
and certainly most other places in the code use those terms pretty
interchangeably, so I find both the old and new names unclear here.
How about ri_InsertedCheckClauseStates instead for the second list?
And similarly if you're using "Expr" to mean ExprState anywhere else.
Actually ... does struct ResultRelInfo need to carry the original WCO
clauses at all, rather than just the exprstate list? In most places
we do not store expr and exprstate lists in the same node in the first
place, so we can get away with using the same field name for corresponding
lists in plan and planstate nodes. That's why we don't already have a
convention like "fooStates" for such lists.
Another thought is that as long as these are lists specifically of
WithCheckOption nodes, and not arbitrary expressions, "clause" isn't an
especially good term for them; it implies generality that isn't there.
And CheckClauses invites confusion with, for example, CHECK clauses of
domain types. So maybe better names would be "ri_InsertedCheckOptions"
(and "ri_InsertedCheckOptionStates" if you still need that). Or maybe
"ri_InsertedWCOClauses" and "ri_InsertedWCOClauseStates". I'm less sure
about whether this is an improvement, though.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 24 September 2015 at 21:25, Tom Lane <tgl@sss.pgh.pa.us> wrote:
I wrote:
- List *ri_WithCheckOptions; - List *ri_WithCheckOptionExprs; + List *ri_InsertedCheckClauses; + List *ri_InsertedCheckClauseExprs;The distinction between a "clause" and an "expr" is not very obvious,
and certainly most other places in the code use those terms pretty
interchangeably, so I find both the old and new names unclear here.
How about ri_InsertedCheckClauseStates instead for the second list?
And similarly if you're using "Expr" to mean ExprState anywhere else.Actually ... does struct ResultRelInfo need to carry the original WCO
clauses at all, rather than just the exprstate list? In most places
we do not store expr and exprstate lists in the same node in the first
place, so we can get away with using the same field name for corresponding
lists in plan and planstate nodes. That's why we don't already have a
convention like "fooStates" for such lists.Another thought is that as long as these are lists specifically of
WithCheckOption nodes, and not arbitrary expressions, "clause" isn't an
especially good term for them; it implies generality that isn't there.
And CheckClauses invites confusion with, for example, CHECK clauses of
domain types. So maybe better names would be "ri_InsertedCheckOptions"
(and "ri_InsertedCheckOptionStates" if you still need that). Or maybe
"ri_InsertedWCOClauses" and "ri_InsertedWCOClauseStates". I'm less sure
about whether this is an improvement, though.
None of this renaming seems like an improvement to me.
ri_InsertedCheckClauses is misleading because they're not clauses,
they're WithCheckOption nodes carrying various pieces of information,
only one of which is the clause to check.
ri_InsertedCheckOptions is a bit better from that perspective, but it
still seems messy for the executor to carry the knowledge that these
checks were inserted by the rewriter. In the executor they're just
checks to be executed, and "WithCheck" reflects their original source
better than "InsertedCheck".
ri_InsertedCheckOptionStates is inconsistent with the names of the
other lists of expr states on that node -- ri_TrigWhenExprs and
ri_ConstraintExprs.
Also, these were added in 9.4, so introducing this many differences
between 9.4 and 9.5+ will make back-patching harder.
The original objection to the name WithCheckOptions was in relation to
the Query struct, and the fact that this field isn't the parsed
representation of WITH CHECK OPTION's on the query. I think that can
be cured with a suitable comment.
Regards,
Dean
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
None of this renaming seems like an improvement to me.
I have to admit that I'm not entirely sure it's improving things either.
Certainly, we shouldn't be dumping out the withCheckOptions node and
perhaps we should move its place in Query and add comments to it, but
I'm not sure about the other changes.
ri_InsertedCheckClauses is misleading because they're not clauses,
they're WithCheckOption nodes carrying various pieces of information,
only one of which is the clause to check.ri_InsertedCheckOptions is a bit better from that perspective, but it
still seems messy for the executor to carry the knowledge that these
checks were inserted by the rewriter. In the executor they're just
checks to be executed, and "WithCheck" reflects their original source
better than "InsertedCheck".
Yeah, the actual structure is 'WithCheckOption' and everything is pretty
consistent around that. One possibly confusing bit is that we have a
ViewCheckOption enum in ViewStmt called "withCheckOption". Perhaps
that's what we should change? Semes like it's either that or rename the
structure itself to NewTupleCheck (or similar) and rename everything to
go along with that instead.
Also, these were added in 9.4, so introducing this many differences
between 9.4 and 9.5+ will make back-patching harder.
That's certainly true, but we don't want current or future hackers to be
confused either.
The original objection to the name WithCheckOptions was in relation to
the Query struct, and the fact that this field isn't the parsed
representation of WITH CHECK OPTION's on the query. I think that can
be cured with a suitable comment.
There's also the issue that outfuncs is including that node when it
really shouldn't be. That can be fixed independnetly of this
discussion, of course.
Thanks!
Stephen
Stephen Frost <sfrost@snowman.net> writes:
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
Also, these were added in 9.4, so introducing this many differences
between 9.4 and 9.5+ will make back-patching harder.
That's certainly true, but we don't want current or future hackers to be
confused either.
Yes. I do not think that we should stick with badly chosen names just
because of back-patching concerns. By that argument, we should never
fix any erroneous comments either.
Whether these particular names are improvements is, of course, a fit
subject for debate. I have to agree that I don't feel like we've quite
hit on le mot juste yet.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
* Tom Lane (tgl@sss.pgh.pa.us) wrote:
Stephen Frost <sfrost@snowman.net> writes:
* Dean Rasheed (dean.a.rasheed@gmail.com) wrote:
Also, these were added in 9.4, so introducing this many differences
between 9.4 and 9.5+ will make back-patching harder.That's certainly true, but we don't want current or future hackers to be
confused either.Yes. I do not think that we should stick with badly chosen names just
because of back-patching concerns. By that argument, we should never
fix any erroneous comments either.Whether these particular names are improvements is, of course, a fit
subject for debate. I have to agree that I don't feel like we've quite
hit on le mot juste yet.
I've gone ahead and at least removed the withCheckOptions empty-list
from being written out as part of Query for 9.5 and HEAD, and bumped
catversion accordingly.
I came to realize that ModifyTable actually is planned to be used for
parallel query and therefore the list for that needs to stay, along with
support for reading the WithCheckOption node in.
Thanks!
Stephen