Thread: Writeable CTE patch
Hi,
Attached is the latest version of this patch.
I altered rewriting a bit (I've brought the problems with the previous
approach up a couple of times before) and this version should have the
expected output in all situations. This patch doesn't allow you to use
INSERT/UPDATE/DELETE as the top level statement, but you can get around
that by putting the desired top-level statement in a new CTE.
Since the last patch I also moved ExecOpenIndices to nodeModifyTable.c
because the top-level executor doesn't know which result relations are
opened for which operations.
One thing which has bothered me a while is that there is no clear option
for commandType when you have a multiple types of statements in a single
Query. In some places it'd help to know that there are multiple
different statements. This is now achieved by having hasWritableCtes
variable in PlannedStmt, but that doesn't help in places where you don't
have access to (or there isn't yet one) PlannedStmt, which has lead me
to think that we could have a CMD_MULTI or a similar value to mark these
Queries. I haven't taken the time to look at this in detail, but it's
something to think about.
Regards,
Marko Tiikkaja
diff --git a/doc/src/sgml/queries.sgml b/doc/src/sgml/queries.sgml
index b2741bc..3aa7da5 100644
--- a/doc/src/sgml/queries.sgml
+++ b/doc/src/sgml/queries.sgml
@@ -1499,7 +1499,7 @@ SELECT 3, 'three';
<synopsis>
SELECT <replaceable>select_list</replaceable> FROM <replaceable>table_expression</replaceable>
</synopsis>
- and can appear anywhere a <literal>SELECT</> can. For example, you can
+ and can appear anywhere a <literal>SELECT</literal> can. For example, you can
use it as part of a <literal>UNION</>, or attach a
<replaceable>sort_specification</replaceable> (<literal>ORDER BY</>,
<literal>LIMIT</>, and/or <literal>OFFSET</>) to it. <literal>VALUES</>
@@ -1529,10 +1529,11 @@ SELECT <replaceable>select_list</replaceable> FROM <replaceable>table_expression
</indexterm>
<para>
- <literal>WITH</> provides a way to write subqueries for use in a larger
- <literal>SELECT</> query. The subqueries can be thought of as defining
- temporary tables that exist just for this query. One use of this feature
- is to break down complicated queries into simpler parts. An example is:
+ <literal>WITH</> provides a way to write subqueries for use in a
+ larger query. The subqueries can be thought of as defining
+ temporary tables that exist just for this query. One use of this
+ feature is to break down complicated queries into simpler parts.
+ An example is:
<programlisting>
WITH regional_sales AS (
@@ -1560,6 +1561,30 @@ GROUP BY region, product;
</para>
<para>
+ A <literal>WITH</literal> clause can also have an
+ <literal>INSERT</literal>, <literal>UPDATE</literal> or
+ <literal>DELETE</literal> (each optionally with a
+ <literal>RETURNING</literal> clause) statement in it. The example below
+ moves rows from the main table, foo_log into a partition,
+ foo_log_200910.
+
+<programlisting>
+WITH rows AS (
+ DELETE FROM ONLY foo_log
+ WHERE
+ foo_date >= '2009-10-01' AND
+ foo_date < '2009-11-01'
+ RETURNING *
+ ), t AS (
+ INSERT INTO foo_log_200910
+ SELECT * FROM rows
+ )
+VALUES(true);
+</programlisting>
+
+ </para>
+
+ <para>
The optional <literal>RECURSIVE</> modifier changes <literal>WITH</>
from a mere syntactic convenience into a feature that accomplishes
things not otherwise possible in standard SQL. Using
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 8954693..3634d43 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -58,7 +58,7 @@ SELECT [ ALL | DISTINCT [ ON ( <replaceable class="parameter">expression</replac
<phrase>and <replaceable class="parameter">with_query</replaceable> is:</phrase>
- <replaceable class="parameter">with_query_name</replaceable> [ ( <replaceable
class="parameter">column_name</replaceable>[, ...] ) ] AS ( <replaceable class="parameter">select</replaceable> )
+ <replaceable class="parameter">with_query_name</replaceable> [ ( <replaceable
class="parameter">column_name</replaceable>[, ...] ) ] AS ( <replaceable class="parameter">select</replaceable> |
(<replaceableclass="parameter">insert</replaceable> | <replaceable class="parameter">update</replaceable> |
<replaceableclass="parameter">delete</replaceable> [ RETURNING...]))
TABLE { [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ] | <replaceable
class="parameter">with_query_name</replaceable>}
</synopsis>
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 9100dd9..78d2344 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2160,7 +2160,8 @@ CopyFrom(CopyState cstate)
heap_insert(cstate->rel, tuple, mycid, hi_options, bistate);
if (resultRelInfo->ri_NumIndices > 0)
- recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self),
+ recheckIndexes = ExecInsertIndexTuples(resultRelInfo,
+ slot, &(tuple->t_self),
estate, false);
/* AFTER ROW INSERT Triggers */
diff --git a/src/backend/commands/portalcmds.c b/src/backend/commands/portalcmds.c
index 9ca2e84..9e1f05d 100644
--- a/src/backend/commands/portalcmds.c
+++ b/src/backend/commands/portalcmds.c
@@ -48,6 +48,11 @@ PerformCursorOpen(PlannedStmt *stmt, ParamListInfo params,
Portal portal;
MemoryContext oldContext;
+ if (stmt->hasWritableCtes)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("Non-SELECT cursors are not implemented")));
+
if (cstmt == NULL || !IsA(cstmt, DeclareCursorStmt))
elog(ERROR, "PerformCursorOpen called for non-cursor query");
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 7f2e270..a91b49a 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -3088,7 +3088,7 @@ move_chain_tuple(Relation rel,
if (ec->resultRelInfo->ri_NumIndices > 0)
{
ExecStoreTuple(&newtup, ec->slot, InvalidBuffer, false);
- ExecInsertIndexTuples(ec->slot, &(newtup.t_self), ec->estate, true);
+ ExecInsertIndexTuples(ec->resultRelInfo, ec->slot, &(newtup.t_self), ec->estate, true);
ResetPerTupleExprContext(ec->estate);
}
}
@@ -3214,7 +3214,7 @@ move_plain_tuple(Relation rel,
if (ec->resultRelInfo->ri_NumIndices > 0)
{
ExecStoreTuple(&newtup, ec->slot, InvalidBuffer, false);
- ExecInsertIndexTuples(ec->slot, &(newtup.t_self), ec->estate, true);
+ ExecInsertIndexTuples(ec->resultRelInfo, ec->slot, &(newtup.t_self), ec->estate, true);
ResetPerTupleExprContext(ec->estate);
}
}
diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c
index edb6ae7..0908803 100644
--- a/src/backend/commands/view.c
+++ b/src/backend/commands/view.c
@@ -394,6 +394,7 @@ DefineView(ViewStmt *stmt, const char *queryString)
Query *viewParse;
Oid viewOid;
RangeVar *view;
+ ListCell *lc;
/*
* Run parse analysis to convert the raw parse tree to a Query. Note this
@@ -412,6 +413,18 @@ DefineView(ViewStmt *stmt, const char *queryString)
viewParse->commandType != CMD_SELECT)
elog(ERROR, "unexpected parse analysis result");
+ /* .. but it doesn't check for DML inside CTEs */
+ foreach(lc, viewParse->cteList)
+ {
+ CommonTableExpr *cte;
+
+ cte = (CommonTableExpr *) lfirst(lc);
+ if (((Query *) cte->ctequery)->commandType != CMD_SELECT)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("INSERT/UPDATE/DELETE inside a CTE not allowed in a view definition")));
+ }
+
/*
* If a list of column names was given, run through and insert these into
* the actual query tree. - thomas 2000-03-08
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 1383123..3247c5c 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -924,16 +924,6 @@ InitResultRelInfo(ResultRelInfo *resultRelInfo,
resultRelInfo->ri_ConstraintExprs = NULL;
resultRelInfo->ri_junkFilter = NULL;
resultRelInfo->ri_projectReturning = NULL;
-
- /*
- * If there are indices on the result relation, open them and save
- * descriptors in the result relation info, so that we can add new index
- * entries for the tuples we add/update. We need not do this for a
- * DELETE, however, since deletion doesn't affect indexes.
- */
- if (resultRelationDesc->rd_rel->relhasindex &&
- operation != CMD_DELETE)
- ExecOpenIndices(resultRelInfo);
}
/*
@@ -1174,6 +1164,88 @@ ExecutePlan(EState *estate,
*/
estate->es_direction = direction;
+ /* Process top-level CTEs in case they have writes inside */
+ if (estate->es_plannedstmt->hasWritableCtes)
+ {
+ ListCell *lc;
+
+ foreach(lc, estate->es_plannedstmt->planTree->initPlan)
+ {
+ SubPlan *sp;
+ int cte_param_id;
+ ParamExecData* prmdata;
+ CteScanState *leader;
+
+ sp = (SubPlan *) lfirst(lc);
+ if (sp->subLinkType != CTE_SUBLINK)
+ continue;
+
+ cte_param_id = linitial_int(sp->setParam);
+ prmdata = &(estate->es_param_exec_vals[cte_param_id]);
+ leader = (CteScanState *) DatumGetPointer(prmdata->value);
+
+
+ /*
+ * bump CID.
+ *
+ * We're currently relying on the fact that there can only be
+ * a SELECT or VALUES as the top-level statement.
+ *
+ * XXX we should probably update the snapshot a bit differently
+ */
+ CommandCounterIncrement();
+ estate->es_output_cid = GetCurrentCommandId(true);
+ estate->es_snapshot->curcid = estate->es_output_cid;
+
+ /*
+ * If there's no leader, the CTE isn't referenced anywhere
+ * so we can just go ahead and scan the plan
+ */
+ if (!leader)
+ {
+ TupleTableSlot *slot;
+ PlanState *ps = (PlanState *) list_nth(estate->es_subplanstates,
+ sp->plan_id - 1);
+
+ Assert(IsA(ps, ModifyTableState));
+
+ /*
+ * We might have a RETURNING here, which means that
+ * we have to loop until the plan returns NULL.
+ */
+ for (;;)
+ {
+ slot = ExecProcNode(ps);
+ if (TupIsNull(slot))
+ break;
+ }
+ }
+ else
+ {
+ TupleTableSlot* slot;
+ PlanState *ps = (PlanState *) list_nth(estate->es_subplanstates,
+ sp->plan_id - 1);
+
+ /* Regular CTE, ignore */
+ if (!IsA(ps, ModifyTableState))
+ continue;
+
+ /*
+ * Scan through the leader CTE so the RETURNING tuples are
+ * stored into the tuple store.
+ */
+ for (;;)
+ {
+ slot = ExecProcNode((PlanState *) leader);
+ if (TupIsNull(slot))
+ break;
+ }
+
+ ExecReScan((PlanState *) leader, NULL);
+ }
+ }
+ }
+
/*
* Loop until we've processed the proper number of tuples from the plan.
*/
@@ -1943,7 +2015,8 @@ EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree)
* ExecInitSubPlan expects to be able to find these entries.
* Some of the SubPlans might not be used in the part of the plan tree
* we intend to run, but since it's not easy to tell which, we just
- * initialize them all.
+ * initialize them all. However, we will never run ModifyTable nodes in
+ * EvalPlanQual() so don't initialize them.
*/
Assert(estate->es_subplanstates == NIL);
foreach(l, parentestate->es_plannedstmt->subplans)
@@ -1951,7 +2024,11 @@ EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree)
Plan *subplan = (Plan *) lfirst(l);
PlanState *subplanstate;
- subplanstate = ExecInitNode(subplan, estate, 0);
+ /* Don't initialize ModifyTable subplans. */
+ if (IsA(subplan, ModifyTable))
+ subplanstate = NULL;
+ else
+ subplanstate = ExecInitNode(subplan, estate, 0);
estate->es_subplanstates = lappend(estate->es_subplanstates,
subplanstate);
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index b968473..44f7a47 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -968,13 +968,13 @@ ExecCloseIndices(ResultRelInfo *resultRelInfo)
* ----------------------------------------------------------------
*/
List *
-ExecInsertIndexTuples(TupleTableSlot *slot,
+ExecInsertIndexTuples(ResultRelInfo* resultRelInfo,
+ TupleTableSlot *slot,
ItemPointer tupleid,
EState *estate,
bool is_vacuum_full)
{
List *result = NIL;
- ResultRelInfo *resultRelInfo;
int i;
int numIndices;
RelationPtr relationDescs;
@@ -987,7 +987,6 @@ ExecInsertIndexTuples(TupleTableSlot *slot,
/*
* Get information from the result relation info structure.
*/
- resultRelInfo = estate->es_result_relation_info;
numIndices = resultRelInfo->ri_NumIndices;
relationDescs = resultRelInfo->ri_IndexRelationDescs;
indexInfoArray = resultRelInfo->ri_IndexRelationInfo;
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index a4828ac..61f5026 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -158,12 +158,12 @@ ExecProcessReturning(ProjectionInfo *projectReturning,
* ----------------------------------------------------------------
*/
static TupleTableSlot *
-ExecInsert(TupleTableSlot *slot,
+ExecInsert(ResultRelInfo *resultRelInfo,
+ TupleTableSlot *slot,
TupleTableSlot *planSlot,
EState *estate)
{
HeapTuple tuple;
- ResultRelInfo *resultRelInfo;
Relation resultRelationDesc;
Oid newId;
List *recheckIndexes = NIL;
@@ -177,7 +177,6 @@ ExecInsert(TupleTableSlot *slot,
/*
* get information on the (current) result relation
*/
- resultRelInfo = estate->es_result_relation_info;
resultRelationDesc = resultRelInfo->ri_RelationDesc;
/*
@@ -247,7 +246,8 @@ ExecInsert(TupleTableSlot *slot,
* insert index entries for tuple
*/
if (resultRelInfo->ri_NumIndices > 0)
- recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self),
+ recheckIndexes = ExecInsertIndexTuples(resultRelInfo,
+ slot, &(tuple->t_self),
estate, false);
/* AFTER ROW INSERT Triggers */
@@ -271,12 +271,12 @@ ExecInsert(TupleTableSlot *slot,
* ----------------------------------------------------------------
*/
static TupleTableSlot *
-ExecDelete(ItemPointer tupleid,
+ExecDelete(ResultRelInfo *resultRelInfo,
+ ItemPointer tupleid,
TupleTableSlot *planSlot,
EPQState *epqstate,
EState *estate)
{
- ResultRelInfo *resultRelInfo;
Relation resultRelationDesc;
HTSU_Result result;
ItemPointerData update_ctid;
@@ -285,7 +285,6 @@ ExecDelete(ItemPointer tupleid,
/*
* get information on the (current) result relation
*/
- resultRelInfo = estate->es_result_relation_info;
resultRelationDesc = resultRelInfo->ri_RelationDesc;
/* BEFORE ROW DELETE Triggers */
@@ -414,14 +413,14 @@ ldelete:;
* ----------------------------------------------------------------
*/
static TupleTableSlot *
-ExecUpdate(ItemPointer tupleid,
+ExecUpdate(ResultRelInfo *resultRelInfo,
+ ItemPointer tupleid,
TupleTableSlot *slot,
TupleTableSlot *planSlot,
EPQState *epqstate,
EState *estate)
{
HeapTuple tuple;
- ResultRelInfo *resultRelInfo;
Relation resultRelationDesc;
HTSU_Result result;
ItemPointerData update_ctid;
@@ -443,7 +442,6 @@ ExecUpdate(ItemPointer tupleid,
/*
* get information on the (current) result relation
*/
- resultRelInfo = estate->es_result_relation_info;
resultRelationDesc = resultRelInfo->ri_RelationDesc;
/* BEFORE ROW UPDATE Triggers */
@@ -561,7 +559,8 @@ lreplace:;
* If it's a HOT update, we mustn't insert new index entries.
*/
if (resultRelInfo->ri_NumIndices > 0 && !HeapTupleIsHeapOnly(tuple))
- recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self),
+ recheckIndexes = ExecInsertIndexTuples(resultRelInfo,
+ slot, &(tuple->t_self),
estate, false);
/* AFTER ROW UPDATE Triggers */
@@ -587,15 +586,15 @@ fireBSTriggers(ModifyTableState *node)
{
case CMD_INSERT:
ExecBSInsertTriggers(node->ps.state,
- node->ps.state->es_result_relations);
+ node->resultRelInfo);
break;
case CMD_UPDATE:
ExecBSUpdateTriggers(node->ps.state,
- node->ps.state->es_result_relations);
+ node->resultRelInfo);
break;
case CMD_DELETE:
ExecBSDeleteTriggers(node->ps.state,
- node->ps.state->es_result_relations);
+ node->resultRelInfo);
break;
default:
elog(ERROR, "unknown operation");
@@ -613,15 +612,15 @@ fireASTriggers(ModifyTableState *node)
{
case CMD_INSERT:
ExecASInsertTriggers(node->ps.state,
- node->ps.state->es_result_relations);
+ node->resultRelInfo);
break;
case CMD_UPDATE:
ExecASUpdateTriggers(node->ps.state,
- node->ps.state->es_result_relations);
+ node->resultRelInfo);
break;
case CMD_DELETE:
ExecASDeleteTriggers(node->ps.state,
- node->ps.state->es_result_relations);
+ node->resultRelInfo);
break;
default:
elog(ERROR, "unknown operation");
@@ -643,6 +642,7 @@ ExecModifyTable(ModifyTableState *node)
EState *estate = node->ps.state;
CmdType operation = node->operation;
PlanState *subplanstate;
+ ResultRelInfo *resultRelInfo;
JunkFilter *junkfilter;
TupleTableSlot *slot;
TupleTableSlot *planSlot;
@@ -658,17 +658,10 @@ ExecModifyTable(ModifyTableState *node)
node->fireBSTriggers = false;
}
- /*
- * es_result_relation_info must point to the currently active result
- * relation. (Note we assume that ModifyTable nodes can't be nested.)
- * We want it to be NULL whenever we're not within ModifyTable, though.
- */
- estate->es_result_relation_info =
- estate->es_result_relations + node->mt_whichplan;
-
/* Preload local variables */
subplanstate = node->mt_plans[node->mt_whichplan];
- junkfilter = estate->es_result_relation_info->ri_junkFilter;
+ resultRelInfo = node->resultRelInfo + node->mt_whichplan;
+ junkfilter = resultRelInfo->ri_junkFilter;
/*
* Fetch rows from subplan(s), and execute the required table modification
@@ -684,9 +677,9 @@ ExecModifyTable(ModifyTableState *node)
node->mt_whichplan++;
if (node->mt_whichplan < node->mt_nplans)
{
- estate->es_result_relation_info++;
subplanstate = node->mt_plans[node->mt_whichplan];
- junkfilter = estate->es_result_relation_info->ri_junkFilter;
+ resultRelInfo = node->resultRelInfo + node->mt_whichplan;
+ junkfilter = resultRelInfo->ri_junkFilter;
EvalPlanQualSetPlan(&node->mt_epqstate, subplanstate->plan);
continue;
}
@@ -728,14 +721,17 @@ ExecModifyTable(ModifyTableState *node)
switch (operation)
{
case CMD_INSERT:
- slot = ExecInsert(slot, planSlot, estate);
+ slot = ExecInsert(resultRelInfo,
+ slot, planSlot, estate);
break;
case CMD_UPDATE:
- slot = ExecUpdate(tupleid, slot, planSlot,
+ slot = ExecUpdate(resultRelInfo,
+ tupleid, slot, planSlot,
&node->mt_epqstate, estate);
break;
case CMD_DELETE:
- slot = ExecDelete(tupleid, planSlot,
+ slot = ExecDelete(resultRelInfo,
+ tupleid, planSlot,
&node->mt_epqstate, estate);
break;
default:
@@ -748,15 +744,9 @@ ExecModifyTable(ModifyTableState *node)
* the work on next call.
*/
if (slot)
- {
- estate->es_result_relation_info = NULL;
return slot;
- }
}
- /* Reset es_result_relation_info before exiting */
- estate->es_result_relation_info = NULL;
-
/*
* We're done, but fire AFTER STATEMENT triggers before exiting.
*/
@@ -803,25 +793,39 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
mtstate->mt_plans = (PlanState **) palloc0(sizeof(PlanState *) * nplans);
mtstate->mt_nplans = nplans;
mtstate->operation = operation;
+ mtstate->resultRelIndex = node->resultRelIndex;
+ mtstate->resultRelInfo = estate->es_result_relations + node->resultRelIndex;
+
/* set up epqstate with dummy subplan pointer for the moment */
EvalPlanQualInit(&mtstate->mt_epqstate, estate, NULL, node->epqParam);
mtstate->fireBSTriggers = true;
- /* For the moment, assume our targets are exactly the global result rels */
-
/*
* call ExecInitNode on each of the plans to be executed and save the
* results into the array "mt_plans". Note we *must* set
* estate->es_result_relation_info correctly while we initialize each
* sub-plan; ExecContextForcesOids depends on that!
*/
- estate->es_result_relation_info = estate->es_result_relations;
i = 0;
+ resultRelInfo = mtstate->resultRelInfo;
foreach(l, node->plans)
{
subplan = (Plan *) lfirst(l);
+
+ /*
+ * If there are indices on the result relation, open them and save
+ * descriptors in the result relation info, so that we can add new index
+ * entries for the tuples we add/update. We need not do this for a
+ * DELETE, however, since deletion doesn't affect indexes.
+ */
+ if (resultRelInfo->ri_RelationDesc->rd_rel->relhasindex &&
+ operation != CMD_DELETE)
+ ExecOpenIndices(resultRelInfo);
+
+ estate->es_result_relation_info = resultRelInfo;
mtstate->mt_plans[i] = ExecInitNode(subplan, estate, eflags);
- estate->es_result_relation_info++;
+
+ resultRelInfo++;
i++;
}
estate->es_result_relation_info = NULL;
@@ -858,8 +862,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
/*
* Build a projection for each result rel.
*/
- Assert(list_length(node->returningLists) == estate->es_num_result_relations);
- resultRelInfo = estate->es_result_relations;
+ resultRelInfo = mtstate->resultRelInfo;
foreach(l, node->returningLists)
{
List *rlist = (List *) lfirst(l);
@@ -958,7 +961,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
if (junk_filter_needed)
{
- resultRelInfo = estate->es_result_relations;
+ resultRelInfo = mtstate->resultRelInfo;
for (i = 0; i < nplans; i++)
{
JunkFilter *j;
@@ -987,7 +990,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
else
{
if (operation == CMD_INSERT)
- ExecCheckPlanOutput(estate->es_result_relations->ri_RelationDesc,
+ ExecCheckPlanOutput(mtstate->resultRelInfo->ri_RelationDesc,
subplan->targetlist);
}
}
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index a30e685..bf63570 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -84,6 +84,7 @@ _copyPlannedStmt(PlannedStmt *from)
COPY_NODE_FIELD(resultRelations);
COPY_NODE_FIELD(utilityStmt);
COPY_NODE_FIELD(intoClause);
+ COPY_SCALAR_FIELD(hasWritableCtes);
COPY_NODE_FIELD(subplans);
COPY_BITMAPSET_FIELD(rewindPlanIDs);
COPY_NODE_FIELD(rowMarks);
@@ -172,6 +173,7 @@ _copyModifyTable(ModifyTable *from)
*/
COPY_SCALAR_FIELD(operation);
COPY_NODE_FIELD(resultRelations);
+ COPY_SCALAR_FIELD(resultRelIndex);
COPY_NODE_FIELD(plans);
COPY_NODE_FIELD(returningLists);
COPY_NODE_FIELD(rowMarks);
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index b40db0b..5412e01 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -2383,6 +2383,50 @@ bool
return true;
}
break;
+ case T_InsertStmt:
+ {
+ InsertStmt *stmt = (InsertStmt *) node;
+
+ if (walker(stmt->relation, context))
+ return true;
+ if (walker(stmt->cols, context))
+ return true;
+ if (walker(stmt->selectStmt, context))
+ return true;
+ if (walker(stmt->returningList, context))
+ return true;
+ }
+ break;
+ case T_UpdateStmt:
+ {
+ UpdateStmt *stmt = (UpdateStmt *) node;
+
+ if (walker(stmt->relation, context))
+ return true;
+ if (walker(stmt->targetList, context))
+ return true;
+ if (walker(stmt->whereClause, context))
+ return true;
+ if (walker(stmt->fromClause, context))
+ return true;
+ if (walker(stmt->returningList, context))
+ return true;
+ }
+ break;
+ case T_DeleteStmt:
+ {
+ DeleteStmt *stmt = (DeleteStmt *) node;
+
+ if (walker(stmt->relation, context))
+ return true;
+ if (walker(stmt->usingClause, context))
+ return true;
+ if (walker(stmt->whereClause, context))
+ return true;
+ if (walker(stmt->returningList, context))
+ return true;
+ }
+ break;
case T_A_Expr:
{
A_Expr *expr = (A_Expr *) node;
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 7a85f92..b981a38 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -327,6 +327,7 @@ _outModifyTable(StringInfo str, ModifyTable *node)
WRITE_ENUM_FIELD(operation, CmdType);
WRITE_NODE_FIELD(resultRelations);
+ WRITE_INT_FIELD(resultRelIndex);
WRITE_NODE_FIELD(plans);
WRITE_NODE_FIELD(returningLists);
WRITE_NODE_FIELD(rowMarks);
@@ -1529,6 +1530,7 @@ _outPlannerGlobal(StringInfo str, PlannerGlobal *node)
WRITE_NODE_FIELD(finalrowmarks);
WRITE_NODE_FIELD(relationOids);
WRITE_NODE_FIELD(invalItems);
+ WRITE_NODE_FIELD(resultRelations);
WRITE_UINT_FIELD(lastPHId);
WRITE_BOOL_FIELD(transientPlan);
}
@@ -1543,7 +1545,6 @@ _outPlannerInfo(StringInfo str, PlannerInfo *node)
WRITE_NODE_FIELD(glob);
WRITE_UINT_FIELD(query_level);
WRITE_NODE_FIELD(join_rel_list);
- WRITE_NODE_FIELD(resultRelations);
WRITE_NODE_FIELD(init_plans);
WRITE_NODE_FIELD(cte_plan_ids);
WRITE_NODE_FIELD(eq_classes);
diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c
index 331963f..4402092 100644
--- a/src/backend/optimizer/plan/createplan.c
+++ b/src/backend/optimizer/plan/createplan.c
@@ -3759,10 +3759,6 @@ make_modifytable(CmdType operation, List *resultRelations,
double total_size;
ListCell *subnode;
- Assert(list_length(resultRelations) == list_length(subplans));
- Assert(returningLists == NIL ||
- list_length(resultRelations) == list_length(returningLists));
-
/*
* Compute cost as sum of subplan costs.
*/
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 7f2f0c6..b352956 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -160,6 +160,8 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
glob->finalrowmarks = NIL;
glob->relationOids = NIL;
glob->invalItems = NIL;
+ glob->hasWritableCtes = false;
+ glob->resultRelations = NIL;
glob->lastPHId = 0;
glob->transientPlan = false;
@@ -237,9 +239,10 @@ standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
result->transientPlan = glob->transientPlan;
result->planTree = top_plan;
result->rtable = glob->finalrtable;
- result->resultRelations = root->resultRelations;
+ result->resultRelations = glob->resultRelations;
result->utilityStmt = parse->utilityStmt;
result->intoClause = parse->intoClause;
+ result->hasWritableCtes = glob->hasWritableCtes;
result->subplans = glob->subplans;
result->rewindPlanIDs = glob->rewindPlanIDs;
result->rowMarks = glob->finalrowmarks;
@@ -541,7 +544,7 @@ subquery_planner(PlannerGlobal *glob, Query *parse,
rowMarks = root->rowMarks;
plan = (Plan *) make_modifytable(parse->commandType,
- copyObject(root->resultRelations),
+ list_make1_int(parse->resultRelation),
list_make1(plan),
returningLists,
rowMarks,
@@ -706,13 +709,13 @@ inheritance_planner(PlannerInfo *root)
Query *parse = root->parse;
int parentRTindex = parse->resultRelation;
List *subplans = NIL;
- List *resultRelations = NIL;
List *returningLists = NIL;
List *rtable = NIL;
List *rowMarks;
List *tlist;
PlannerInfo subroot;
ListCell *l;
+ List *resultRelations = NIL;
foreach(l, root->append_rel_list)
{
@@ -772,8 +775,6 @@ inheritance_planner(PlannerInfo *root)
}
}
- root->resultRelations = resultRelations;
-
/* Mark result as unordered (probably unnecessary) */
root->query_pathkeys = NIL;
@@ -783,7 +784,6 @@ inheritance_planner(PlannerInfo *root)
*/
if (subplans == NIL)
{
- root->resultRelations = list_make1_int(parentRTindex);
/* although dummy, it must have a valid tlist for executor */
tlist = preprocess_targetlist(root, parse->targetList);
return (Plan *) make_result(root,
@@ -818,7 +818,7 @@ inheritance_planner(PlannerInfo *root)
/* And last, tack on a ModifyTable node to do the UPDATE/DELETE work */
return (Plan *) make_modifytable(parse->commandType,
- copyObject(root->resultRelations),
+ resultRelations,
subplans,
returningLists,
rowMarks,
@@ -1667,12 +1667,6 @@ grouping_planner(PlannerInfo *root, double tuple_fraction)
count_est);
}
- /* Compute result-relations list if needed */
- if (parse->resultRelation)
- root->resultRelations = list_make1_int(parse->resultRelation);
- else
- root->resultRelations = NIL;
-
/*
* Return the actual output ordering in query_pathkeys for possible use by
* an outer query level.
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 34c3ea6..47d2c00 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -516,6 +516,10 @@ set_plan_refs(PlannerGlobal *glob, Plan *plan, int rtoffset)
(Plan *) lfirst(l),
rtoffset);
}
+
+ splan->resultRelIndex = list_length(glob->resultRelations);
+ glob->resultRelations = list_concat(glob->resultRelations,
+ splan->resultRelations);
}
break;
case T_Append:
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index 291ec99..bd61cc6 100644
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -873,16 +873,33 @@ SS_process_ctes(PlannerInfo *root)
Bitmapset *tmpset;
int paramid;
Param *prm;
+ CmdType cmdType = ((Query *) cte->ctequery)->commandType;
/*
- * Ignore CTEs that are not actually referenced anywhere.
+ * Ignore SELECT CTEs that are not actually referenced anywhere.
*/
- if (cte->cterefcount == 0)
+ if (cte->cterefcount == 0 && cmdType == CMD_SELECT)
{
/* Make a dummy entry in cte_plan_ids */
root->cte_plan_ids = lappend_int(root->cte_plan_ids, -1);
continue;
}
+ else if (cmdType != CMD_SELECT)
+ {
+ /* We don't know reference counts until here */
+ if (cte->cterefcount > 0 &&
+ ((Query *) cte->ctequery)->returningList == NIL)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("INSERT/UPDATE/DELETE without RETURNING is only allowed inside a non-referenced
CTE")));
+ }
+
+ if (root->query_level > 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("INSERT/UPDATE/DELETE inside a CTE is only allowed on the top level")));
+ }
/*
* Copy the source Query node. Probably not necessary, but let's keep
@@ -899,6 +916,9 @@ SS_process_ctes(PlannerInfo *root)
cte->cterecursive, 0.0,
&subroot);
+ if (subroot->parse->commandType != CMD_SELECT)
+ root->glob->hasWritableCtes = true;
+
/*
* Make a SubPlan node for it. This is just enough unlike
* build_subplan that we can't share code.
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 4ed5b06..cc748c7 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -7374,6 +7374,33 @@ common_table_expr: name opt_name_list AS select_with_parens
n->location = @1;
$$ = (Node *) n;
}
+ | name opt_name_list AS '(' InsertStmt ')'
+ {
+ CommonTableExpr *n = makeNode(CommonTableExpr);
+ n->ctename = $1;
+ n->aliascolnames = $2;
+ n->ctequery = $5;
+ n->location = @1;
+ $$ = (Node *) n;
+ }
+ | name opt_name_list AS '(' UpdateStmt ')'
+ {
+ CommonTableExpr *n = makeNode(CommonTableExpr);
+ n->ctename = $1;
+ n->aliascolnames = $2;
+ n->ctequery = $5;
+ n->location = @1;
+ $$ = (Node *) n;
+ }
+ | name opt_name_list AS '(' DeleteStmt ')'
+ {
+ CommonTableExpr *n = makeNode(CommonTableExpr);
+ n->ctename = $1;
+ n->aliascolnames = $2;
+ n->ctequery = $5;
+ n->location = @1;
+ $$ = (Node *) n;
+ }
;
into_clause:
diff --git a/src/backend/parser/parse_cte.c b/src/backend/parser/parse_cte.c
index e2fcc60..f251ec8 100644
--- a/src/backend/parser/parse_cte.c
+++ b/src/backend/parser/parse_cte.c
@@ -18,6 +18,7 @@
#include "nodes/nodeFuncs.h"
#include "parser/analyze.h"
#include "parser/parse_cte.h"
+#include "nodes/plannodes.h"
#include "utils/builtins.h"
@@ -225,22 +226,25 @@ static void
analyzeCTE(ParseState *pstate, CommonTableExpr *cte)
{
Query *query;
-
- /* Analysis not done already */
- Assert(IsA(cte->ctequery, SelectStmt));
+ List *cteList;
query = parse_sub_analyze(cte->ctequery, pstate, cte, false);
cte->ctequery = (Node *) query;
+ if (query->commandType == CMD_SELECT)
+ cteList = query->targetList;
+ else
+ cteList = query->returningList;
+
/*
* Check that we got something reasonable. Many of these conditions are
* impossible given restrictions of the grammar, but check 'em anyway.
- * (These are the same checks as in transformRangeSubselect.)
+ * Note, however, that we can't yet decice whether to allow
+ * INSERT/UPDATE/DELETE without a RETURNING clause or not because we don't
+ * know the refcount.
*/
- if (!IsA(query, Query) ||
- query->commandType != CMD_SELECT ||
- query->utilityStmt != NULL)
- elog(ERROR, "unexpected non-SELECT command in subquery in WITH");
+ Assert(IsA(query, Query) && query->utilityStmt == NULL);
+
if (query->intoClause)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
@@ -251,7 +255,7 @@ analyzeCTE(ParseState *pstate, CommonTableExpr *cte)
if (!cte->cterecursive)
{
/* Compute the output column names/types if not done yet */
- analyzeCTETargetList(pstate, cte, query->targetList);
+ analyzeCTETargetList(pstate, cte, cteList);
}
else
{
@@ -269,7 +273,7 @@ analyzeCTE(ParseState *pstate, CommonTableExpr *cte)
lctyp = list_head(cte->ctecoltypes);
lctypmod = list_head(cte->ctecoltypmods);
varattno = 0;
- foreach(lctlist, query->targetList)
+ foreach(lctlist, cteList)
{
TargetEntry *te = (TargetEntry *) lfirst(lctlist);
Node *texpr;
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index 7140758..24a2cc2 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -24,6 +24,7 @@
#include "funcapi.h"
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
+#include "nodes/plannodes.h"
#include "parser/parsetree.h"
#include "parser/parse_relation.h"
#include "parser/parse_type.h"
diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c
index 6883dc3..e09875d 100644
--- a/src/backend/parser/parse_target.c
+++ b/src/backend/parser/parse_target.c
@@ -314,10 +314,20 @@ markTargetListOrigin(ParseState *pstate, TargetEntry *tle,
{
CommonTableExpr *cte = GetCTEForRTE(pstate, rte, netlevelsup);
TargetEntry *ste;
+ List *cteList;
+ Query *ctequery;
/* should be analyzed by now */
Assert(IsA(cte->ctequery, Query));
- ste = get_tle_by_resno(((Query *) cte->ctequery)->targetList,
+
+ ctequery = (Query *) cte->ctequery;
+
+ if (ctequery->commandType == CMD_SELECT)
+ cteList = ctequery->targetList;
+ else
+ cteList = ctequery->returningList;
+
+ ste = get_tle_by_resno(cteList,
attnum);
if (ste == NULL || ste->resjunk)
elog(ERROR, "subquery %s does not have attribute %d",
@@ -1345,11 +1355,20 @@ expandRecordVariable(ParseState *pstate, Var *var, int levelsup)
{
CommonTableExpr *cte = GetCTEForRTE(pstate, rte, netlevelsup);
TargetEntry *ste;
+ List *cteList;
+ Query *ctequery;
/* should be analyzed by now */
Assert(IsA(cte->ctequery, Query));
- ste = get_tle_by_resno(((Query *) cte->ctequery)->targetList,
- attnum);
+
+ ctequery = (Query *) cte->ctequery;
+
+ if (ctequery->commandType == CMD_SELECT)
+ cteList = ctequery->targetList;
+ else
+ cteList = ctequery->returningList;
+
+ ste = get_tle_by_resno(cteList, attnum);
if (ste == NULL || ste->resjunk)
elog(ERROR, "subquery %s does not have attribute %d",
rte->eref->aliasname, attnum);
@@ -1372,7 +1391,7 @@ expandRecordVariable(ParseState *pstate, Var *var, int levelsup)
levelsup++)
pstate = pstate->parentParseState;
mypstate.parentParseState = pstate;
- mypstate.p_rtable = ((Query *) cte->ctequery)->rtable;
+ mypstate.p_rtable = ctequery->rtable;
/* don't bother filling the rest of the fake pstate */
return expandRecordVariable(&mypstate, (Var *) expr, 0);
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 7af481d..2cc05fb 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -1632,6 +1632,10 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
bool returning = false;
Query *qual_product = NULL;
List *rewritten = NIL;
+ ListCell *lc;
+ CommonTableExpr *cte;
+ Query *ctequery;
+ List *newstuff;
/*
* If the statement is an update, insert or delete - fire rules on it.
@@ -1749,7 +1753,6 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
foreach(n, product_queries)
{
Query *pt = (Query *) lfirst(n);
- List *newstuff;
newstuff = RewriteQuery(pt, rewrite_events);
rewritten = list_concat(rewritten, newstuff);
@@ -1804,6 +1807,55 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
}
/*
+ * Rewrite DML statements inside CTEs. If there are any
+ * DO ALSO rules, they are added to the top level (there
+ * won't be any non-top-level CTEs with DML).
+ */
+ foreach(lc, parsetree->cteList)
+ {
+ cte = lfirst(lc);
+
+ ctequery = (Query *) cte->ctequery;
+
+ if (ctequery->commandType == CMD_SELECT)
+ continue;
+
+ newstuff = RewriteQuery(ctequery, NIL);
+
+ /*
+ * For UPDATE and DELETE, the actual query is
+ * added to the end of the list.
+ */
+ if (list_length(newstuff) > 1 &&
+ ctequery->commandType != CMD_INSERT)
+ {
+ ListCell *lc;
+ int n = 1;
+
+ foreach(lc, newstuff)
+ {
+ /*
+ * If this is the last one, don't add it to the results.
+ * Instead, update the query inside the CTE.
+ */
+ if (n == list_length(newstuff))
+ cte->ctequery = (Node *) lfirst(lc);
+ else
+ rewritten = lcons((void *) lfirst(lc), rewritten);
+
+ n++;
+ }
+
+ }
+ else
+ {
+ cte->ctequery = (Node *) linitial(newstuff);
+ rewritten = list_concat(rewritten,
+ list_delete_first(newstuff));
+ }
+ }
+
+ /*
* For INSERTs, the original query is done first; for UPDATE/DELETE, it is
* done last. This is needed because update and delete rule actions might
* not do anything if they are invoked after the update or delete is
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index 6f46a29..1c6c56e 100644
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -293,6 +293,7 @@ ChoosePortalStrategy(List *stmts)
if (pstmt->canSetTag)
{
if (pstmt->commandType == CMD_SELECT &&
+ pstmt->hasWritableCtes == false &&
pstmt->utilityStmt == NULL &&
pstmt->intoClause == NULL)
return PORTAL_ONE_SELECT;
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 42b1c6a..be7a1b5 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -3858,9 +3858,16 @@ get_name_for_var_field(Var *var, int fieldno,
}
if (lc != NULL)
{
- Query *ctequery = (Query *) cte->ctequery;
- TargetEntry *ste = get_tle_by_resno(ctequery->targetList,
- attnum);
+ Query *ctequery = (Query *) cte->ctequery;
+ List *ctelist;
+ TargetEntry *ste;
+
+ if (ctequery->commandType != CMD_SELECT)
+ ctelist = ctequery->returningList;
+ else
+ ctelist = ctequery->targetList;
+
+ ste = get_tle_by_resno(ctelist, attnum);
if (ste == NULL || ste->resjunk)
elog(ERROR, "subquery %s does not have attribute %d",
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index d367b2a..cccc8f6 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -319,7 +319,8 @@ extern void ExecCloseScanRelation(Relation scanrel);
extern void ExecOpenIndices(ResultRelInfo *resultRelInfo);
extern void ExecCloseIndices(ResultRelInfo *resultRelInfo);
-extern List *ExecInsertIndexTuples(TupleTableSlot *slot, ItemPointer tupleid,
+extern List *ExecInsertIndexTuples(ResultRelInfo *resultRelInfo,
+ TupleTableSlot *slot, ItemPointer tupleid,
EState *estate, bool is_vacuum_full);
extern void RegisterExprContextCallback(ExprContext *econtext,
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 9acd5ec..2c84493 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1021,6 +1021,8 @@ typedef struct ModifyTableState
PlanState **mt_plans; /* subplans (one per target rel) */
int mt_nplans; /* number of plans in the array */
int mt_whichplan; /* which one is being executed (0..n-1) */
+ int resultRelIndex;
+ ResultRelInfo *resultRelInfo;
EPQState mt_epqstate; /* for evaluating EvalPlanQual rechecks */
bool fireBSTriggers; /* do we need to fire stmt triggers? */
} ModifyTableState;
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index d8a89fa..5b67b0a 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -55,6 +55,8 @@ typedef struct PlannedStmt
IntoClause *intoClause; /* target for SELECT INTO / CREATE TABLE AS */
+ bool hasWritableCtes;
+
List *subplans; /* Plan trees for SubPlan expressions */
Bitmapset *rewindPlanIDs; /* indices of subplans that require REWIND */
@@ -165,6 +167,7 @@ typedef struct ModifyTable
Plan plan;
CmdType operation; /* INSERT, UPDATE, or DELETE */
List *resultRelations; /* integer list of RT indexes */
+ int resultRelIndex;
List *plans; /* plan(s) producing source data */
List *returningLists; /* per-target-table RETURNING tlists */
List *rowMarks; /* PlanRowMarks (non-locking only) */
diff --git a/src/include/nodes/relation.h b/src/include/nodes/relation.h
index 4c82106..5f13076 100644
--- a/src/include/nodes/relation.h
+++ b/src/include/nodes/relation.h
@@ -80,6 +80,10 @@ typedef struct PlannerGlobal
List *invalItems; /* other dependencies, as PlanInvalItems */
+ bool hasWritableCtes;/* is there an (INSERT|UPDATE|DELETE) .. RETURNING inside a CTE? */
+
+ List *resultRelations;/* list of result relations */
+
Index lastPHId; /* highest PlaceHolderVar ID assigned */
bool transientPlan; /* redo plan when TransactionXmin changes? */
@@ -142,8 +146,6 @@ typedef struct PlannerInfo
List *join_rel_list; /* list of join-relation RelOptInfos */
struct HTAB *join_rel_hash; /* optional hashtable for join relations */
- List *resultRelations; /* integer list of RT indexes, or NIL */
-
List *init_plans; /* init SubPlans for query */
List *cte_plan_ids; /* per-CTE-item list of subplan IDs */
diff --git a/src/test/regress/expected/with.out b/src/test/regress/expected/with.out
index a3e94e9..4cfb569 100644
--- a/src/test/regress/expected/with.out
+++ b/src/test/regress/expected/with.out
@@ -1026,3 +1026,85 @@ SELECT * FROM t;
10
(55 rows)
+--
+-- Writeable CTEs with RETURNING
+--
+WITH t AS (
+ INSERT INTO y
+ VALUES
+ (11),
+ (12),
+ (13),
+ (14),
+ (15),
+ (16),
+ (17),
+ (18),
+ (19),
+ (20)
+ RETURNING *
+)
+SELECT * FROM t;
+ a
+----
+ 11
+ 12
+ 13
+ 14
+ 15
+ 16
+ 17
+ 18
+ 19
+ 20
+(10 rows)
+
+WITH t AS (
+ UPDATE y
+ SET a=a+1
+ RETURNING *
+)
+SELECT * FROM t;
+ a
+----
+ 2
+ 3
+ 4
+ 5
+ 6
+ 7
+ 8
+ 9
+ 10
+ 11
+ 12
+ 13
+ 14
+ 15
+ 16
+ 17
+ 18
+ 19
+ 20
+ 21
+(20 rows)
+
+WITH t AS (
+ DELETE FROM y
+ WHERE a <= 10
+ RETURNING *
+)
+SELECT * FROM t;
+ a
+----
+ 2
+ 3
+ 4
+ 5
+ 6
+ 7
+ 8
+ 9
+ 10
+(9 rows)
+
diff --git a/src/test/regress/sql/with.sql b/src/test/regress/sql/with.sql
index 2cbaa42..5b35af3 100644
--- a/src/test/regress/sql/with.sql
+++ b/src/test/regress/sql/with.sql
@@ -500,3 +500,38 @@ WITH RECURSIVE t(j) AS (
SELECT j+1 FROM t WHERE j < 10
)
SELECT * FROM t;
+
+--
+-- Writeable CTEs with RETURNING
+--
+
+WITH t AS (
+ INSERT INTO y
+ VALUES
+ (11),
+ (12),
+ (13),
+ (14),
+ (15),
+ (16),
+ (17),
+ (18),
+ (19),
+ (20)
+ RETURNING *
+)
+SELECT * FROM t;
+
+WITH t AS (
+ UPDATE y
+ SET a=a+1
+ RETURNING *
+)
+SELECT * FROM t;
+
+WITH t AS (
+ DELETE FROM y
+ WHERE a <= 10
+ RETURNING *
+)
+SELECT * FROM t;
I wrote:
> Attached is the latest version of this patch.
Here's that same patch in context diff format. Sorry for the noise.
Regards,
Marko Tiikkaja
*** a/doc/src/sgml/queries.sgml
--- b/doc/src/sgml/queries.sgml
***************
*** 1499,1505 **** SELECT 3, 'three';
<synopsis>
SELECT <replaceable>select_list</replaceable> FROM <replaceable>table_expression</replaceable>
</synopsis>
! and can appear anywhere a <literal>SELECT</> can. For example, you can
use it as part of a <literal>UNION</>, or attach a
<replaceable>sort_specification</replaceable> (<literal>ORDER BY</>,
<literal>LIMIT</>, and/or <literal>OFFSET</>) to it. <literal>VALUES</>
--- 1499,1505 ----
<synopsis>
SELECT <replaceable>select_list</replaceable> FROM <replaceable>table_expression</replaceable>
</synopsis>
! and can appear anywhere a <literal>SELECT</literal> can. For example, you can
use it as part of a <literal>UNION</>, or attach a
<replaceable>sort_specification</replaceable> (<literal>ORDER BY</>,
<literal>LIMIT</>, and/or <literal>OFFSET</>) to it. <literal>VALUES</>
***************
*** 1529,1538 **** SELECT <replaceable>select_list</replaceable> FROM <replaceable>table_expression
</indexterm>
<para>
! <literal>WITH</> provides a way to write subqueries for use in a larger
! <literal>SELECT</> query. The subqueries can be thought of as defining
! temporary tables that exist just for this query. One use of this feature
! is to break down complicated queries into simpler parts. An example is:
<programlisting>
WITH regional_sales AS (
--- 1529,1539 ----
</indexterm>
<para>
! <literal>WITH</> provides a way to write subqueries for use in a
! larger query. The subqueries can be thought of as defining
! temporary tables that exist just for this query. One use of this
! feature is to break down complicated queries into simpler parts.
! An example is:
<programlisting>
WITH regional_sales AS (
***************
*** 1560,1565 **** GROUP BY region, product;
--- 1561,1590 ----
</para>
<para>
+ A <literal>WITH</literal> clause can also have an
+ <literal>INSERT</literal>, <literal>UPDATE</literal> or
+ <literal>DELETE</literal> (each optionally with a
+ <literal>RETURNING</literal> clause) statement in it. The example below
+ moves rows from the main table, foo_log into a partition,
+ foo_log_200910.
+
+ <programlisting>
+ WITH rows AS (
+ DELETE FROM ONLY foo_log
+ WHERE
+ foo_date >= '2009-10-01' AND
+ foo_date < '2009-11-01'
+ RETURNING *
+ ), t AS (
+ INSERT INTO foo_log_200910
+ SELECT * FROM rows
+ )
+ VALUES(true);
+ </programlisting>
+
+ </para>
+
+ <para>
The optional <literal>RECURSIVE</> modifier changes <literal>WITH</>
from a mere syntactic convenience into a feature that accomplishes
things not otherwise possible in standard SQL. Using
*** a/doc/src/sgml/ref/select.sgml
--- b/doc/src/sgml/ref/select.sgml
***************
*** 58,64 **** SELECT [ ALL | DISTINCT [ ON ( <replaceable class="parameter">expression</replac
<phrase>and <replaceable class="parameter">with_query</replaceable> is:</phrase>
! <replaceable class="parameter">with_query_name</replaceable> [ ( <replaceable
class="parameter">column_name</replaceable>[, ...] ) ] AS ( <replaceable class="parameter">select</replaceable> )
TABLE { [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ] | <replaceable
class="parameter">with_query_name</replaceable>}
</synopsis>
--- 58,64 ----
<phrase>and <replaceable class="parameter">with_query</replaceable> is:</phrase>
! <replaceable class="parameter">with_query_name</replaceable> [ ( <replaceable
class="parameter">column_name</replaceable>[, ...] ) ] AS ( <replaceable class="parameter">select</replaceable> |
(<replaceableclass="parameter">insert</replaceable> | <replaceable class="parameter">update</replaceable> |
<replaceableclass="parameter">delete</replaceable> [ RETURNING...]))
TABLE { [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ] | <replaceable
class="parameter">with_query_name</replaceable>}
</synopsis>
*** a/src/backend/commands/copy.c
--- b/src/backend/commands/copy.c
***************
*** 2160,2166 **** CopyFrom(CopyState cstate)
heap_insert(cstate->rel, tuple, mycid, hi_options, bistate);
if (resultRelInfo->ri_NumIndices > 0)
! recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self),
estate, false);
/* AFTER ROW INSERT Triggers */
--- 2160,2167 ----
heap_insert(cstate->rel, tuple, mycid, hi_options, bistate);
if (resultRelInfo->ri_NumIndices > 0)
! recheckIndexes = ExecInsertIndexTuples(resultRelInfo,
! slot, &(tuple->t_self),
estate, false);
/* AFTER ROW INSERT Triggers */
*** a/src/backend/commands/portalcmds.c
--- b/src/backend/commands/portalcmds.c
***************
*** 48,53 **** PerformCursorOpen(PlannedStmt *stmt, ParamListInfo params,
--- 48,58 ----
Portal portal;
MemoryContext oldContext;
+ if (stmt->hasWritableCtes)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("Non-SELECT cursors are not implemented")));
+
if (cstmt == NULL || !IsA(cstmt, DeclareCursorStmt))
elog(ERROR, "PerformCursorOpen called for non-cursor query");
*** a/src/backend/commands/vacuum.c
--- b/src/backend/commands/vacuum.c
***************
*** 3088,3094 **** move_chain_tuple(Relation rel,
if (ec->resultRelInfo->ri_NumIndices > 0)
{
ExecStoreTuple(&newtup, ec->slot, InvalidBuffer, false);
! ExecInsertIndexTuples(ec->slot, &(newtup.t_self), ec->estate, true);
ResetPerTupleExprContext(ec->estate);
}
}
--- 3088,3094 ----
if (ec->resultRelInfo->ri_NumIndices > 0)
{
ExecStoreTuple(&newtup, ec->slot, InvalidBuffer, false);
! ExecInsertIndexTuples(ec->resultRelInfo, ec->slot, &(newtup.t_self), ec->estate, true);
ResetPerTupleExprContext(ec->estate);
}
}
***************
*** 3214,3220 **** move_plain_tuple(Relation rel,
if (ec->resultRelInfo->ri_NumIndices > 0)
{
ExecStoreTuple(&newtup, ec->slot, InvalidBuffer, false);
! ExecInsertIndexTuples(ec->slot, &(newtup.t_self), ec->estate, true);
ResetPerTupleExprContext(ec->estate);
}
}
--- 3214,3220 ----
if (ec->resultRelInfo->ri_NumIndices > 0)
{
ExecStoreTuple(&newtup, ec->slot, InvalidBuffer, false);
! ExecInsertIndexTuples(ec->resultRelInfo, ec->slot, &(newtup.t_self), ec->estate, true);
ResetPerTupleExprContext(ec->estate);
}
}
*** a/src/backend/commands/view.c
--- b/src/backend/commands/view.c
***************
*** 394,399 **** DefineView(ViewStmt *stmt, const char *queryString)
--- 394,400 ----
Query *viewParse;
Oid viewOid;
RangeVar *view;
+ ListCell *lc;
/*
* Run parse analysis to convert the raw parse tree to a Query. Note this
***************
*** 412,417 **** DefineView(ViewStmt *stmt, const char *queryString)
--- 413,430 ----
viewParse->commandType != CMD_SELECT)
elog(ERROR, "unexpected parse analysis result");
+ /* .. but it doesn't check for DML inside CTEs */
+ foreach(lc, viewParse->cteList)
+ {
+ CommonTableExpr *cte;
+
+ cte = (CommonTableExpr *) lfirst(lc);
+ if (((Query *) cte->ctequery)->commandType != CMD_SELECT)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("INSERT/UPDATE/DELETE inside a CTE not allowed in a view definition")));
+ }
+
/*
* If a list of column names was given, run through and insert these into
* the actual query tree. - thomas 2000-03-08
*** a/src/backend/executor/execMain.c
--- b/src/backend/executor/execMain.c
***************
*** 924,939 **** InitResultRelInfo(ResultRelInfo *resultRelInfo,
resultRelInfo->ri_ConstraintExprs = NULL;
resultRelInfo->ri_junkFilter = NULL;
resultRelInfo->ri_projectReturning = NULL;
-
- /*
- * If there are indices on the result relation, open them and save
- * descriptors in the result relation info, so that we can add new index
- * entries for the tuples we add/update. We need not do this for a
- * DELETE, however, since deletion doesn't affect indexes.
- */
- if (resultRelationDesc->rd_rel->relhasindex &&
- operation != CMD_DELETE)
- ExecOpenIndices(resultRelInfo);
}
/*
--- 924,929 ----
***************
*** 1174,1179 **** ExecutePlan(EState *estate,
--- 1164,1251 ----
*/
estate->es_direction = direction;
+ /* Process top-level CTEs in case they have writes inside */
+ if (estate->es_plannedstmt->hasWritableCtes)
+ {
+ ListCell *lc;
+
+ foreach(lc, estate->es_plannedstmt->planTree->initPlan)
+ {
+ SubPlan *sp;
+ int cte_param_id;
+ ParamExecData* prmdata;
+ CteScanState *leader;
+
+ sp = (SubPlan *) lfirst(lc);
+ if (sp->subLinkType != CTE_SUBLINK)
+ continue;
+
+ cte_param_id = linitial_int(sp->setParam);
+ prmdata = &(estate->es_param_exec_vals[cte_param_id]);
+ leader = (CteScanState *) DatumGetPointer(prmdata->value);
+
+
+ /*
+ * bump CID.
+ *
+ * We're currently relying on the fact that there can only be
+ * a SELECT or VALUES as the top-level statement.
+ *
+ * XXX we should probably update the snapshot a bit differently
+ */
+ CommandCounterIncrement();
+ estate->es_output_cid = GetCurrentCommandId(true);
+ estate->es_snapshot->curcid = estate->es_output_cid;
+
+ /*
+ * If there's no leader, the CTE isn't referenced anywhere
+ * so we can just go ahead and scan the plan
+ */
+ if (!leader)
+ {
+ TupleTableSlot *slot;
+ PlanState *ps = (PlanState *) list_nth(estate->es_subplanstates,
+ sp->plan_id - 1);
+
+ Assert(IsA(ps, ModifyTableState));
+
+ /*
+ * We might have a RETURNING here, which means that
+ * we have to loop until the plan returns NULL.
+ */
+ for (;;)
+ {
+ slot = ExecProcNode(ps);
+ if (TupIsNull(slot))
+ break;
+ }
+ }
+ else
+ {
+ TupleTableSlot* slot;
+ PlanState *ps = (PlanState *) list_nth(estate->es_subplanstates,
+ sp->plan_id - 1);
+
+ /* Regular CTE, ignore */
+ if (!IsA(ps, ModifyTableState))
+ continue;
+
+ /*
+ * Scan through the leader CTE so the RETURNING tuples are
+ * stored into the tuple store.
+ */
+ for (;;)
+ {
+ slot = ExecProcNode((PlanState *) leader);
+ if (TupIsNull(slot))
+ break;
+ }
+
+ ExecReScan((PlanState *) leader, NULL);
+ }
+ }
+ }
+
/*
* Loop until we've processed the proper number of tuples from the plan.
*/
***************
*** 1943,1949 **** EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree)
* ExecInitSubPlan expects to be able to find these entries.
* Some of the SubPlans might not be used in the part of the plan tree
* we intend to run, but since it's not easy to tell which, we just
! * initialize them all.
*/
Assert(estate->es_subplanstates == NIL);
foreach(l, parentestate->es_plannedstmt->subplans)
--- 2015,2022 ----
* ExecInitSubPlan expects to be able to find these entries.
* Some of the SubPlans might not be used in the part of the plan tree
* we intend to run, but since it's not easy to tell which, we just
! * initialize them all. However, we will never run ModifyTable nodes in
! * EvalPlanQual() so don't initialize them.
*/
Assert(estate->es_subplanstates == NIL);
foreach(l, parentestate->es_plannedstmt->subplans)
***************
*** 1951,1957 **** EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree)
Plan *subplan = (Plan *) lfirst(l);
PlanState *subplanstate;
! subplanstate = ExecInitNode(subplan, estate, 0);
estate->es_subplanstates = lappend(estate->es_subplanstates,
subplanstate);
--- 2024,2034 ----
Plan *subplan = (Plan *) lfirst(l);
PlanState *subplanstate;
! /* Don't initialize ModifyTable subplans. */
! if (IsA(subplan, ModifyTable))
! subplanstate = NULL;
! else
! subplanstate = ExecInitNode(subplan, estate, 0);
estate->es_subplanstates = lappend(estate->es_subplanstates,
subplanstate);
*** a/src/backend/executor/execUtils.c
--- b/src/backend/executor/execUtils.c
***************
*** 968,980 **** ExecCloseIndices(ResultRelInfo *resultRelInfo)
* ----------------------------------------------------------------
*/
List *
! ExecInsertIndexTuples(TupleTableSlot *slot,
ItemPointer tupleid,
EState *estate,
bool is_vacuum_full)
{
List *result = NIL;
- ResultRelInfo *resultRelInfo;
int i;
int numIndices;
RelationPtr relationDescs;
--- 968,980 ----
* ----------------------------------------------------------------
*/
List *
! ExecInsertIndexTuples(ResultRelInfo* resultRelInfo,
! TupleTableSlot *slot,
ItemPointer tupleid,
EState *estate,
bool is_vacuum_full)
{
List *result = NIL;
int i;
int numIndices;
RelationPtr relationDescs;
***************
*** 987,993 **** ExecInsertIndexTuples(TupleTableSlot *slot,
/*
* Get information from the result relation info structure.
*/
- resultRelInfo = estate->es_result_relation_info;
numIndices = resultRelInfo->ri_NumIndices;
relationDescs = resultRelInfo->ri_IndexRelationDescs;
indexInfoArray = resultRelInfo->ri_IndexRelationInfo;
--- 987,992 ----
*** a/src/backend/executor/nodeModifyTable.c
--- b/src/backend/executor/nodeModifyTable.c
***************
*** 158,169 **** ExecProcessReturning(ProjectionInfo *projectReturning,
* ----------------------------------------------------------------
*/
static TupleTableSlot *
! ExecInsert(TupleTableSlot *slot,
TupleTableSlot *planSlot,
EState *estate)
{
HeapTuple tuple;
- ResultRelInfo *resultRelInfo;
Relation resultRelationDesc;
Oid newId;
List *recheckIndexes = NIL;
--- 158,169 ----
* ----------------------------------------------------------------
*/
static TupleTableSlot *
! ExecInsert(ResultRelInfo *resultRelInfo,
! TupleTableSlot *slot,
TupleTableSlot *planSlot,
EState *estate)
{
HeapTuple tuple;
Relation resultRelationDesc;
Oid newId;
List *recheckIndexes = NIL;
***************
*** 177,183 **** ExecInsert(TupleTableSlot *slot,
/*
* get information on the (current) result relation
*/
- resultRelInfo = estate->es_result_relation_info;
resultRelationDesc = resultRelInfo->ri_RelationDesc;
/*
--- 177,182 ----
***************
*** 247,253 **** ExecInsert(TupleTableSlot *slot,
* insert index entries for tuple
*/
if (resultRelInfo->ri_NumIndices > 0)
! recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self),
estate, false);
/* AFTER ROW INSERT Triggers */
--- 246,253 ----
* insert index entries for tuple
*/
if (resultRelInfo->ri_NumIndices > 0)
! recheckIndexes = ExecInsertIndexTuples(resultRelInfo,
! slot, &(tuple->t_self),
estate, false);
/* AFTER ROW INSERT Triggers */
***************
*** 271,282 **** ExecInsert(TupleTableSlot *slot,
* ----------------------------------------------------------------
*/
static TupleTableSlot *
! ExecDelete(ItemPointer tupleid,
TupleTableSlot *planSlot,
EPQState *epqstate,
EState *estate)
{
- ResultRelInfo *resultRelInfo;
Relation resultRelationDesc;
HTSU_Result result;
ItemPointerData update_ctid;
--- 271,282 ----
* ----------------------------------------------------------------
*/
static TupleTableSlot *
! ExecDelete(ResultRelInfo *resultRelInfo,
! ItemPointer tupleid,
TupleTableSlot *planSlot,
EPQState *epqstate,
EState *estate)
{
Relation resultRelationDesc;
HTSU_Result result;
ItemPointerData update_ctid;
***************
*** 285,291 **** ExecDelete(ItemPointer tupleid,
/*
* get information on the (current) result relation
*/
- resultRelInfo = estate->es_result_relation_info;
resultRelationDesc = resultRelInfo->ri_RelationDesc;
/* BEFORE ROW DELETE Triggers */
--- 285,290 ----
***************
*** 414,427 **** ldelete:;
* ----------------------------------------------------------------
*/
static TupleTableSlot *
! ExecUpdate(ItemPointer tupleid,
TupleTableSlot *slot,
TupleTableSlot *planSlot,
EPQState *epqstate,
EState *estate)
{
HeapTuple tuple;
- ResultRelInfo *resultRelInfo;
Relation resultRelationDesc;
HTSU_Result result;
ItemPointerData update_ctid;
--- 413,426 ----
* ----------------------------------------------------------------
*/
static TupleTableSlot *
! ExecUpdate(ResultRelInfo *resultRelInfo,
! ItemPointer tupleid,
TupleTableSlot *slot,
TupleTableSlot *planSlot,
EPQState *epqstate,
EState *estate)
{
HeapTuple tuple;
Relation resultRelationDesc;
HTSU_Result result;
ItemPointerData update_ctid;
***************
*** 443,449 **** ExecUpdate(ItemPointer tupleid,
/*
* get information on the (current) result relation
*/
- resultRelInfo = estate->es_result_relation_info;
resultRelationDesc = resultRelInfo->ri_RelationDesc;
/* BEFORE ROW UPDATE Triggers */
--- 442,447 ----
***************
*** 561,567 **** lreplace:;
* If it's a HOT update, we mustn't insert new index entries.
*/
if (resultRelInfo->ri_NumIndices > 0 && !HeapTupleIsHeapOnly(tuple))
! recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self),
estate, false);
/* AFTER ROW UPDATE Triggers */
--- 559,566 ----
* If it's a HOT update, we mustn't insert new index entries.
*/
if (resultRelInfo->ri_NumIndices > 0 && !HeapTupleIsHeapOnly(tuple))
! recheckIndexes = ExecInsertIndexTuples(resultRelInfo,
! slot, &(tuple->t_self),
estate, false);
/* AFTER ROW UPDATE Triggers */
***************
*** 587,601 **** fireBSTriggers(ModifyTableState *node)
{
case CMD_INSERT:
ExecBSInsertTriggers(node->ps.state,
! node->ps.state->es_result_relations);
break;
case CMD_UPDATE:
ExecBSUpdateTriggers(node->ps.state,
! node->ps.state->es_result_relations);
break;
case CMD_DELETE:
ExecBSDeleteTriggers(node->ps.state,
! node->ps.state->es_result_relations);
break;
default:
elog(ERROR, "unknown operation");
--- 586,600 ----
{
case CMD_INSERT:
ExecBSInsertTriggers(node->ps.state,
! node->resultRelInfo);
break;
case CMD_UPDATE:
ExecBSUpdateTriggers(node->ps.state,
! node->resultRelInfo);
break;
case CMD_DELETE:
ExecBSDeleteTriggers(node->ps.state,
! node->resultRelInfo);
break;
default:
elog(ERROR, "unknown operation");
***************
*** 613,627 **** fireASTriggers(ModifyTableState *node)
{
case CMD_INSERT:
ExecASInsertTriggers(node->ps.state,
! node->ps.state->es_result_relations);
break;
case CMD_UPDATE:
ExecASUpdateTriggers(node->ps.state,
! node->ps.state->es_result_relations);
break;
case CMD_DELETE:
ExecASDeleteTriggers(node->ps.state,
! node->ps.state->es_result_relations);
break;
default:
elog(ERROR, "unknown operation");
--- 612,626 ----
{
case CMD_INSERT:
ExecASInsertTriggers(node->ps.state,
! node->resultRelInfo);
break;
case CMD_UPDATE:
ExecASUpdateTriggers(node->ps.state,
! node->resultRelInfo);
break;
case CMD_DELETE:
ExecASDeleteTriggers(node->ps.state,
! node->resultRelInfo);
break;
default:
elog(ERROR, "unknown operation");
***************
*** 643,648 **** ExecModifyTable(ModifyTableState *node)
--- 642,648 ----
EState *estate = node->ps.state;
CmdType operation = node->operation;
PlanState *subplanstate;
+ ResultRelInfo *resultRelInfo;
JunkFilter *junkfilter;
TupleTableSlot *slot;
TupleTableSlot *planSlot;
***************
*** 658,674 **** ExecModifyTable(ModifyTableState *node)
node->fireBSTriggers = false;
}
- /*
- * es_result_relation_info must point to the currently active result
- * relation. (Note we assume that ModifyTable nodes can't be nested.)
- * We want it to be NULL whenever we're not within ModifyTable, though.
- */
- estate->es_result_relation_info =
- estate->es_result_relations + node->mt_whichplan;
-
/* Preload local variables */
subplanstate = node->mt_plans[node->mt_whichplan];
! junkfilter = estate->es_result_relation_info->ri_junkFilter;
/*
* Fetch rows from subplan(s), and execute the required table modification
--- 658,667 ----
node->fireBSTriggers = false;
}
/* Preload local variables */
subplanstate = node->mt_plans[node->mt_whichplan];
! resultRelInfo = node->resultRelInfo + node->mt_whichplan;
! junkfilter = resultRelInfo->ri_junkFilter;
/*
* Fetch rows from subplan(s), and execute the required table modification
***************
*** 684,692 **** ExecModifyTable(ModifyTableState *node)
node->mt_whichplan++;
if (node->mt_whichplan < node->mt_nplans)
{
- estate->es_result_relation_info++;
subplanstate = node->mt_plans[node->mt_whichplan];
! junkfilter = estate->es_result_relation_info->ri_junkFilter;
EvalPlanQualSetPlan(&node->mt_epqstate, subplanstate->plan);
continue;
}
--- 677,685 ----
node->mt_whichplan++;
if (node->mt_whichplan < node->mt_nplans)
{
subplanstate = node->mt_plans[node->mt_whichplan];
! resultRelInfo = node->resultRelInfo + node->mt_whichplan;
! junkfilter = resultRelInfo->ri_junkFilter;
EvalPlanQualSetPlan(&node->mt_epqstate, subplanstate->plan);
continue;
}
***************
*** 728,741 **** ExecModifyTable(ModifyTableState *node)
switch (operation)
{
case CMD_INSERT:
! slot = ExecInsert(slot, planSlot, estate);
break;
case CMD_UPDATE:
! slot = ExecUpdate(tupleid, slot, planSlot,
&node->mt_epqstate, estate);
break;
case CMD_DELETE:
! slot = ExecDelete(tupleid, planSlot,
&node->mt_epqstate, estate);
break;
default:
--- 721,737 ----
switch (operation)
{
case CMD_INSERT:
! slot = ExecInsert(resultRelInfo,
! slot, planSlot, estate);
break;
case CMD_UPDATE:
! slot = ExecUpdate(resultRelInfo,
! tupleid, slot, planSlot,
&node->mt_epqstate, estate);
break;
case CMD_DELETE:
! slot = ExecDelete(resultRelInfo,
! tupleid, planSlot,
&node->mt_epqstate, estate);
break;
default:
***************
*** 748,762 **** ExecModifyTable(ModifyTableState *node)
* the work on next call.
*/
if (slot)
- {
- estate->es_result_relation_info = NULL;
return slot;
- }
}
- /* Reset es_result_relation_info before exiting */
- estate->es_result_relation_info = NULL;
-
/*
* We're done, but fire AFTER STATEMENT triggers before exiting.
*/
--- 744,752 ----
***************
*** 803,827 **** ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
mtstate->mt_plans = (PlanState **) palloc0(sizeof(PlanState *) * nplans);
mtstate->mt_nplans = nplans;
mtstate->operation = operation;
/* set up epqstate with dummy subplan pointer for the moment */
EvalPlanQualInit(&mtstate->mt_epqstate, estate, NULL, node->epqParam);
mtstate->fireBSTriggers = true;
- /* For the moment, assume our targets are exactly the global result rels */
-
/*
* call ExecInitNode on each of the plans to be executed and save the
* results into the array "mt_plans". Note we *must* set
* estate->es_result_relation_info correctly while we initialize each
* sub-plan; ExecContextForcesOids depends on that!
*/
- estate->es_result_relation_info = estate->es_result_relations;
i = 0;
foreach(l, node->plans)
{
subplan = (Plan *) lfirst(l);
mtstate->mt_plans[i] = ExecInitNode(subplan, estate, eflags);
! estate->es_result_relation_info++;
i++;
}
estate->es_result_relation_info = NULL;
--- 793,831 ----
mtstate->mt_plans = (PlanState **) palloc0(sizeof(PlanState *) * nplans);
mtstate->mt_nplans = nplans;
mtstate->operation = operation;
+ mtstate->resultRelIndex = node->resultRelIndex;
+ mtstate->resultRelInfo = estate->es_result_relations + node->resultRelIndex;
+
/* set up epqstate with dummy subplan pointer for the moment */
EvalPlanQualInit(&mtstate->mt_epqstate, estate, NULL, node->epqParam);
mtstate->fireBSTriggers = true;
/*
* call ExecInitNode on each of the plans to be executed and save the
* results into the array "mt_plans". Note we *must* set
* estate->es_result_relation_info correctly while we initialize each
* sub-plan; ExecContextForcesOids depends on that!
*/
i = 0;
+ resultRelInfo = mtstate->resultRelInfo;
foreach(l, node->plans)
{
subplan = (Plan *) lfirst(l);
+
+ /*
+ * If there are indices on the result relation, open them and save
+ * descriptors in the result relation info, so that we can add new index
+ * entries for the tuples we add/update. We need not do this for a
+ * DELETE, however, since deletion doesn't affect indexes.
+ */
+ if (resultRelInfo->ri_RelationDesc->rd_rel->relhasindex &&
+ operation != CMD_DELETE)
+ ExecOpenIndices(resultRelInfo);
+
+ estate->es_result_relation_info = resultRelInfo;
mtstate->mt_plans[i] = ExecInitNode(subplan, estate, eflags);
!
! resultRelInfo++;
i++;
}
estate->es_result_relation_info = NULL;
***************
*** 858,865 **** ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
/*
* Build a projection for each result rel.
*/
! Assert(list_length(node->returningLists) == estate->es_num_result_relations);
! resultRelInfo = estate->es_result_relations;
foreach(l, node->returningLists)
{
List *rlist = (List *) lfirst(l);
--- 862,868 ----
/*
* Build a projection for each result rel.
*/
! resultRelInfo = mtstate->resultRelInfo;
foreach(l, node->returningLists)
{
List *rlist = (List *) lfirst(l);
***************
*** 958,964 **** ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
if (junk_filter_needed)
{
! resultRelInfo = estate->es_result_relations;
for (i = 0; i < nplans; i++)
{
JunkFilter *j;
--- 961,967 ----
if (junk_filter_needed)
{
! resultRelInfo = mtstate->resultRelInfo;
for (i = 0; i < nplans; i++)
{
JunkFilter *j;
***************
*** 987,993 **** ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
else
{
if (operation == CMD_INSERT)
! ExecCheckPlanOutput(estate->es_result_relations->ri_RelationDesc,
subplan->targetlist);
}
}
--- 990,996 ----
else
{
if (operation == CMD_INSERT)
! ExecCheckPlanOutput(mtstate->resultRelInfo->ri_RelationDesc,
subplan->targetlist);
}
}
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
***************
*** 84,89 **** _copyPlannedStmt(PlannedStmt *from)
--- 84,90 ----
COPY_NODE_FIELD(resultRelations);
COPY_NODE_FIELD(utilityStmt);
COPY_NODE_FIELD(intoClause);
+ COPY_SCALAR_FIELD(hasWritableCtes);
COPY_NODE_FIELD(subplans);
COPY_BITMAPSET_FIELD(rewindPlanIDs);
COPY_NODE_FIELD(rowMarks);
***************
*** 172,177 **** _copyModifyTable(ModifyTable *from)
--- 173,179 ----
*/
COPY_SCALAR_FIELD(operation);
COPY_NODE_FIELD(resultRelations);
+ COPY_SCALAR_FIELD(resultRelIndex);
COPY_NODE_FIELD(plans);
COPY_NODE_FIELD(returningLists);
COPY_NODE_FIELD(rowMarks);
*** a/src/backend/nodes/nodeFuncs.c
--- b/src/backend/nodes/nodeFuncs.c
***************
*** 2383,2388 **** bool
--- 2383,2432 ----
return true;
}
break;
+ case T_InsertStmt:
+ {
+ InsertStmt *stmt = (InsertStmt *) node;
+
+ if (walker(stmt->relation, context))
+ return true;
+ if (walker(stmt->cols, context))
+ return true;
+ if (walker(stmt->selectStmt, context))
+ return true;
+ if (walker(stmt->returningList, context))
+ return true;
+ }
+ break;
+ case T_UpdateStmt:
+ {
+ UpdateStmt *stmt = (UpdateStmt *) node;
+
+ if (walker(stmt->relation, context))
+ return true;
+ if (walker(stmt->targetList, context))
+ return true;
+ if (walker(stmt->whereClause, context))
+ return true;
+ if (walker(stmt->fromClause, context))
+ return true;
+ if (walker(stmt->returningList, context))
+ return true;
+ }
+ break;
+ case T_DeleteStmt:
+ {
+ DeleteStmt *stmt = (DeleteStmt *) node;
+
+ if (walker(stmt->relation, context))
+ return true;
+ if (walker(stmt->usingClause, context))
+ return true;
+ if (walker(stmt->whereClause, context))
+ return true;
+ if (walker(stmt->returningList, context))
+ return true;
+ }
+ break;
case T_A_Expr:
{
A_Expr *expr = (A_Expr *) node;
*** a/src/backend/nodes/outfuncs.c
--- b/src/backend/nodes/outfuncs.c
***************
*** 327,332 **** _outModifyTable(StringInfo str, ModifyTable *node)
--- 327,333 ----
WRITE_ENUM_FIELD(operation, CmdType);
WRITE_NODE_FIELD(resultRelations);
+ WRITE_INT_FIELD(resultRelIndex);
WRITE_NODE_FIELD(plans);
WRITE_NODE_FIELD(returningLists);
WRITE_NODE_FIELD(rowMarks);
***************
*** 1529,1534 **** _outPlannerGlobal(StringInfo str, PlannerGlobal *node)
--- 1530,1536 ----
WRITE_NODE_FIELD(finalrowmarks);
WRITE_NODE_FIELD(relationOids);
WRITE_NODE_FIELD(invalItems);
+ WRITE_NODE_FIELD(resultRelations);
WRITE_UINT_FIELD(lastPHId);
WRITE_BOOL_FIELD(transientPlan);
}
***************
*** 1543,1549 **** _outPlannerInfo(StringInfo str, PlannerInfo *node)
WRITE_NODE_FIELD(glob);
WRITE_UINT_FIELD(query_level);
WRITE_NODE_FIELD(join_rel_list);
- WRITE_NODE_FIELD(resultRelations);
WRITE_NODE_FIELD(init_plans);
WRITE_NODE_FIELD(cte_plan_ids);
WRITE_NODE_FIELD(eq_classes);
--- 1545,1550 ----
*** a/src/backend/optimizer/plan/createplan.c
--- b/src/backend/optimizer/plan/createplan.c
***************
*** 3759,3768 **** make_modifytable(CmdType operation, List *resultRelations,
double total_size;
ListCell *subnode;
- Assert(list_length(resultRelations) == list_length(subplans));
- Assert(returningLists == NIL ||
- list_length(resultRelations) == list_length(returningLists));
-
/*
* Compute cost as sum of subplan costs.
*/
--- 3759,3764 ----
*** a/src/backend/optimizer/plan/planner.c
--- b/src/backend/optimizer/plan/planner.c
***************
*** 160,165 **** standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
--- 160,167 ----
glob->finalrowmarks = NIL;
glob->relationOids = NIL;
glob->invalItems = NIL;
+ glob->hasWritableCtes = false;
+ glob->resultRelations = NIL;
glob->lastPHId = 0;
glob->transientPlan = false;
***************
*** 237,245 **** standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
result->transientPlan = glob->transientPlan;
result->planTree = top_plan;
result->rtable = glob->finalrtable;
! result->resultRelations = root->resultRelations;
result->utilityStmt = parse->utilityStmt;
result->intoClause = parse->intoClause;
result->subplans = glob->subplans;
result->rewindPlanIDs = glob->rewindPlanIDs;
result->rowMarks = glob->finalrowmarks;
--- 239,248 ----
result->transientPlan = glob->transientPlan;
result->planTree = top_plan;
result->rtable = glob->finalrtable;
! result->resultRelations = glob->resultRelations;
result->utilityStmt = parse->utilityStmt;
result->intoClause = parse->intoClause;
+ result->hasWritableCtes = glob->hasWritableCtes;
result->subplans = glob->subplans;
result->rewindPlanIDs = glob->rewindPlanIDs;
result->rowMarks = glob->finalrowmarks;
***************
*** 541,547 **** subquery_planner(PlannerGlobal *glob, Query *parse,
rowMarks = root->rowMarks;
plan = (Plan *) make_modifytable(parse->commandType,
! copyObject(root->resultRelations),
list_make1(plan),
returningLists,
rowMarks,
--- 544,550 ----
rowMarks = root->rowMarks;
plan = (Plan *) make_modifytable(parse->commandType,
! list_make1_int(parse->resultRelation),
list_make1(plan),
returningLists,
rowMarks,
***************
*** 706,718 **** inheritance_planner(PlannerInfo *root)
Query *parse = root->parse;
int parentRTindex = parse->resultRelation;
List *subplans = NIL;
- List *resultRelations = NIL;
List *returningLists = NIL;
List *rtable = NIL;
List *rowMarks;
List *tlist;
PlannerInfo subroot;
ListCell *l;
foreach(l, root->append_rel_list)
{
--- 709,721 ----
Query *parse = root->parse;
int parentRTindex = parse->resultRelation;
List *subplans = NIL;
List *returningLists = NIL;
List *rtable = NIL;
List *rowMarks;
List *tlist;
PlannerInfo subroot;
ListCell *l;
+ List *resultRelations = NIL;
foreach(l, root->append_rel_list)
{
***************
*** 772,779 **** inheritance_planner(PlannerInfo *root)
}
}
- root->resultRelations = resultRelations;
-
/* Mark result as unordered (probably unnecessary) */
root->query_pathkeys = NIL;
--- 775,780 ----
***************
*** 783,789 **** inheritance_planner(PlannerInfo *root)
*/
if (subplans == NIL)
{
- root->resultRelations = list_make1_int(parentRTindex);
/* although dummy, it must have a valid tlist for executor */
tlist = preprocess_targetlist(root, parse->targetList);
return (Plan *) make_result(root,
--- 784,789 ----
***************
*** 818,824 **** inheritance_planner(PlannerInfo *root)
/* And last, tack on a ModifyTable node to do the UPDATE/DELETE work */
return (Plan *) make_modifytable(parse->commandType,
! copyObject(root->resultRelations),
subplans,
returningLists,
rowMarks,
--- 818,824 ----
/* And last, tack on a ModifyTable node to do the UPDATE/DELETE work */
return (Plan *) make_modifytable(parse->commandType,
! resultRelations,
subplans,
returningLists,
rowMarks,
***************
*** 1667,1678 **** grouping_planner(PlannerInfo *root, double tuple_fraction)
count_est);
}
- /* Compute result-relations list if needed */
- if (parse->resultRelation)
- root->resultRelations = list_make1_int(parse->resultRelation);
- else
- root->resultRelations = NIL;
-
/*
* Return the actual output ordering in query_pathkeys for possible use by
* an outer query level.
--- 1667,1672 ----
*** a/src/backend/optimizer/plan/setrefs.c
--- b/src/backend/optimizer/plan/setrefs.c
***************
*** 516,521 **** set_plan_refs(PlannerGlobal *glob, Plan *plan, int rtoffset)
--- 516,525 ----
(Plan *) lfirst(l),
rtoffset);
}
+
+ splan->resultRelIndex = list_length(glob->resultRelations);
+ glob->resultRelations = list_concat(glob->resultRelations,
+ splan->resultRelations);
}
break;
case T_Append:
*** a/src/backend/optimizer/plan/subselect.c
--- b/src/backend/optimizer/plan/subselect.c
***************
*** 873,888 **** SS_process_ctes(PlannerInfo *root)
Bitmapset *tmpset;
int paramid;
Param *prm;
/*
! * Ignore CTEs that are not actually referenced anywhere.
*/
! if (cte->cterefcount == 0)
{
/* Make a dummy entry in cte_plan_ids */
root->cte_plan_ids = lappend_int(root->cte_plan_ids, -1);
continue;
}
/*
* Copy the source Query node. Probably not necessary, but let's keep
--- 873,905 ----
Bitmapset *tmpset;
int paramid;
Param *prm;
+ CmdType cmdType = ((Query *) cte->ctequery)->commandType;
/*
! * Ignore SELECT CTEs that are not actually referenced anywhere.
*/
! if (cte->cterefcount == 0 && cmdType == CMD_SELECT)
{
/* Make a dummy entry in cte_plan_ids */
root->cte_plan_ids = lappend_int(root->cte_plan_ids, -1);
continue;
}
+ else if (cmdType != CMD_SELECT)
+ {
+ /* We don't know reference counts until here */
+ if (cte->cterefcount > 0 &&
+ ((Query *) cte->ctequery)->returningList == NIL)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("INSERT/UPDATE/DELETE without RETURNING is only allowed inside a non-referenced
CTE")));
+ }
+
+ if (root->query_level > 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("INSERT/UPDATE/DELETE inside a CTE is only allowed on the top level")));
+ }
/*
* Copy the source Query node. Probably not necessary, but let's keep
***************
*** 899,904 **** SS_process_ctes(PlannerInfo *root)
--- 916,924 ----
cte->cterecursive, 0.0,
&subroot);
+ if (subroot->parse->commandType != CMD_SELECT)
+ root->glob->hasWritableCtes = true;
+
/*
* Make a SubPlan node for it. This is just enough unlike
* build_subplan that we can't share code.
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 7374,7379 **** common_table_expr: name opt_name_list AS select_with_parens
--- 7374,7406 ----
n->location = @1;
$$ = (Node *) n;
}
+ | name opt_name_list AS '(' InsertStmt ')'
+ {
+ CommonTableExpr *n = makeNode(CommonTableExpr);
+ n->ctename = $1;
+ n->aliascolnames = $2;
+ n->ctequery = $5;
+ n->location = @1;
+ $$ = (Node *) n;
+ }
+ | name opt_name_list AS '(' UpdateStmt ')'
+ {
+ CommonTableExpr *n = makeNode(CommonTableExpr);
+ n->ctename = $1;
+ n->aliascolnames = $2;
+ n->ctequery = $5;
+ n->location = @1;
+ $$ = (Node *) n;
+ }
+ | name opt_name_list AS '(' DeleteStmt ')'
+ {
+ CommonTableExpr *n = makeNode(CommonTableExpr);
+ n->ctename = $1;
+ n->aliascolnames = $2;
+ n->ctequery = $5;
+ n->location = @1;
+ $$ = (Node *) n;
+ }
;
into_clause:
*** a/src/backend/parser/parse_cte.c
--- b/src/backend/parser/parse_cte.c
***************
*** 18,23 ****
--- 18,24 ----
#include "nodes/nodeFuncs.h"
#include "parser/analyze.h"
#include "parser/parse_cte.h"
+ #include "nodes/plannodes.h"
#include "utils/builtins.h"
***************
*** 225,246 **** static void
analyzeCTE(ParseState *pstate, CommonTableExpr *cte)
{
Query *query;
!
! /* Analysis not done already */
! Assert(IsA(cte->ctequery, SelectStmt));
query = parse_sub_analyze(cte->ctequery, pstate, cte, false);
cte->ctequery = (Node *) query;
/*
* Check that we got something reasonable. Many of these conditions are
* impossible given restrictions of the grammar, but check 'em anyway.
! * (These are the same checks as in transformRangeSubselect.)
*/
! if (!IsA(query, Query) ||
! query->commandType != CMD_SELECT ||
! query->utilityStmt != NULL)
! elog(ERROR, "unexpected non-SELECT command in subquery in WITH");
if (query->intoClause)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
--- 226,250 ----
analyzeCTE(ParseState *pstate, CommonTableExpr *cte)
{
Query *query;
! List *cteList;
query = parse_sub_analyze(cte->ctequery, pstate, cte, false);
cte->ctequery = (Node *) query;
+ if (query->commandType == CMD_SELECT)
+ cteList = query->targetList;
+ else
+ cteList = query->returningList;
+
/*
* Check that we got something reasonable. Many of these conditions are
* impossible given restrictions of the grammar, but check 'em anyway.
! * Note, however, that we can't yet decice whether to allow
! * INSERT/UPDATE/DELETE without a RETURNING clause or not because we don't
! * know the refcount.
*/
! Assert(IsA(query, Query) && query->utilityStmt == NULL);
!
if (query->intoClause)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
***************
*** 251,257 **** analyzeCTE(ParseState *pstate, CommonTableExpr *cte)
if (!cte->cterecursive)
{
/* Compute the output column names/types if not done yet */
! analyzeCTETargetList(pstate, cte, query->targetList);
}
else
{
--- 255,261 ----
if (!cte->cterecursive)
{
/* Compute the output column names/types if not done yet */
! analyzeCTETargetList(pstate, cte, cteList);
}
else
{
***************
*** 269,275 **** analyzeCTE(ParseState *pstate, CommonTableExpr *cte)
lctyp = list_head(cte->ctecoltypes);
lctypmod = list_head(cte->ctecoltypmods);
varattno = 0;
! foreach(lctlist, query->targetList)
{
TargetEntry *te = (TargetEntry *) lfirst(lctlist);
Node *texpr;
--- 273,279 ----
lctyp = list_head(cte->ctecoltypes);
lctypmod = list_head(cte->ctecoltypmods);
varattno = 0;
! foreach(lctlist, cteList)
{
TargetEntry *te = (TargetEntry *) lfirst(lctlist);
Node *texpr;
*** a/src/backend/parser/parse_relation.c
--- b/src/backend/parser/parse_relation.c
***************
*** 24,29 ****
--- 24,30 ----
#include "funcapi.h"
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
+ #include "nodes/plannodes.h"
#include "parser/parsetree.h"
#include "parser/parse_relation.h"
#include "parser/parse_type.h"
*** a/src/backend/parser/parse_target.c
--- b/src/backend/parser/parse_target.c
***************
*** 314,323 **** markTargetListOrigin(ParseState *pstate, TargetEntry *tle,
{
CommonTableExpr *cte = GetCTEForRTE(pstate, rte, netlevelsup);
TargetEntry *ste;
/* should be analyzed by now */
Assert(IsA(cte->ctequery, Query));
! ste = get_tle_by_resno(((Query *) cte->ctequery)->targetList,
attnum);
if (ste == NULL || ste->resjunk)
elog(ERROR, "subquery %s does not have attribute %d",
--- 314,333 ----
{
CommonTableExpr *cte = GetCTEForRTE(pstate, rte, netlevelsup);
TargetEntry *ste;
+ List *cteList;
+ Query *ctequery;
/* should be analyzed by now */
Assert(IsA(cte->ctequery, Query));
!
! ctequery = (Query *) cte->ctequery;
!
! if (ctequery->commandType == CMD_SELECT)
! cteList = ctequery->targetList;
! else
! cteList = ctequery->returningList;
!
! ste = get_tle_by_resno(cteList,
attnum);
if (ste == NULL || ste->resjunk)
elog(ERROR, "subquery %s does not have attribute %d",
***************
*** 1345,1355 **** expandRecordVariable(ParseState *pstate, Var *var, int levelsup)
{
CommonTableExpr *cte = GetCTEForRTE(pstate, rte, netlevelsup);
TargetEntry *ste;
/* should be analyzed by now */
Assert(IsA(cte->ctequery, Query));
! ste = get_tle_by_resno(((Query *) cte->ctequery)->targetList,
! attnum);
if (ste == NULL || ste->resjunk)
elog(ERROR, "subquery %s does not have attribute %d",
rte->eref->aliasname, attnum);
--- 1355,1374 ----
{
CommonTableExpr *cte = GetCTEForRTE(pstate, rte, netlevelsup);
TargetEntry *ste;
+ List *cteList;
+ Query *ctequery;
/* should be analyzed by now */
Assert(IsA(cte->ctequery, Query));
!
! ctequery = (Query *) cte->ctequery;
!
! if (ctequery->commandType == CMD_SELECT)
! cteList = ctequery->targetList;
! else
! cteList = ctequery->returningList;
!
! ste = get_tle_by_resno(cteList, attnum);
if (ste == NULL || ste->resjunk)
elog(ERROR, "subquery %s does not have attribute %d",
rte->eref->aliasname, attnum);
***************
*** 1372,1378 **** expandRecordVariable(ParseState *pstate, Var *var, int levelsup)
levelsup++)
pstate = pstate->parentParseState;
mypstate.parentParseState = pstate;
! mypstate.p_rtable = ((Query *) cte->ctequery)->rtable;
/* don't bother filling the rest of the fake pstate */
return expandRecordVariable(&mypstate, (Var *) expr, 0);
--- 1391,1397 ----
levelsup++)
pstate = pstate->parentParseState;
mypstate.parentParseState = pstate;
! mypstate.p_rtable = ctequery->rtable;
/* don't bother filling the rest of the fake pstate */
return expandRecordVariable(&mypstate, (Var *) expr, 0);
*** a/src/backend/rewrite/rewriteHandler.c
--- b/src/backend/rewrite/rewriteHandler.c
***************
*** 1632,1637 **** RewriteQuery(Query *parsetree, List *rewrite_events)
--- 1632,1641 ----
bool returning = false;
Query *qual_product = NULL;
List *rewritten = NIL;
+ ListCell *lc;
+ CommonTableExpr *cte;
+ Query *ctequery;
+ List *newstuff;
/*
* If the statement is an update, insert or delete - fire rules on it.
***************
*** 1749,1755 **** RewriteQuery(Query *parsetree, List *rewrite_events)
foreach(n, product_queries)
{
Query *pt = (Query *) lfirst(n);
- List *newstuff;
newstuff = RewriteQuery(pt, rewrite_events);
rewritten = list_concat(rewritten, newstuff);
--- 1753,1758 ----
***************
*** 1804,1809 **** RewriteQuery(Query *parsetree, List *rewrite_events)
--- 1807,1861 ----
}
/*
+ * Rewrite DML statements inside CTEs. If there are any
+ * DO ALSO rules, they are added to the top level (there
+ * won't be any non-top-level CTEs with DML).
+ */
+ foreach(lc, parsetree->cteList)
+ {
+ cte = lfirst(lc);
+
+ ctequery = (Query *) cte->ctequery;
+
+ if (ctequery->commandType == CMD_SELECT)
+ continue;
+
+ newstuff = RewriteQuery(ctequery, NIL);
+
+ /*
+ * For UPDATE and DELETE, the actual query is
+ * added to the end of the list.
+ */
+ if (list_length(newstuff) > 1 &&
+ ctequery->commandType != CMD_INSERT)
+ {
+ ListCell *lc;
+ int n = 1;
+
+ foreach(lc, newstuff)
+ {
+ /*
+ * If this is the last one, don't add it to the results.
+ * Instead, update the query inside the CTE.
+ */
+ if (n == list_length(newstuff))
+ cte->ctequery = (Node *) lfirst(lc);
+ else
+ rewritten = lcons((void *) lfirst(lc), rewritten);
+
+ n++;
+ }
+
+ }
+ else
+ {
+ cte->ctequery = (Node *) linitial(newstuff);
+ rewritten = list_concat(rewritten,
+ list_delete_first(newstuff));
+ }
+ }
+
+ /*
* For INSERTs, the original query is done first; for UPDATE/DELETE, it is
* done last. This is needed because update and delete rule actions might
* not do anything if they are invoked after the update or delete is
*** a/src/backend/tcop/pquery.c
--- b/src/backend/tcop/pquery.c
***************
*** 293,298 **** ChoosePortalStrategy(List *stmts)
--- 293,299 ----
if (pstmt->canSetTag)
{
if (pstmt->commandType == CMD_SELECT &&
+ pstmt->hasWritableCtes == false &&
pstmt->utilityStmt == NULL &&
pstmt->intoClause == NULL)
return PORTAL_ONE_SELECT;
*** a/src/backend/utils/adt/ruleutils.c
--- b/src/backend/utils/adt/ruleutils.c
***************
*** 3858,3866 **** get_name_for_var_field(Var *var, int fieldno,
}
if (lc != NULL)
{
! Query *ctequery = (Query *) cte->ctequery;
! TargetEntry *ste = get_tle_by_resno(ctequery->targetList,
! attnum);
if (ste == NULL || ste->resjunk)
elog(ERROR, "subquery %s does not have attribute %d",
--- 3858,3873 ----
}
if (lc != NULL)
{
! Query *ctequery = (Query *) cte->ctequery;
! List *ctelist;
! TargetEntry *ste;
!
! if (ctequery->commandType != CMD_SELECT)
! ctelist = ctequery->returningList;
! else
! ctelist = ctequery->targetList;
!
! ste = get_tle_by_resno(ctelist, attnum);
if (ste == NULL || ste->resjunk)
elog(ERROR, "subquery %s does not have attribute %d",
*** a/src/include/executor/executor.h
--- b/src/include/executor/executor.h
***************
*** 319,325 **** extern void ExecCloseScanRelation(Relation scanrel);
extern void ExecOpenIndices(ResultRelInfo *resultRelInfo);
extern void ExecCloseIndices(ResultRelInfo *resultRelInfo);
! extern List *ExecInsertIndexTuples(TupleTableSlot *slot, ItemPointer tupleid,
EState *estate, bool is_vacuum_full);
extern void RegisterExprContextCallback(ExprContext *econtext,
--- 319,326 ----
extern void ExecOpenIndices(ResultRelInfo *resultRelInfo);
extern void ExecCloseIndices(ResultRelInfo *resultRelInfo);
! extern List *ExecInsertIndexTuples(ResultRelInfo *resultRelInfo,
! TupleTableSlot *slot, ItemPointer tupleid,
EState *estate, bool is_vacuum_full);
extern void RegisterExprContextCallback(ExprContext *econtext,
*** a/src/include/nodes/execnodes.h
--- b/src/include/nodes/execnodes.h
***************
*** 1021,1026 **** typedef struct ModifyTableState
--- 1021,1028 ----
PlanState **mt_plans; /* subplans (one per target rel) */
int mt_nplans; /* number of plans in the array */
int mt_whichplan; /* which one is being executed (0..n-1) */
+ int resultRelIndex;
+ ResultRelInfo *resultRelInfo;
EPQState mt_epqstate; /* for evaluating EvalPlanQual rechecks */
bool fireBSTriggers; /* do we need to fire stmt triggers? */
} ModifyTableState;
*** a/src/include/nodes/plannodes.h
--- b/src/include/nodes/plannodes.h
***************
*** 55,60 **** typedef struct PlannedStmt
--- 55,62 ----
IntoClause *intoClause; /* target for SELECT INTO / CREATE TABLE AS */
+ bool hasWritableCtes;
+
List *subplans; /* Plan trees for SubPlan expressions */
Bitmapset *rewindPlanIDs; /* indices of subplans that require REWIND */
***************
*** 165,170 **** typedef struct ModifyTable
--- 167,173 ----
Plan plan;
CmdType operation; /* INSERT, UPDATE, or DELETE */
List *resultRelations; /* integer list of RT indexes */
+ int resultRelIndex;
List *plans; /* plan(s) producing source data */
List *returningLists; /* per-target-table RETURNING tlists */
List *rowMarks; /* PlanRowMarks (non-locking only) */
*** a/src/include/nodes/relation.h
--- b/src/include/nodes/relation.h
***************
*** 80,85 **** typedef struct PlannerGlobal
--- 80,89 ----
List *invalItems; /* other dependencies, as PlanInvalItems */
+ bool hasWritableCtes;/* is there an (INSERT|UPDATE|DELETE) .. RETURNING inside a CTE? */
+
+ List *resultRelations;/* list of result relations */
+
Index lastPHId; /* highest PlaceHolderVar ID assigned */
bool transientPlan; /* redo plan when TransactionXmin changes? */
***************
*** 142,149 **** typedef struct PlannerInfo
List *join_rel_list; /* list of join-relation RelOptInfos */
struct HTAB *join_rel_hash; /* optional hashtable for join relations */
- List *resultRelations; /* integer list of RT indexes, or NIL */
-
List *init_plans; /* init SubPlans for query */
List *cte_plan_ids; /* per-CTE-item list of subplan IDs */
--- 146,151 ----
*** a/src/test/regress/expected/with.out
--- b/src/test/regress/expected/with.out
***************
*** 1026,1028 **** SELECT * FROM t;
--- 1026,1110 ----
10
(55 rows)
+ --
+ -- Writeable CTEs with RETURNING
+ --
+ WITH t AS (
+ INSERT INTO y
+ VALUES
+ (11),
+ (12),
+ (13),
+ (14),
+ (15),
+ (16),
+ (17),
+ (18),
+ (19),
+ (20)
+ RETURNING *
+ )
+ SELECT * FROM t;
+ a
+ ----
+ 11
+ 12
+ 13
+ 14
+ 15
+ 16
+ 17
+ 18
+ 19
+ 20
+ (10 rows)
+
+ WITH t AS (
+ UPDATE y
+ SET a=a+1
+ RETURNING *
+ )
+ SELECT * FROM t;
+ a
+ ----
+ 2
+ 3
+ 4
+ 5
+ 6
+ 7
+ 8
+ 9
+ 10
+ 11
+ 12
+ 13
+ 14
+ 15
+ 16
+ 17
+ 18
+ 19
+ 20
+ 21
+ (20 rows)
+
+ WITH t AS (
+ DELETE FROM y
+ WHERE a <= 10
+ RETURNING *
+ )
+ SELECT * FROM t;
+ a
+ ----
+ 2
+ 3
+ 4
+ 5
+ 6
+ 7
+ 8
+ 9
+ 10
+ (9 rows)
+
*** a/src/test/regress/sql/with.sql
--- b/src/test/regress/sql/with.sql
***************
*** 500,502 **** WITH RECURSIVE t(j) AS (
--- 500,537 ----
SELECT j+1 FROM t WHERE j < 10
)
SELECT * FROM t;
+
+ --
+ -- Writeable CTEs with RETURNING
+ --
+
+ WITH t AS (
+ INSERT INTO y
+ VALUES
+ (11),
+ (12),
+ (13),
+ (14),
+ (15),
+ (16),
+ (17),
+ (18),
+ (19),
+ (20)
+ RETURNING *
+ )
+ SELECT * FROM t;
+
+ WITH t AS (
+ UPDATE y
+ SET a=a+1
+ RETURNING *
+ )
+ SELECT * FROM t;
+
+ WITH t AS (
+ DELETE FROM y
+ WHERE a <= 10
+ RETURNING *
+ )
+ SELECT * FROM t;
On Sun, Nov 15, 2009 at 14:27, Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi> wrote: > I wrote: >> >> Attached is the latest version of this patch. Find attached a incremental diff with the following changes: -get rid of operation argument to InitResultRelInfo, its unused now -add some asserts to make sure places we use subplanstate now that it can be null (*note* AFAICT its a cant happen, but it made me nervous hence the Asserts) -remove unneeded plannodes.h includes -minor whitespace fix Other comments: You have an "XXX we should probably update the snapshot a bit differently". Any plans on that? Thats quite a bit of new code in ExecutePlan, worth splitting into its own function? Also, after reading through the previous threads; it was not immediately obvious that you dealt with http://archives.postgresql.org/pgsql-hackers/2009-10/msg00566.php by only allowing selects or values at the top level of with. Find below the standard review boilerplate from http://wiki.postgresql.org/wiki/Reviewing_a_Patch Summary: looks ready for a commiter to me after above comments are addressed. Submission review: *Is the patch in context diff format? Yes * Does it apply cleanly to the current CVS HEAD? Yes, with fuzz * Does it include reasonable tests, necessary doc patches, etc? Yes Usability review: Read what the patch is supposed to do, and consider: * Does the patch actually implement that? Yes * Do we want that? Yes * Do we already have it? No * Does it follow SQL spec, or the community-agreed behavior? Yes * Does it include pg_dump support (if applicable)? N/A * Are there dangers? No * Have all the bases been covered? All the ones I can see Feature test: Apply the patch, compile it and test: * Does the feature work as advertised? Yes * Are there corner cases the author has failed to consider? Not that I could trigger * Are there any assertion failures or crashes? No o Review should be done with the configure options --enable-cassert and --enable-debug turned on; Yes Performance review: *Does the patch slow down simple tests: No *If it claims to improve performance, does it? N/A *Does it slow down other things No Coding review: Read the changes to the code in detail and consider: * Does it follow the project coding guidelines? Yes * Are there portability issues? No * Will it work on Windows/BSD etc? Yes * Are the comments sufficient and accurate? Yes * Does it do what it says, correctly? Yes * Does it produce compiler warnings? No * Can you make it crash? No Architecture review: Consider the changes to the code in the context of the project as a whole: * Is everything done in a way that fits together coherently with other features/modules? I think so. * Are there interdependencies than can cause problems? No
Attachment
Alex Hunsaker wrote: > Find attached a incremental diff with the following changes: > -get rid of operation argument to InitResultRelInfo, its unused now Missed that one. Thanks. > -add some asserts to make sure places we use subplanstate now that it > can be null > (*note* AFAICT its a cant happen, but it made me nervous hence the Asserts) Indeed, it shouldn't happen, but this seems like a decent precaution. > Other comments: > You have an "XXX we should probably update the snapshot a bit > differently". Any plans on that? I've looked into that, but couldn't find a better way. Maybe I should take out my scuba gear for a new dive into the snapshot code.. > Thats quite a bit of new code in ExecutePlan, worth splitting into its > own function? Could probably be. > Also, after reading through the previous threads; it was not > immediately obvious that you dealt with > http://archives.postgresql.org/pgsql-hackers/2009-10/msg00566.php by > only allowing selects or values at the top level of with. This is actually just something missing from the current implementation. The relevant posts are in the same thread: http://archives.postgresql.org/pgsql-hackers/2009-10/msg00558.php and the two follow-ups. The comment in ExecutePlan() is a bit misleading. What I meant is that we don't call GetCurrentCommandId() in standard_ExecutorStart(). Instead we get a new CID for every CTE with INSERT/UPDATE/DELETE. That comment tried to point out the fact that this strategy could fail if there was a non-SELECT query as the top-level statement because we wouldn't increment the CID after the last CTE. I did it this way because it works well for the purposes of this patch and I didn't see an obvious way to determine whether we need a new CID for the top-level statement or not. I'll send an updated patch in a couple of days. Regards, Marko Tiikkaja
On Tue, Nov 17, 2009 at 03:54, Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi> wrote: >> Also, after reading through the previous threads; it was not >> immediately obvious that you dealt with >> http://archives.postgresql.org/pgsql-hackers/2009-10/msg00566.php by >> only allowing selects or values at the top level of with. > > This is actually just something missing from the current implementation. > The relevant posts are in the same thread: > http://archives.postgresql.org/pgsql-hackers/2009-10/msg00558.php and > the two follow-ups. The comment in ExecutePlan() is a bit misleading. Hrm I tried the various forms of: with x as ( ... ) insert/update/delete and could not get any of them to work. So I assumed the comment about only SELECT and values were allowed was correct. Maybe a function that does an insert or update at the top level could get it to break? > What I meant is that we don't call GetCurrentCommandId() in > standard_ExecutorStart(). Instead we get a new CID for every CTE with > INSERT/UPDATE/DELETE. That comment tried to point out the fact that > this strategy could fail if there was a non-SELECT query as the > top-level statement because we wouldn't increment the CID after the last > CTE. Right... Which I thought was more or less the recommendation? Guess Ill have to go re-read that discussion. > I did it this way because it works well for the purposes of this > patch and I didn't see an obvious way to determine whether we need a new > CID for the top-level statement or not. > > I'll send an updated patch in a couple of days. Peachy.
Hi,
Sorry for the delay, I've been very busy for the last two weeks.
Attached is the latest version of the patch.
The snapshot update code is still the same, I have no good idea what, if
anything, should be done to it. In addition to that, I decided to keep
the code in ExecutePlan() as it was in the last patch. I tried to make
the comments a bit more clear to avoid confusion.
Regards,
Marko Tiikkaja
*** a/doc/src/sgml/queries.sgml
--- b/doc/src/sgml/queries.sgml
***************
*** 1499,1505 **** SELECT 3, 'three';
<synopsis>
SELECT <replaceable>select_list</replaceable> FROM <replaceable>table_expression</replaceable>
</synopsis>
! and can appear anywhere a <literal>SELECT</> can. For example, you can
use it as part of a <literal>UNION</>, or attach a
<replaceable>sort_specification</replaceable> (<literal>ORDER BY</>,
<literal>LIMIT</>, and/or <literal>OFFSET</>) to it. <literal>VALUES</>
--- 1499,1505 ----
<synopsis>
SELECT <replaceable>select_list</replaceable> FROM <replaceable>table_expression</replaceable>
</synopsis>
! and can appear anywhere a <literal>SELECT</literal> can. For example, you can
use it as part of a <literal>UNION</>, or attach a
<replaceable>sort_specification</replaceable> (<literal>ORDER BY</>,
<literal>LIMIT</>, and/or <literal>OFFSET</>) to it. <literal>VALUES</>
***************
*** 1529,1538 **** SELECT <replaceable>select_list</replaceable> FROM <replaceable>table_expression
</indexterm>
<para>
! <literal>WITH</> provides a way to write subqueries for use in a larger
! <literal>SELECT</> query. The subqueries can be thought of as defining
! temporary tables that exist just for this query. One use of this feature
! is to break down complicated queries into simpler parts. An example is:
<programlisting>
WITH regional_sales AS (
--- 1529,1539 ----
</indexterm>
<para>
! <literal>WITH</> provides a way to write subqueries for use in a
! larger query. The subqueries can be thought of as defining
! temporary tables that exist just for this query. One use of this
! feature is to break down complicated queries into simpler parts.
! An example is:
<programlisting>
WITH regional_sales AS (
***************
*** 1560,1565 **** GROUP BY region, product;
--- 1561,1590 ----
</para>
<para>
+ A <literal>WITH</literal> clause can also have an
+ <literal>INSERT</literal>, <literal>UPDATE</literal> or
+ <literal>DELETE</literal> (each optionally with a
+ <literal>RETURNING</literal> clause) statement in it. The example below
+ moves rows from the main table, foo_log into a partition,
+ foo_log_200910.
+
+ <programlisting>
+ WITH rows AS (
+ DELETE FROM ONLY foo_log
+ WHERE
+ foo_date >= '2009-10-01' AND
+ foo_date < '2009-11-01'
+ RETURNING *
+ ), t AS (
+ INSERT INTO foo_log_200910
+ SELECT * FROM rows
+ )
+ VALUES(true);
+ </programlisting>
+
+ </para>
+
+ <para>
The optional <literal>RECURSIVE</> modifier changes <literal>WITH</>
from a mere syntactic convenience into a feature that accomplishes
things not otherwise possible in standard SQL. Using
*** a/doc/src/sgml/ref/select.sgml
--- b/doc/src/sgml/ref/select.sgml
***************
*** 58,64 **** SELECT [ ALL | DISTINCT [ ON ( <replaceable class="parameter">expression</replac
<phrase>and <replaceable class="parameter">with_query</replaceable> is:</phrase>
! <replaceable class="parameter">with_query_name</replaceable> [ ( <replaceable
class="parameter">column_name</replaceable>[, ...] ) ] AS ( <replaceable class="parameter">select</replaceable> )
TABLE { [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ] | <replaceable
class="parameter">with_query_name</replaceable>}
</synopsis>
--- 58,64 ----
<phrase>and <replaceable class="parameter">with_query</replaceable> is:</phrase>
! <replaceable class="parameter">with_query_name</replaceable> [ ( <replaceable
class="parameter">column_name</replaceable>[, ...] ) ] AS ( <replaceable class="parameter">select</replaceable> |
(<replaceableclass="parameter">insert</replaceable> | <replaceable class="parameter">update</replaceable> |
<replaceableclass="parameter">delete</replaceable> [ RETURNING...]))
TABLE { [ ONLY ] <replaceable class="parameter">table_name</replaceable> [ * ] | <replaceable
class="parameter">with_query_name</replaceable>}
</synopsis>
*** a/src/backend/commands/copy.c
--- b/src/backend/commands/copy.c
***************
*** 2165,2171 **** CopyFrom(CopyState cstate)
heap_insert(cstate->rel, tuple, mycid, hi_options, bistate);
if (resultRelInfo->ri_NumIndices > 0)
! recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self),
estate, false);
/* AFTER ROW INSERT Triggers */
--- 2165,2172 ----
heap_insert(cstate->rel, tuple, mycid, hi_options, bistate);
if (resultRelInfo->ri_NumIndices > 0)
! recheckIndexes = ExecInsertIndexTuples(resultRelInfo,
! slot, &(tuple->t_self),
estate, false);
/* AFTER ROW INSERT Triggers */
*** a/src/backend/commands/portalcmds.c
--- b/src/backend/commands/portalcmds.c
***************
*** 48,53 **** PerformCursorOpen(PlannedStmt *stmt, ParamListInfo params,
--- 48,58 ----
Portal portal;
MemoryContext oldContext;
+ if (stmt->hasWritableCtes)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("Non-SELECT cursors are not implemented")));
+
if (cstmt == NULL || !IsA(cstmt, DeclareCursorStmt))
elog(ERROR, "PerformCursorOpen called for non-cursor query");
*** a/src/backend/commands/tablecmds.c
--- b/src/backend/commands/tablecmds.c
***************
*** 925,931 **** ExecuteTruncate(TruncateStmt *stmt)
InitResultRelInfo(resultRelInfo,
rel,
0, /* dummy rangetable index */
- CMD_DELETE, /* don't need any index info */
false);
resultRelInfo++;
}
--- 925,930 ----
*** a/src/backend/commands/vacuum.c
--- b/src/backend/commands/vacuum.c
***************
*** 3099,3105 **** move_chain_tuple(Relation rel,
if (ec->resultRelInfo->ri_NumIndices > 0)
{
ExecStoreTuple(&newtup, ec->slot, InvalidBuffer, false);
! ExecInsertIndexTuples(ec->slot, &(newtup.t_self), ec->estate, true);
ResetPerTupleExprContext(ec->estate);
}
}
--- 3099,3105 ----
if (ec->resultRelInfo->ri_NumIndices > 0)
{
ExecStoreTuple(&newtup, ec->slot, InvalidBuffer, false);
! ExecInsertIndexTuples(ec->resultRelInfo, ec->slot, &(newtup.t_self), ec->estate, true);
ResetPerTupleExprContext(ec->estate);
}
}
***************
*** 3225,3231 **** move_plain_tuple(Relation rel,
if (ec->resultRelInfo->ri_NumIndices > 0)
{
ExecStoreTuple(&newtup, ec->slot, InvalidBuffer, false);
! ExecInsertIndexTuples(ec->slot, &(newtup.t_self), ec->estate, true);
ResetPerTupleExprContext(ec->estate);
}
}
--- 3225,3231 ----
if (ec->resultRelInfo->ri_NumIndices > 0)
{
ExecStoreTuple(&newtup, ec->slot, InvalidBuffer, false);
! ExecInsertIndexTuples(ec->resultRelInfo, ec->slot, &(newtup.t_self), ec->estate, true);
ResetPerTupleExprContext(ec->estate);
}
}
*** a/src/backend/commands/view.c
--- b/src/backend/commands/view.c
***************
*** 394,399 **** DefineView(ViewStmt *stmt, const char *queryString)
--- 394,400 ----
Query *viewParse;
Oid viewOid;
RangeVar *view;
+ ListCell *lc;
/*
* Run parse analysis to convert the raw parse tree to a Query. Note this
***************
*** 412,417 **** DefineView(ViewStmt *stmt, const char *queryString)
--- 413,430 ----
viewParse->commandType != CMD_SELECT)
elog(ERROR, "unexpected parse analysis result");
+ /* .. but it doesn't check for DML inside CTEs */
+ foreach(lc, viewParse->cteList)
+ {
+ CommonTableExpr *cte;
+
+ cte = (CommonTableExpr *) lfirst(lc);
+ if (((Query *) cte->ctequery)->commandType != CMD_SELECT)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("INSERT/UPDATE/DELETE inside a CTE not allowed in a view definition")));
+ }
+
/*
* If a list of column names was given, run through and insert these into
* the actual query tree. - thomas 2000-03-08
*** a/src/backend/executor/execMain.c
--- b/src/backend/executor/execMain.c
***************
*** 665,671 **** InitPlan(QueryDesc *queryDesc, int eflags)
InitResultRelInfo(resultRelInfo,
resultRelation,
resultRelationIndex,
- operation,
estate->es_instrument);
resultRelInfo++;
}
--- 665,670 ----
***************
*** 858,864 **** void
InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation resultRelationDesc,
Index resultRelationIndex,
- CmdType operation,
bool doInstrument)
{
/*
--- 857,862 ----
***************
*** 928,943 **** InitResultRelInfo(ResultRelInfo *resultRelInfo,
resultRelInfo->ri_ConstraintExprs = NULL;
resultRelInfo->ri_junkFilter = NULL;
resultRelInfo->ri_projectReturning = NULL;
-
- /*
- * If there are indices on the result relation, open them and save
- * descriptors in the result relation info, so that we can add new index
- * entries for the tuples we add/update. We need not do this for a
- * DELETE, however, since deletion doesn't affect indexes.
- */
- if (resultRelationDesc->rd_rel->relhasindex &&
- operation != CMD_DELETE)
- ExecOpenIndices(resultRelInfo);
}
/*
--- 926,931 ----
***************
*** 993,1007 **** ExecGetTriggerResultRel(EState *estate, Oid relid)
/*
* Make the new entry in the right context. Currently, we don't need any
! * index information in ResultRelInfos used only for triggers, so tell
! * InitResultRelInfo it's a DELETE.
*/
oldcontext = MemoryContextSwitchTo(estate->es_query_cxt);
rInfo = makeNode(ResultRelInfo);
InitResultRelInfo(rInfo,
rel,
0, /* dummy rangetable index */
- CMD_DELETE,
estate->es_instrument);
estate->es_trig_target_relations =
lappend(estate->es_trig_target_relations, rInfo);
--- 981,993 ----
/*
* Make the new entry in the right context. Currently, we don't need any
! * index information in ResultRelInfos used only for triggers.
*/
oldcontext = MemoryContextSwitchTo(estate->es_query_cxt);
rInfo = makeNode(ResultRelInfo);
InitResultRelInfo(rInfo,
rel,
0, /* dummy rangetable index */
estate->es_instrument);
estate->es_trig_target_relations =
lappend(estate->es_trig_target_relations, rInfo);
***************
*** 1178,1183 **** ExecutePlan(EState *estate,
--- 1164,1250 ----
*/
estate->es_direction = direction;
+ /* Process top-level CTEs in case they have writes inside */
+ if (estate->es_plannedstmt->hasWritableCtes)
+ {
+ ListCell *lc;
+
+ foreach(lc, estate->es_plannedstmt->planTree->initPlan)
+ {
+ SubPlan *sp;
+ int cte_param_id;
+ ParamExecData* prmdata;
+ CteScanState *leader;
+
+ sp = (SubPlan *) lfirst(lc);
+ if (sp->subLinkType != CTE_SUBLINK)
+ continue;
+
+ cte_param_id = linitial_int(sp->setParam);
+ prmdata = &(estate->es_param_exec_vals[cte_param_id]);
+ leader = (CteScanState *) DatumGetPointer(prmdata->value);
+
+
+ /*
+ * bump CID.
+ *
+ * This currently relies on the fact that the top-level statement
+ * can only be SELECT or VALUES. Otherwise we'd possibly have to
+ * increment the CID after the last CTE.
+ */
+ CommandCounterIncrement();
+ estate->es_output_cid = GetCurrentCommandId(true);
+ estate->es_snapshot->curcid = estate->es_output_cid;
+
+ /*
+ * If there's no leader, the CTE isn't referenced anywhere
+ * so we can just go ahead and scan the plan.
+ */
+ if (!leader)
+ {
+ TupleTableSlot *slot;
+ PlanState *ps = (PlanState *) list_nth(estate->es_subplanstates,
+ sp->plan_id - 1);
+
+ Assert(IsA(ps, ModifyTableState));
+
+ /*
+ * We might have a RETURNING here, which means that
+ * we have to loop until the plan returns NULL.
+ */
+ for (;;)
+ {
+ slot = ExecProcNode(ps);
+ if (TupIsNull(slot))
+ break;
+ }
+ }
+ else
+ {
+ TupleTableSlot* slot;
+ PlanState *ps = (PlanState *) list_nth(estate->es_subplanstates,
+ sp->plan_id - 1);
+
+ /* Regular CTE, ignore */
+ if (!IsA(ps, ModifyTableState))
+ continue;
+
+ /*
+ * Scan through the leader CTE so the RETURNING tuples are
+ * stored into the tuple store.
+ */
+ for (;;)
+ {
+ slot = ExecProcNode((PlanState *) leader);
+ if (TupIsNull(slot))
+ break;
+ }
+
+ ExecReScan((PlanState *) leader, NULL);
+ }
+ }
+ }
+
/*
* Loop until we've processed the proper number of tuples from the plan.
*/
***************
*** 1947,1953 **** EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree)
* ExecInitSubPlan expects to be able to find these entries.
* Some of the SubPlans might not be used in the part of the plan tree
* we intend to run, but since it's not easy to tell which, we just
! * initialize them all.
*/
Assert(estate->es_subplanstates == NIL);
foreach(l, parentestate->es_plannedstmt->subplans)
--- 2014,2021 ----
* ExecInitSubPlan expects to be able to find these entries.
* Some of the SubPlans might not be used in the part of the plan tree
* we intend to run, but since it's not easy to tell which, we just
! * initialize them all. However, we will never run ModifyTable nodes in
! * EvalPlanQual() so don't initialize them.
*/
Assert(estate->es_subplanstates == NIL);
foreach(l, parentestate->es_plannedstmt->subplans)
***************
*** 1955,1961 **** EvalPlanQualStart(EPQState *epqstate, EState *parentestate, Plan *planTree)
Plan *subplan = (Plan *) lfirst(l);
PlanState *subplanstate;
! subplanstate = ExecInitNode(subplan, estate, 0);
estate->es_subplanstates = lappend(estate->es_subplanstates,
subplanstate);
--- 2023,2033 ----
Plan *subplan = (Plan *) lfirst(l);
PlanState *subplanstate;
! /* Don't initialize ModifyTable subplans. */
! if (IsA(subplan, ModifyTable))
! subplanstate = NULL;
! else
! subplanstate = ExecInitNode(subplan, estate, 0);
estate->es_subplanstates = lappend(estate->es_subplanstates,
subplanstate);
*** a/src/backend/executor/execUtils.c
--- b/src/backend/executor/execUtils.c
***************
*** 969,981 **** ExecCloseIndices(ResultRelInfo *resultRelInfo)
* ----------------------------------------------------------------
*/
List *
! ExecInsertIndexTuples(TupleTableSlot *slot,
ItemPointer tupleid,
EState *estate,
bool is_vacuum_full)
{
List *result = NIL;
- ResultRelInfo *resultRelInfo;
int i;
int numIndices;
RelationPtr relationDescs;
--- 969,981 ----
* ----------------------------------------------------------------
*/
List *
! ExecInsertIndexTuples(ResultRelInfo* resultRelInfo,
! TupleTableSlot *slot,
ItemPointer tupleid,
EState *estate,
bool is_vacuum_full)
{
List *result = NIL;
int i;
int numIndices;
RelationPtr relationDescs;
***************
*** 988,994 **** ExecInsertIndexTuples(TupleTableSlot *slot,
/*
* Get information from the result relation info structure.
*/
- resultRelInfo = estate->es_result_relation_info;
numIndices = resultRelInfo->ri_NumIndices;
relationDescs = resultRelInfo->ri_IndexRelationDescs;
indexInfoArray = resultRelInfo->ri_IndexRelationInfo;
--- 988,993 ----
*** a/src/backend/executor/nodeModifyTable.c
--- b/src/backend/executor/nodeModifyTable.c
***************
*** 158,169 **** ExecProcessReturning(ProjectionInfo *projectReturning,
* ----------------------------------------------------------------
*/
static TupleTableSlot *
! ExecInsert(TupleTableSlot *slot,
TupleTableSlot *planSlot,
EState *estate)
{
HeapTuple tuple;
- ResultRelInfo *resultRelInfo;
Relation resultRelationDesc;
Oid newId;
List *recheckIndexes = NIL;
--- 158,169 ----
* ----------------------------------------------------------------
*/
static TupleTableSlot *
! ExecInsert(ResultRelInfo *resultRelInfo,
! TupleTableSlot *slot,
TupleTableSlot *planSlot,
EState *estate)
{
HeapTuple tuple;
Relation resultRelationDesc;
Oid newId;
List *recheckIndexes = NIL;
***************
*** 177,183 **** ExecInsert(TupleTableSlot *slot,
/*
* get information on the (current) result relation
*/
- resultRelInfo = estate->es_result_relation_info;
resultRelationDesc = resultRelInfo->ri_RelationDesc;
/*
--- 177,182 ----
***************
*** 248,254 **** ExecInsert(TupleTableSlot *slot,
* insert index entries for tuple
*/
if (resultRelInfo->ri_NumIndices > 0)
! recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self),
estate, false);
/* AFTER ROW INSERT Triggers */
--- 247,254 ----
* insert index entries for tuple
*/
if (resultRelInfo->ri_NumIndices > 0)
! recheckIndexes = ExecInsertIndexTuples(resultRelInfo,
! slot, &(tuple->t_self),
estate, false);
/* AFTER ROW INSERT Triggers */
***************
*** 272,283 **** ExecInsert(TupleTableSlot *slot,
* ----------------------------------------------------------------
*/
static TupleTableSlot *
! ExecDelete(ItemPointer tupleid,
TupleTableSlot *planSlot,
EPQState *epqstate,
EState *estate)
{
- ResultRelInfo *resultRelInfo;
Relation resultRelationDesc;
HTSU_Result result;
ItemPointerData update_ctid;
--- 272,283 ----
* ----------------------------------------------------------------
*/
static TupleTableSlot *
! ExecDelete(ResultRelInfo *resultRelInfo,
! ItemPointer tupleid,
TupleTableSlot *planSlot,
EPQState *epqstate,
EState *estate)
{
Relation resultRelationDesc;
HTSU_Result result;
ItemPointerData update_ctid;
***************
*** 286,292 **** ExecDelete(ItemPointer tupleid,
/*
* get information on the (current) result relation
*/
- resultRelInfo = estate->es_result_relation_info;
resultRelationDesc = resultRelInfo->ri_RelationDesc;
/* BEFORE ROW DELETE Triggers */
--- 286,291 ----
***************
*** 415,428 **** ldelete:;
* ----------------------------------------------------------------
*/
static TupleTableSlot *
! ExecUpdate(ItemPointer tupleid,
TupleTableSlot *slot,
TupleTableSlot *planSlot,
EPQState *epqstate,
EState *estate)
{
HeapTuple tuple;
- ResultRelInfo *resultRelInfo;
Relation resultRelationDesc;
HTSU_Result result;
ItemPointerData update_ctid;
--- 414,427 ----
* ----------------------------------------------------------------
*/
static TupleTableSlot *
! ExecUpdate(ResultRelInfo *resultRelInfo,
! ItemPointer tupleid,
TupleTableSlot *slot,
TupleTableSlot *planSlot,
EPQState *epqstate,
EState *estate)
{
HeapTuple tuple;
Relation resultRelationDesc;
HTSU_Result result;
ItemPointerData update_ctid;
***************
*** 444,450 **** ExecUpdate(ItemPointer tupleid,
/*
* get information on the (current) result relation
*/
- resultRelInfo = estate->es_result_relation_info;
resultRelationDesc = resultRelInfo->ri_RelationDesc;
/* BEFORE ROW UPDATE Triggers */
--- 443,448 ----
***************
*** 563,569 **** lreplace:;
* If it's a HOT update, we mustn't insert new index entries.
*/
if (resultRelInfo->ri_NumIndices > 0 && !HeapTupleIsHeapOnly(tuple))
! recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self),
estate, false);
/* AFTER ROW UPDATE Triggers */
--- 561,568 ----
* If it's a HOT update, we mustn't insert new index entries.
*/
if (resultRelInfo->ri_NumIndices > 0 && !HeapTupleIsHeapOnly(tuple))
! recheckIndexes = ExecInsertIndexTuples(resultRelInfo,
! slot, &(tuple->t_self),
estate, false);
/* AFTER ROW UPDATE Triggers */
***************
*** 589,603 **** fireBSTriggers(ModifyTableState *node)
{
case CMD_INSERT:
ExecBSInsertTriggers(node->ps.state,
! node->ps.state->es_result_relations);
break;
case CMD_UPDATE:
ExecBSUpdateTriggers(node->ps.state,
! node->ps.state->es_result_relations);
break;
case CMD_DELETE:
ExecBSDeleteTriggers(node->ps.state,
! node->ps.state->es_result_relations);
break;
default:
elog(ERROR, "unknown operation");
--- 588,602 ----
{
case CMD_INSERT:
ExecBSInsertTriggers(node->ps.state,
! node->resultRelInfo);
break;
case CMD_UPDATE:
ExecBSUpdateTriggers(node->ps.state,
! node->resultRelInfo);
break;
case CMD_DELETE:
ExecBSDeleteTriggers(node->ps.state,
! node->resultRelInfo);
break;
default:
elog(ERROR, "unknown operation");
***************
*** 615,629 **** fireASTriggers(ModifyTableState *node)
{
case CMD_INSERT:
ExecASInsertTriggers(node->ps.state,
! node->ps.state->es_result_relations);
break;
case CMD_UPDATE:
ExecASUpdateTriggers(node->ps.state,
! node->ps.state->es_result_relations);
break;
case CMD_DELETE:
ExecASDeleteTriggers(node->ps.state,
! node->ps.state->es_result_relations);
break;
default:
elog(ERROR, "unknown operation");
--- 614,628 ----
{
case CMD_INSERT:
ExecASInsertTriggers(node->ps.state,
! node->resultRelInfo);
break;
case CMD_UPDATE:
ExecASUpdateTriggers(node->ps.state,
! node->resultRelInfo);
break;
case CMD_DELETE:
ExecASDeleteTriggers(node->ps.state,
! node->resultRelInfo);
break;
default:
elog(ERROR, "unknown operation");
***************
*** 645,650 **** ExecModifyTable(ModifyTableState *node)
--- 644,650 ----
EState *estate = node->ps.state;
CmdType operation = node->operation;
PlanState *subplanstate;
+ ResultRelInfo *resultRelInfo;
JunkFilter *junkfilter;
TupleTableSlot *slot;
TupleTableSlot *planSlot;
***************
*** 660,676 **** ExecModifyTable(ModifyTableState *node)
node->fireBSTriggers = false;
}
- /*
- * es_result_relation_info must point to the currently active result
- * relation. (Note we assume that ModifyTable nodes can't be nested.)
- * We want it to be NULL whenever we're not within ModifyTable, though.
- */
- estate->es_result_relation_info =
- estate->es_result_relations + node->mt_whichplan;
-
/* Preload local variables */
subplanstate = node->mt_plans[node->mt_whichplan];
! junkfilter = estate->es_result_relation_info->ri_junkFilter;
/*
* Fetch rows from subplan(s), and execute the required table modification
--- 660,669 ----
node->fireBSTriggers = false;
}
/* Preload local variables */
subplanstate = node->mt_plans[node->mt_whichplan];
! resultRelInfo = node->resultRelInfo + node->mt_whichplan;
! junkfilter = resultRelInfo->ri_junkFilter;
/*
* Fetch rows from subplan(s), and execute the required table modification
***************
*** 686,694 **** ExecModifyTable(ModifyTableState *node)
node->mt_whichplan++;
if (node->mt_whichplan < node->mt_nplans)
{
- estate->es_result_relation_info++;
subplanstate = node->mt_plans[node->mt_whichplan];
! junkfilter = estate->es_result_relation_info->ri_junkFilter;
EvalPlanQualSetPlan(&node->mt_epqstate, subplanstate->plan);
continue;
}
--- 679,687 ----
node->mt_whichplan++;
if (node->mt_whichplan < node->mt_nplans)
{
subplanstate = node->mt_plans[node->mt_whichplan];
! resultRelInfo = node->resultRelInfo + node->mt_whichplan;
! junkfilter = resultRelInfo->ri_junkFilter;
EvalPlanQualSetPlan(&node->mt_epqstate, subplanstate->plan);
continue;
}
***************
*** 730,743 **** ExecModifyTable(ModifyTableState *node)
switch (operation)
{
case CMD_INSERT:
! slot = ExecInsert(slot, planSlot, estate);
break;
case CMD_UPDATE:
! slot = ExecUpdate(tupleid, slot, planSlot,
&node->mt_epqstate, estate);
break;
case CMD_DELETE:
! slot = ExecDelete(tupleid, planSlot,
&node->mt_epqstate, estate);
break;
default:
--- 723,739 ----
switch (operation)
{
case CMD_INSERT:
! slot = ExecInsert(resultRelInfo,
! slot, planSlot, estate);
break;
case CMD_UPDATE:
! slot = ExecUpdate(resultRelInfo,
! tupleid, slot, planSlot,
&node->mt_epqstate, estate);
break;
case CMD_DELETE:
! slot = ExecDelete(resultRelInfo,
! tupleid, planSlot,
&node->mt_epqstate, estate);
break;
default:
***************
*** 750,764 **** ExecModifyTable(ModifyTableState *node)
* the work on next call.
*/
if (slot)
- {
- estate->es_result_relation_info = NULL;
return slot;
- }
}
- /* Reset es_result_relation_info before exiting */
- estate->es_result_relation_info = NULL;
-
/*
* We're done, but fire AFTER STATEMENT triggers before exiting.
*/
--- 746,754 ----
***************
*** 805,829 **** ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
mtstate->mt_plans = (PlanState **) palloc0(sizeof(PlanState *) * nplans);
mtstate->mt_nplans = nplans;
mtstate->operation = operation;
/* set up epqstate with dummy subplan pointer for the moment */
EvalPlanQualInit(&mtstate->mt_epqstate, estate, NULL, node->epqParam);
mtstate->fireBSTriggers = true;
- /* For the moment, assume our targets are exactly the global result rels */
-
/*
* call ExecInitNode on each of the plans to be executed and save the
* results into the array "mt_plans". Note we *must* set
* estate->es_result_relation_info correctly while we initialize each
* sub-plan; ExecContextForcesOids depends on that!
*/
- estate->es_result_relation_info = estate->es_result_relations;
i = 0;
foreach(l, node->plans)
{
subplan = (Plan *) lfirst(l);
mtstate->mt_plans[i] = ExecInitNode(subplan, estate, eflags);
! estate->es_result_relation_info++;
i++;
}
estate->es_result_relation_info = NULL;
--- 795,833 ----
mtstate->mt_plans = (PlanState **) palloc0(sizeof(PlanState *) * nplans);
mtstate->mt_nplans = nplans;
mtstate->operation = operation;
+ mtstate->resultRelIndex = node->resultRelIndex;
+ mtstate->resultRelInfo = estate->es_result_relations + node->resultRelIndex;
+
/* set up epqstate with dummy subplan pointer for the moment */
EvalPlanQualInit(&mtstate->mt_epqstate, estate, NULL, node->epqParam);
mtstate->fireBSTriggers = true;
/*
* call ExecInitNode on each of the plans to be executed and save the
* results into the array "mt_plans". Note we *must* set
* estate->es_result_relation_info correctly while we initialize each
* sub-plan; ExecContextForcesOids depends on that!
*/
i = 0;
+ resultRelInfo = mtstate->resultRelInfo;
foreach(l, node->plans)
{
subplan = (Plan *) lfirst(l);
+
+ /*
+ * If there are indices on the result relation, open them and save
+ * descriptors in the result relation info, so that we can add new index
+ * entries for the tuples we add/update. We need not do this for a
+ * DELETE, however, since deletion doesn't affect indexes.
+ */
+ if (resultRelInfo->ri_RelationDesc->rd_rel->relhasindex &&
+ operation != CMD_DELETE)
+ ExecOpenIndices(resultRelInfo);
+
+ estate->es_result_relation_info = resultRelInfo;
mtstate->mt_plans[i] = ExecInitNode(subplan, estate, eflags);
!
! resultRelInfo++;
i++;
}
estate->es_result_relation_info = NULL;
***************
*** 860,867 **** ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
/*
* Build a projection for each result rel.
*/
! Assert(list_length(node->returningLists) == estate->es_num_result_relations);
! resultRelInfo = estate->es_result_relations;
foreach(l, node->returningLists)
{
List *rlist = (List *) lfirst(l);
--- 864,870 ----
/*
* Build a projection for each result rel.
*/
! resultRelInfo = mtstate->resultRelInfo;
foreach(l, node->returningLists)
{
List *rlist = (List *) lfirst(l);
***************
*** 960,966 **** ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
if (junk_filter_needed)
{
! resultRelInfo = estate->es_result_relations;
for (i = 0; i < nplans; i++)
{
JunkFilter *j;
--- 963,969 ----
if (junk_filter_needed)
{
! resultRelInfo = mtstate->resultRelInfo;
for (i = 0; i < nplans; i++)
{
JunkFilter *j;
***************
*** 989,995 **** ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
else
{
if (operation == CMD_INSERT)
! ExecCheckPlanOutput(estate->es_result_relations->ri_RelationDesc,
subplan->targetlist);
}
}
--- 992,998 ----
else
{
if (operation == CMD_INSERT)
! ExecCheckPlanOutput(mtstate->resultRelInfo->ri_RelationDesc,
subplan->targetlist);
}
}
*** a/src/backend/executor/nodeSubplan.c
--- b/src/backend/executor/nodeSubplan.c
***************
*** 668,673 **** ExecInitSubPlan(SubPlan *subplan, PlanState *parent)
--- 668,675 ----
sstate->planstate = (PlanState *) list_nth(estate->es_subplanstates,
subplan->plan_id - 1);
+ Assert(sstate->planstate != NULL);
+
/* Initialize subexpressions */
sstate->testexpr = ExecInitExpr((Expr *) subplan->testexpr, parent);
sstate->args = (List *) ExecInitExpr((Expr *) subplan->args, parent);
*** a/src/backend/nodes/copyfuncs.c
--- b/src/backend/nodes/copyfuncs.c
***************
*** 84,89 **** _copyPlannedStmt(PlannedStmt *from)
--- 84,90 ----
COPY_NODE_FIELD(resultRelations);
COPY_NODE_FIELD(utilityStmt);
COPY_NODE_FIELD(intoClause);
+ COPY_SCALAR_FIELD(hasWritableCtes);
COPY_NODE_FIELD(subplans);
COPY_BITMAPSET_FIELD(rewindPlanIDs);
COPY_NODE_FIELD(rowMarks);
***************
*** 172,177 **** _copyModifyTable(ModifyTable *from)
--- 173,179 ----
*/
COPY_SCALAR_FIELD(operation);
COPY_NODE_FIELD(resultRelations);
+ COPY_SCALAR_FIELD(resultRelIndex);
COPY_NODE_FIELD(plans);
COPY_NODE_FIELD(returningLists);
COPY_NODE_FIELD(rowMarks);
*** a/src/backend/nodes/nodeFuncs.c
--- b/src/backend/nodes/nodeFuncs.c
***************
*** 2383,2388 **** bool
--- 2383,2432 ----
return true;
}
break;
+ case T_InsertStmt:
+ {
+ InsertStmt *stmt = (InsertStmt *) node;
+
+ if (walker(stmt->relation, context))
+ return true;
+ if (walker(stmt->cols, context))
+ return true;
+ if (walker(stmt->selectStmt, context))
+ return true;
+ if (walker(stmt->returningList, context))
+ return true;
+ }
+ break;
+ case T_UpdateStmt:
+ {
+ UpdateStmt *stmt = (UpdateStmt *) node;
+
+ if (walker(stmt->relation, context))
+ return true;
+ if (walker(stmt->targetList, context))
+ return true;
+ if (walker(stmt->whereClause, context))
+ return true;
+ if (walker(stmt->fromClause, context))
+ return true;
+ if (walker(stmt->returningList, context))
+ return true;
+ }
+ break;
+ case T_DeleteStmt:
+ {
+ DeleteStmt *stmt = (DeleteStmt *) node;
+
+ if (walker(stmt->relation, context))
+ return true;
+ if (walker(stmt->usingClause, context))
+ return true;
+ if (walker(stmt->whereClause, context))
+ return true;
+ if (walker(stmt->returningList, context))
+ return true;
+ }
+ break;
case T_A_Expr:
{
A_Expr *expr = (A_Expr *) node;
*** a/src/backend/nodes/outfuncs.c
--- b/src/backend/nodes/outfuncs.c
***************
*** 327,332 **** _outModifyTable(StringInfo str, ModifyTable *node)
--- 327,333 ----
WRITE_ENUM_FIELD(operation, CmdType);
WRITE_NODE_FIELD(resultRelations);
+ WRITE_INT_FIELD(resultRelIndex);
WRITE_NODE_FIELD(plans);
WRITE_NODE_FIELD(returningLists);
WRITE_NODE_FIELD(rowMarks);
***************
*** 1530,1535 **** _outPlannerGlobal(StringInfo str, PlannerGlobal *node)
--- 1531,1537 ----
WRITE_NODE_FIELD(finalrowmarks);
WRITE_NODE_FIELD(relationOids);
WRITE_NODE_FIELD(invalItems);
+ WRITE_NODE_FIELD(resultRelations);
WRITE_UINT_FIELD(lastPHId);
WRITE_BOOL_FIELD(transientPlan);
}
***************
*** 1544,1550 **** _outPlannerInfo(StringInfo str, PlannerInfo *node)
WRITE_NODE_FIELD(glob);
WRITE_UINT_FIELD(query_level);
WRITE_NODE_FIELD(join_rel_list);
- WRITE_NODE_FIELD(resultRelations);
WRITE_NODE_FIELD(init_plans);
WRITE_NODE_FIELD(cte_plan_ids);
WRITE_NODE_FIELD(eq_classes);
--- 1546,1551 ----
*** a/src/backend/optimizer/plan/createplan.c
--- b/src/backend/optimizer/plan/createplan.c
***************
*** 3753,3762 **** make_modifytable(CmdType operation, List *resultRelations,
double total_size;
ListCell *subnode;
- Assert(list_length(resultRelations) == list_length(subplans));
- Assert(returningLists == NIL ||
- list_length(resultRelations) == list_length(returningLists));
-
/*
* Compute cost as sum of subplan costs.
*/
--- 3753,3758 ----
*** a/src/backend/optimizer/plan/planner.c
--- b/src/backend/optimizer/plan/planner.c
***************
*** 160,165 **** standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
--- 160,167 ----
glob->finalrowmarks = NIL;
glob->relationOids = NIL;
glob->invalItems = NIL;
+ glob->hasWritableCtes = false;
+ glob->resultRelations = NIL;
glob->lastPHId = 0;
glob->transientPlan = false;
***************
*** 237,245 **** standard_planner(Query *parse, int cursorOptions, ParamListInfo boundParams)
result->transientPlan = glob->transientPlan;
result->planTree = top_plan;
result->rtable = glob->finalrtable;
! result->resultRelations = root->resultRelations;
result->utilityStmt = parse->utilityStmt;
result->intoClause = parse->intoClause;
result->subplans = glob->subplans;
result->rewindPlanIDs = glob->rewindPlanIDs;
result->rowMarks = glob->finalrowmarks;
--- 239,248 ----
result->transientPlan = glob->transientPlan;
result->planTree = top_plan;
result->rtable = glob->finalrtable;
! result->resultRelations = glob->resultRelations;
result->utilityStmt = parse->utilityStmt;
result->intoClause = parse->intoClause;
+ result->hasWritableCtes = glob->hasWritableCtes;
result->subplans = glob->subplans;
result->rewindPlanIDs = glob->rewindPlanIDs;
result->rowMarks = glob->finalrowmarks;
***************
*** 541,547 **** subquery_planner(PlannerGlobal *glob, Query *parse,
rowMarks = root->rowMarks;
plan = (Plan *) make_modifytable(parse->commandType,
! copyObject(root->resultRelations),
list_make1(plan),
returningLists,
rowMarks,
--- 544,550 ----
rowMarks = root->rowMarks;
plan = (Plan *) make_modifytable(parse->commandType,
! list_make1_int(parse->resultRelation),
list_make1(plan),
returningLists,
rowMarks,
***************
*** 706,718 **** inheritance_planner(PlannerInfo *root)
Query *parse = root->parse;
int parentRTindex = parse->resultRelation;
List *subplans = NIL;
- List *resultRelations = NIL;
List *returningLists = NIL;
List *rtable = NIL;
List *rowMarks;
List *tlist;
PlannerInfo subroot;
ListCell *l;
foreach(l, root->append_rel_list)
{
--- 709,721 ----
Query *parse = root->parse;
int parentRTindex = parse->resultRelation;
List *subplans = NIL;
List *returningLists = NIL;
List *rtable = NIL;
List *rowMarks;
List *tlist;
PlannerInfo subroot;
ListCell *l;
+ List *resultRelations = NIL;
foreach(l, root->append_rel_list)
{
***************
*** 772,779 **** inheritance_planner(PlannerInfo *root)
}
}
- root->resultRelations = resultRelations;
-
/* Mark result as unordered (probably unnecessary) */
root->query_pathkeys = NIL;
--- 775,780 ----
***************
*** 783,789 **** inheritance_planner(PlannerInfo *root)
*/
if (subplans == NIL)
{
- root->resultRelations = list_make1_int(parentRTindex);
/* although dummy, it must have a valid tlist for executor */
tlist = preprocess_targetlist(root, parse->targetList);
return (Plan *) make_result(root,
--- 784,789 ----
***************
*** 818,824 **** inheritance_planner(PlannerInfo *root)
/* And last, tack on a ModifyTable node to do the UPDATE/DELETE work */
return (Plan *) make_modifytable(parse->commandType,
! copyObject(root->resultRelations),
subplans,
returningLists,
rowMarks,
--- 818,824 ----
/* And last, tack on a ModifyTable node to do the UPDATE/DELETE work */
return (Plan *) make_modifytable(parse->commandType,
! resultRelations,
subplans,
returningLists,
rowMarks,
***************
*** 1667,1678 **** grouping_planner(PlannerInfo *root, double tuple_fraction)
count_est);
}
- /* Compute result-relations list if needed */
- if (parse->resultRelation)
- root->resultRelations = list_make1_int(parse->resultRelation);
- else
- root->resultRelations = NIL;
-
/*
* Return the actual output ordering in query_pathkeys for possible use by
* an outer query level.
--- 1667,1672 ----
*** a/src/backend/optimizer/plan/setrefs.c
--- b/src/backend/optimizer/plan/setrefs.c
***************
*** 520,525 **** set_plan_refs(PlannerGlobal *glob, Plan *plan, int rtoffset)
--- 520,529 ----
(Plan *) lfirst(l),
rtoffset);
}
+
+ splan->resultRelIndex = list_length(glob->resultRelations);
+ glob->resultRelations = list_concat(glob->resultRelations,
+ splan->resultRelations);
}
break;
case T_Append:
*** a/src/backend/optimizer/plan/subselect.c
--- b/src/backend/optimizer/plan/subselect.c
***************
*** 873,888 **** SS_process_ctes(PlannerInfo *root)
Bitmapset *tmpset;
int paramid;
Param *prm;
/*
! * Ignore CTEs that are not actually referenced anywhere.
*/
! if (cte->cterefcount == 0)
{
/* Make a dummy entry in cte_plan_ids */
root->cte_plan_ids = lappend_int(root->cte_plan_ids, -1);
continue;
}
/*
* Copy the source Query node. Probably not necessary, but let's keep
--- 873,905 ----
Bitmapset *tmpset;
int paramid;
Param *prm;
+ CmdType cmdType = ((Query *) cte->ctequery)->commandType;
/*
! * Ignore SELECT CTEs that are not actually referenced anywhere.
*/
! if (cte->cterefcount == 0 && cmdType == CMD_SELECT)
{
/* Make a dummy entry in cte_plan_ids */
root->cte_plan_ids = lappend_int(root->cte_plan_ids, -1);
continue;
}
+ else if (cmdType != CMD_SELECT)
+ {
+ /* We don't know reference counts until here */
+ if (cte->cterefcount > 0 &&
+ ((Query *) cte->ctequery)->returningList == NIL)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("INSERT/UPDATE/DELETE without RETURNING is only allowed inside a non-referenced
CTE")));
+ }
+
+ if (root->query_level > 1)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("INSERT/UPDATE/DELETE inside a CTE is only allowed on the top level")));
+ }
/*
* Copy the source Query node. Probably not necessary, but let's keep
***************
*** 899,904 **** SS_process_ctes(PlannerInfo *root)
--- 916,924 ----
cte->cterecursive, 0.0,
&subroot);
+ if (subroot->parse->commandType != CMD_SELECT)
+ root->glob->hasWritableCtes = true;
+
/*
* Make a SubPlan node for it. This is just enough unlike
* build_subplan that we can't share code.
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***************
*** 7422,7427 **** common_table_expr: name opt_name_list AS select_with_parens
--- 7422,7454 ----
n->location = @1;
$$ = (Node *) n;
}
+ | name opt_name_list AS '(' InsertStmt ')'
+ {
+ CommonTableExpr *n = makeNode(CommonTableExpr);
+ n->ctename = $1;
+ n->aliascolnames = $2;
+ n->ctequery = $5;
+ n->location = @1;
+ $$ = (Node *) n;
+ }
+ | name opt_name_list AS '(' UpdateStmt ')'
+ {
+ CommonTableExpr *n = makeNode(CommonTableExpr);
+ n->ctename = $1;
+ n->aliascolnames = $2;
+ n->ctequery = $5;
+ n->location = @1;
+ $$ = (Node *) n;
+ }
+ | name opt_name_list AS '(' DeleteStmt ')'
+ {
+ CommonTableExpr *n = makeNode(CommonTableExpr);
+ n->ctename = $1;
+ n->aliascolnames = $2;
+ n->ctequery = $5;
+ n->location = @1;
+ $$ = (Node *) n;
+ }
;
into_clause:
*** a/src/backend/parser/parse_cte.c
--- b/src/backend/parser/parse_cte.c
***************
*** 18,23 ****
--- 18,24 ----
#include "nodes/nodeFuncs.h"
#include "parser/analyze.h"
#include "parser/parse_cte.h"
+ #include "nodes/plannodes.h"
#include "utils/builtins.h"
***************
*** 225,246 **** static void
analyzeCTE(ParseState *pstate, CommonTableExpr *cte)
{
Query *query;
!
! /* Analysis not done already */
! Assert(IsA(cte->ctequery, SelectStmt));
query = parse_sub_analyze(cte->ctequery, pstate, cte, false);
cte->ctequery = (Node *) query;
/*
* Check that we got something reasonable. Many of these conditions are
* impossible given restrictions of the grammar, but check 'em anyway.
! * (These are the same checks as in transformRangeSubselect.)
*/
! if (!IsA(query, Query) ||
! query->commandType != CMD_SELECT ||
! query->utilityStmt != NULL)
! elog(ERROR, "unexpected non-SELECT command in subquery in WITH");
if (query->intoClause)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
--- 226,250 ----
analyzeCTE(ParseState *pstate, CommonTableExpr *cte)
{
Query *query;
! List *cteList;
query = parse_sub_analyze(cte->ctequery, pstate, cte, false);
cte->ctequery = (Node *) query;
+ if (query->commandType == CMD_SELECT)
+ cteList = query->targetList;
+ else
+ cteList = query->returningList;
+
/*
* Check that we got something reasonable. Many of these conditions are
* impossible given restrictions of the grammar, but check 'em anyway.
! * Note, however, that we can't yet decice whether to allow
! * INSERT/UPDATE/DELETE without a RETURNING clause or not because we don't
! * know the refcount.
*/
! Assert(IsA(query, Query) && query->utilityStmt == NULL);
!
if (query->intoClause)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
***************
*** 251,257 **** analyzeCTE(ParseState *pstate, CommonTableExpr *cte)
if (!cte->cterecursive)
{
/* Compute the output column names/types if not done yet */
! analyzeCTETargetList(pstate, cte, query->targetList);
}
else
{
--- 255,261 ----
if (!cte->cterecursive)
{
/* Compute the output column names/types if not done yet */
! analyzeCTETargetList(pstate, cte, cteList);
}
else
{
***************
*** 269,275 **** analyzeCTE(ParseState *pstate, CommonTableExpr *cte)
lctyp = list_head(cte->ctecoltypes);
lctypmod = list_head(cte->ctecoltypmods);
varattno = 0;
! foreach(lctlist, query->targetList)
{
TargetEntry *te = (TargetEntry *) lfirst(lctlist);
Node *texpr;
--- 273,279 ----
lctyp = list_head(cte->ctecoltypes);
lctypmod = list_head(cte->ctecoltypmods);
varattno = 0;
! foreach(lctlist, cteList)
{
TargetEntry *te = (TargetEntry *) lfirst(lctlist);
Node *texpr;
*** a/src/backend/parser/parse_relation.c
--- b/src/backend/parser/parse_relation.c
***************
*** 24,29 ****
--- 24,30 ----
#include "funcapi.h"
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
+ #include "nodes/plannodes.h"
#include "parser/parsetree.h"
#include "parser/parse_relation.h"
#include "parser/parse_type.h"
*** a/src/backend/parser/parse_target.c
--- b/src/backend/parser/parse_target.c
***************
*** 314,323 **** markTargetListOrigin(ParseState *pstate, TargetEntry *tle,
{
CommonTableExpr *cte = GetCTEForRTE(pstate, rte, netlevelsup);
TargetEntry *ste;
/* should be analyzed by now */
Assert(IsA(cte->ctequery, Query));
! ste = get_tle_by_resno(((Query *) cte->ctequery)->targetList,
attnum);
if (ste == NULL || ste->resjunk)
elog(ERROR, "subquery %s does not have attribute %d",
--- 314,333 ----
{
CommonTableExpr *cte = GetCTEForRTE(pstate, rte, netlevelsup);
TargetEntry *ste;
+ List *cteList;
+ Query *ctequery;
/* should be analyzed by now */
Assert(IsA(cte->ctequery, Query));
!
! ctequery = (Query *) cte->ctequery;
!
! if (ctequery->commandType == CMD_SELECT)
! cteList = ctequery->targetList;
! else
! cteList = ctequery->returningList;
!
! ste = get_tle_by_resno(cteList,
attnum);
if (ste == NULL || ste->resjunk)
elog(ERROR, "subquery %s does not have attribute %d",
***************
*** 1345,1355 **** expandRecordVariable(ParseState *pstate, Var *var, int levelsup)
{
CommonTableExpr *cte = GetCTEForRTE(pstate, rte, netlevelsup);
TargetEntry *ste;
/* should be analyzed by now */
Assert(IsA(cte->ctequery, Query));
! ste = get_tle_by_resno(((Query *) cte->ctequery)->targetList,
! attnum);
if (ste == NULL || ste->resjunk)
elog(ERROR, "subquery %s does not have attribute %d",
rte->eref->aliasname, attnum);
--- 1355,1374 ----
{
CommonTableExpr *cte = GetCTEForRTE(pstate, rte, netlevelsup);
TargetEntry *ste;
+ List *cteList;
+ Query *ctequery;
/* should be analyzed by now */
Assert(IsA(cte->ctequery, Query));
!
! ctequery = (Query *) cte->ctequery;
!
! if (ctequery->commandType == CMD_SELECT)
! cteList = ctequery->targetList;
! else
! cteList = ctequery->returningList;
!
! ste = get_tle_by_resno(cteList, attnum);
if (ste == NULL || ste->resjunk)
elog(ERROR, "subquery %s does not have attribute %d",
rte->eref->aliasname, attnum);
***************
*** 1372,1378 **** expandRecordVariable(ParseState *pstate, Var *var, int levelsup)
levelsup++)
pstate = pstate->parentParseState;
mypstate.parentParseState = pstate;
! mypstate.p_rtable = ((Query *) cte->ctequery)->rtable;
/* don't bother filling the rest of the fake pstate */
return expandRecordVariable(&mypstate, (Var *) expr, 0);
--- 1391,1397 ----
levelsup++)
pstate = pstate->parentParseState;
mypstate.parentParseState = pstate;
! mypstate.p_rtable = ctequery->rtable;
/* don't bother filling the rest of the fake pstate */
return expandRecordVariable(&mypstate, (Var *) expr, 0);
*** a/src/backend/rewrite/rewriteHandler.c
--- b/src/backend/rewrite/rewriteHandler.c
***************
*** 1632,1637 **** RewriteQuery(Query *parsetree, List *rewrite_events)
--- 1632,1641 ----
bool returning = false;
Query *qual_product = NULL;
List *rewritten = NIL;
+ ListCell *lc;
+ CommonTableExpr *cte;
+ Query *ctequery;
+ List *newstuff;
/*
* If the statement is an update, insert or delete - fire rules on it.
***************
*** 1749,1755 **** RewriteQuery(Query *parsetree, List *rewrite_events)
foreach(n, product_queries)
{
Query *pt = (Query *) lfirst(n);
- List *newstuff;
newstuff = RewriteQuery(pt, rewrite_events);
rewritten = list_concat(rewritten, newstuff);
--- 1753,1758 ----
***************
*** 1804,1809 **** RewriteQuery(Query *parsetree, List *rewrite_events)
--- 1807,1863 ----
}
/*
+ * Rewrite DML statements inside CTEs. If there are any
+ * DO ALSO rules, they are added to the top level (there
+ * won't be any non-top-level CTEs with DML).
+ */
+ foreach(lc, parsetree->cteList)
+ {
+ cte = lfirst(lc);
+
+ ctequery = (Query *) cte->ctequery;
+
+ if (ctequery->commandType == CMD_SELECT)
+ continue;
+
+ newstuff = RewriteQuery(ctequery, NIL);
+
+ if (list_length(newstuff) > 1 &&
+ ctequery->commandType != CMD_INSERT)
+ {
+ /*
+ * For UPDATE and DELETE, the actual query is
+ * at the end of the list.
+ */
+
+ ListCell *lc;
+ int n = 1;
+
+ foreach(lc, newstuff)
+ {
+ /*
+ * If this is the last one, don't add it to the results.
+ * Instead, update the query inside the CTE.
+ */
+ if (n == list_length(newstuff))
+ cte->ctequery = (Node *) lfirst(lc);
+ else
+ rewritten = lcons((void *) lfirst(lc), rewritten);
+
+ n++;
+ }
+
+ }
+ else
+ {
+ /* For INSERT the actual query is the first one. */
+ cte->ctequery = (Node *) linitial(newstuff);
+ rewritten = list_concat(rewritten,
+ list_delete_first(newstuff));
+ }
+ }
+
+ /*
* For INSERTs, the original query is done first; for UPDATE/DELETE, it is
* done last. This is needed because update and delete rule actions might
* not do anything if they are invoked after the update or delete is
*** a/src/backend/tcop/pquery.c
--- b/src/backend/tcop/pquery.c
***************
*** 293,298 **** ChoosePortalStrategy(List *stmts)
--- 293,299 ----
if (pstmt->canSetTag)
{
if (pstmt->commandType == CMD_SELECT &&
+ pstmt->hasWritableCtes == false &&
pstmt->utilityStmt == NULL &&
pstmt->intoClause == NULL)
return PORTAL_ONE_SELECT;
*** a/src/backend/utils/adt/ruleutils.c
--- b/src/backend/utils/adt/ruleutils.c
***************
*** 3909,3917 **** get_name_for_var_field(Var *var, int fieldno,
}
if (lc != NULL)
{
! Query *ctequery = (Query *) cte->ctequery;
! TargetEntry *ste = get_tle_by_resno(ctequery->targetList,
! attnum);
if (ste == NULL || ste->resjunk)
elog(ERROR, "subquery %s does not have attribute %d",
--- 3909,3924 ----
}
if (lc != NULL)
{
! Query *ctequery = (Query *) cte->ctequery;
! List *ctelist;
! TargetEntry *ste;
!
! if (ctequery->commandType != CMD_SELECT)
! ctelist = ctequery->returningList;
! else
! ctelist = ctequery->targetList;
!
! ste = get_tle_by_resno(ctelist, attnum);
if (ste == NULL || ste->resjunk)
elog(ERROR, "subquery %s does not have attribute %d",
*** a/src/include/executor/executor.h
--- b/src/include/executor/executor.h
***************
*** 160,166 **** extern void ExecutorRewind(QueryDesc *queryDesc);
extern void InitResultRelInfo(ResultRelInfo *resultRelInfo,
Relation resultRelationDesc,
Index resultRelationIndex,
- CmdType operation,
bool doInstrument);
extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids);
--- 160,165 ----
***************
*** 319,325 **** extern void ExecCloseScanRelation(Relation scanrel);
extern void ExecOpenIndices(ResultRelInfo *resultRelInfo);
extern void ExecCloseIndices(ResultRelInfo *resultRelInfo);
! extern List *ExecInsertIndexTuples(TupleTableSlot *slot, ItemPointer tupleid,
EState *estate, bool is_vacuum_full);
extern void RegisterExprContextCallback(ExprContext *econtext,
--- 318,325 ----
extern void ExecOpenIndices(ResultRelInfo *resultRelInfo);
extern void ExecCloseIndices(ResultRelInfo *resultRelInfo);
! extern List *ExecInsertIndexTuples(ResultRelInfo *resultRelInfo,
! TupleTableSlot *slot, ItemPointer tupleid,
EState *estate, bool is_vacuum_full);
extern void RegisterExprContextCallback(ExprContext *econtext,
*** a/src/include/nodes/execnodes.h
--- b/src/include/nodes/execnodes.h
***************
*** 1024,1029 **** typedef struct ModifyTableState
--- 1024,1031 ----
PlanState **mt_plans; /* subplans (one per target rel) */
int mt_nplans; /* number of plans in the array */
int mt_whichplan; /* which one is being executed (0..n-1) */
+ int resultRelIndex;
+ ResultRelInfo *resultRelInfo;
EPQState mt_epqstate; /* for evaluating EvalPlanQual rechecks */
bool fireBSTriggers; /* do we need to fire stmt triggers? */
} ModifyTableState;
*** a/src/include/nodes/plannodes.h
--- b/src/include/nodes/plannodes.h
***************
*** 55,60 **** typedef struct PlannedStmt
--- 55,62 ----
IntoClause *intoClause; /* target for SELECT INTO / CREATE TABLE AS */
+ bool hasWritableCtes;
+
List *subplans; /* Plan trees for SubPlan expressions */
Bitmapset *rewindPlanIDs; /* indices of subplans that require REWIND */
***************
*** 165,170 **** typedef struct ModifyTable
--- 167,173 ----
Plan plan;
CmdType operation; /* INSERT, UPDATE, or DELETE */
List *resultRelations; /* integer list of RT indexes */
+ int resultRelIndex;
List *plans; /* plan(s) producing source data */
List *returningLists; /* per-target-table RETURNING tlists */
List *rowMarks; /* PlanRowMarks (non-locking only) */
*** a/src/include/nodes/relation.h
--- b/src/include/nodes/relation.h
***************
*** 80,85 **** typedef struct PlannerGlobal
--- 80,89 ----
List *invalItems; /* other dependencies, as PlanInvalItems */
+ bool hasWritableCtes;/* is there an (INSERT|UPDATE|DELETE) .. RETURNING inside a CTE? */
+
+ List *resultRelations;/* list of result relations */
+
Index lastPHId; /* highest PlaceHolderVar ID assigned */
bool transientPlan; /* redo plan when TransactionXmin changes? */
***************
*** 142,149 **** typedef struct PlannerInfo
List *join_rel_list; /* list of join-relation RelOptInfos */
struct HTAB *join_rel_hash; /* optional hashtable for join relations */
- List *resultRelations; /* integer list of RT indexes, or NIL */
-
List *init_plans; /* init SubPlans for query */
List *cte_plan_ids; /* per-CTE-item list of subplan IDs */
--- 146,151 ----
*** a/src/test/regress/expected/with.out
--- b/src/test/regress/expected/with.out
***************
*** 1026,1028 **** SELECT * FROM t;
--- 1026,1110 ----
10
(55 rows)
+ --
+ -- Writeable CTEs with RETURNING
+ --
+ WITH t AS (
+ INSERT INTO y
+ VALUES
+ (11),
+ (12),
+ (13),
+ (14),
+ (15),
+ (16),
+ (17),
+ (18),
+ (19),
+ (20)
+ RETURNING *
+ )
+ SELECT * FROM t;
+ a
+ ----
+ 11
+ 12
+ 13
+ 14
+ 15
+ 16
+ 17
+ 18
+ 19
+ 20
+ (10 rows)
+
+ WITH t AS (
+ UPDATE y
+ SET a=a+1
+ RETURNING *
+ )
+ SELECT * FROM t;
+ a
+ ----
+ 2
+ 3
+ 4
+ 5
+ 6
+ 7
+ 8
+ 9
+ 10
+ 11
+ 12
+ 13
+ 14
+ 15
+ 16
+ 17
+ 18
+ 19
+ 20
+ 21
+ (20 rows)
+
+ WITH t AS (
+ DELETE FROM y
+ WHERE a <= 10
+ RETURNING *
+ )
+ SELECT * FROM t;
+ a
+ ----
+ 2
+ 3
+ 4
+ 5
+ 6
+ 7
+ 8
+ 9
+ 10
+ (9 rows)
+
*** a/src/test/regress/sql/with.sql
--- b/src/test/regress/sql/with.sql
***************
*** 500,502 **** WITH RECURSIVE t(j) AS (
--- 500,537 ----
SELECT j+1 FROM t WHERE j < 10
)
SELECT * FROM t;
+
+ --
+ -- Writeable CTEs with RETURNING
+ --
+
+ WITH t AS (
+ INSERT INTO y
+ VALUES
+ (11),
+ (12),
+ (13),
+ (14),
+ (15),
+ (16),
+ (17),
+ (18),
+ (19),
+ (20)
+ RETURNING *
+ )
+ SELECT * FROM t;
+
+ WITH t AS (
+ UPDATE y
+ SET a=a+1
+ RETURNING *
+ )
+ SELECT * FROM t;
+
+ WITH t AS (
+ DELETE FROM y
+ WHERE a <= 10
+ RETURNING *
+ )
+ SELECT * FROM t;
Argh hit the wrong reply button... ---------- Forwarded message ---------- From: Alex Hunsaker <badalex@gmail.com> Date: Wed, Nov 25, 2009 at 10:20 Subject: Re: [HACKERS] Writeable CTE patch To: Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi> On Mon, Nov 23, 2009 at 14:33, Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi> wrote: > Hi, > > Sorry for the delay, I've been very busy for the last two weeks. > Attached is the latest version of the patch. Heh, sorry about my delay. > The snapshot update code is still the same, I have no good idea what, if > anything, should be done to it. Me neither. > In addition to that, I decided to keep > the code in ExecutePlan() as it was in the last patch. Fine with me. I think I've taken this patch about as far as I can take it. So I'm going to mark it as ready for commiter.
Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi> writes:
> Attached is the latest version of the patch.
I started to look at this patch and soon noted that a very substantial
fraction of the delta had to do with getting rid of dependencies on
estate->es_result_relation_info. It seemed to me that if we were going
to do that, we should try to get rid of the field altogether, so I
looked at what that would take. Unfortunately, there's a problem with
that, which I think proves the patch's approach invalid as well. With
the patch's changes, the only remaining user of es_result_relation_info
is ExecContextForcesOids(), which is only called during plan node
initialization. The patch supposes that it's sufficient to set up
es_result_relation_info while a ModifyTable node is initializing its
child nodes. What this misses is EvalPlanQual, which can require
initialization of a new plan tree during execution. To make it work
we'd need to be sure to set up es_result_relation_info during execution
as well, which pretty much destroys any gain from the proposed
refactoring.
When I realized this, my first thought was that we might as well drop
all the proposed changes that involve avoiding use of
es_result_relation_info. I was wondering though whether you had a
functional reason for getting rid of them, or if it was just trying to
tidy the code a bit?
I did think of a plan B: we could get rid of ExecContextForcesOids()
altogether and let plan nodes always do whatever seems locally most
efficient about OIDs. The consequence of this would be that if we
are doing INSERT or SELECT INTO and the plan tree produces the wrong
has-OIDs state, we would need to insert a junkfilter to fix it, which
would represent processing that would be unnecessary if there was no
other reason to have a junkfilter (ie, no junk columns in the result).
This actually does not affect UPDATE, which always has a junk TID
column so always needs a junkfilter anyway; nor DELETE, which doesn't
need to produce tuples for insertion. At the time we put in
ExecContextForcesOids() it seemed that there was enough of a use-case
to justify klugery to avoid the extra filtering overhead. However,
since OIDs in user tables have been deprecated for several versions
now, I'm thinking that maybe the case doesn't arise often enough to
justify keeping such a wart in the executor.
Comments?
regards, tom lane
Tom Lane wrote: > What this misses is EvalPlanQual, which can require > initialization of a new plan tree during execution. Agh. You're right, I missed that. > When I realized this, my first thought was that we might as well drop > all the proposed changes that involve avoiding use of > es_result_relation_info. I was wondering though whether you had a > functional reason for getting rid of them, or if it was just trying to > tidy the code a bit? The latter. > However, > since OIDs in user tables have been deprecated for several versions > now, I'm thinking that maybe the case doesn't arise often enough to > justify keeping such a wart in the executor. Under the circumstances I'd lean towards this option. Regards, Marko Tiikkaja
Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi> writes:
> Tom Lane wrote:
>> since OIDs in user tables have been deprecated for several versions
>> now, I'm thinking that maybe the case doesn't arise often enough to
>> justify keeping such a wart in the executor.
> Under the circumstances I'd lean towards this option.
I've been fooling around with this further and have gotten as far as
the attached patch. It passes regression tests but suffers from an
additional performance loss: the physical-tlist optimization is disabled
when scanning a relation having OIDs. (That is, we'll always use
ExecProject even if the scan is "SELECT * FROM ...".) I think this loss
is worth worrying about since it would apply to queries on system
catalogs, even if the database has no OIDs in user tables. The trick
is to make the knowledge of the required hasoid state available at
ExecAssignResultType time, so that the plan node's result tupdesc is
constructed correctly.
What seems like the best bet is to merge ExecAssignResultTypeFromTL
and ExecAssignScanProjectionInfo into a single function that should
be used by scan node types. It'll do the determination of whether
a physical-tlist optimization is possible, and then set up both the
output tupdesc and the projection info accordingly. This will make the
patch diff a good bit longer but not much more interesting, so I'm
sending it along at this stage.
I think this is worth doing since it cleans up one of the grottier
parts of executor initialization. The whole thing around
ExecContextForcesOids was never pretty, and it's been the source of
more than one bug if memory serves.
Comments?
regards, tom lane
Index: src/backend/commands/copy.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/copy.c,v
retrieving revision 1.318
diff -c -r1.318 copy.c
*** src/backend/commands/copy.c 20 Nov 2009 20:38:10 -0000 1.318
--- src/backend/commands/copy.c 27 Nov 2009 17:21:55 -0000
***************
*** 1811,1817 ****
estate->es_result_relations = resultRelInfo;
estate->es_num_result_relations = 1;
- estate->es_result_relation_info = resultRelInfo;
/* Set up a tuple slot too */
slot = ExecInitExtraTupleSlot(estate);
--- 1811,1816 ----
***************
*** 2165,2171 ****
heap_insert(cstate->rel, tuple, mycid, hi_options, bistate);
if (resultRelInfo->ri_NumIndices > 0)
! recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self),
estate, false);
/* AFTER ROW INSERT Triggers */
--- 2164,2171 ----
heap_insert(cstate->rel, tuple, mycid, hi_options, bistate);
if (resultRelInfo->ri_NumIndices > 0)
! recheckIndexes = ExecInsertIndexTuples(resultRelInfo,
! slot, &(tuple->t_self),
estate, false);
/* AFTER ROW INSERT Triggers */
Index: src/backend/commands/tablecmds.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.306
diff -c -r1.306 tablecmds.c
*** src/backend/commands/tablecmds.c 20 Nov 2009 20:38:10 -0000 1.306
--- src/backend/commands/tablecmds.c 27 Nov 2009 17:21:55 -0000
***************
*** 941,947 ****
resultRelInfo = resultRelInfos;
foreach(cell, rels)
{
- estate->es_result_relation_info = resultRelInfo;
ExecBSTruncateTriggers(estate, resultRelInfo);
resultRelInfo++;
}
--- 941,946 ----
***************
*** 1009,1015 ****
resultRelInfo = resultRelInfos;
foreach(cell, rels)
{
- estate->es_result_relation_info = resultRelInfo;
ExecASTruncateTriggers(estate, resultRelInfo);
resultRelInfo++;
}
--- 1008,1013 ----
Index: src/backend/commands/vacuum.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/commands/vacuum.c,v
retrieving revision 1.396
diff -c -r1.396 vacuum.c
*** src/backend/commands/vacuum.c 16 Nov 2009 21:32:06 -0000 1.396
--- src/backend/commands/vacuum.c 27 Nov 2009 17:21:55 -0000
***************
*** 181,187 ****
ec->estate->es_result_relations = ec->resultRelInfo;
ec->estate->es_num_result_relations = 1;
- ec->estate->es_result_relation_info = ec->resultRelInfo;
/* Set up a tuple slot too */
ec->slot = MakeSingleTupleTableSlot(tupdesc);
--- 181,186 ----
***************
*** 3099,3105 ****
if (ec->resultRelInfo->ri_NumIndices > 0)
{
ExecStoreTuple(&newtup, ec->slot, InvalidBuffer, false);
! ExecInsertIndexTuples(ec->slot, &(newtup.t_self), ec->estate, true);
ResetPerTupleExprContext(ec->estate);
}
}
--- 3098,3105 ----
if (ec->resultRelInfo->ri_NumIndices > 0)
{
ExecStoreTuple(&newtup, ec->slot, InvalidBuffer, false);
! ExecInsertIndexTuples(ec->resultRelInfo, ec->slot, &(newtup.t_self),
! ec->estate, true);
ResetPerTupleExprContext(ec->estate);
}
}
***************
*** 3225,3231 ****
if (ec->resultRelInfo->ri_NumIndices > 0)
{
ExecStoreTuple(&newtup, ec->slot, InvalidBuffer, false);
! ExecInsertIndexTuples(ec->slot, &(newtup.t_self), ec->estate, true);
ResetPerTupleExprContext(ec->estate);
}
}
--- 3225,3232 ----
if (ec->resultRelInfo->ri_NumIndices > 0)
{
ExecStoreTuple(&newtup, ec->slot, InvalidBuffer, false);
! ExecInsertIndexTuples(ec->resultRelInfo, ec->slot, &(newtup.t_self),
! ec->estate, true);
ResetPerTupleExprContext(ec->estate);
}
}
Index: src/backend/executor/execJunk.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execJunk.c,v
retrieving revision 1.59
diff -c -r1.59 execJunk.c
*** src/backend/executor/execJunk.c 10 Oct 2009 01:43:45 -0000 1.59
--- src/backend/executor/execJunk.c 27 Nov 2009 17:21:56 -0000
***************
*** 34,40 ****
* called 'resjunk'. If the value of this field is true then the
* corresponding attribute is a "junk" attribute.
*
! * When we initialize a plan we call ExecInitJunkFilter to create a filter.
*
* We then execute the plan, treating the resjunk attributes like any others.
*
--- 34,42 ----
* called 'resjunk'. If the value of this field is true then the
* corresponding attribute is a "junk" attribute.
*
! * When we initialize a plan we call ExecInitJunkFilter to create a filter,
! * if one is needed. (We need one either if there are junk attributes,
! * or if we have to add or remove an OID column from the result rowtype.)
*
* We then execute the plan, treating the resjunk attributes like any others.
*
Index: src/backend/executor/execMain.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execMain.c,v
retrieving revision 1.335
diff -c -r1.335 execMain.c
*** src/backend/executor/execMain.c 20 Nov 2009 20:38:10 -0000 1.335
--- src/backend/executor/execMain.c 27 Nov 2009 17:21:56 -0000
***************
*** 339,345 ****
/*
* Close the SELECT INTO relation if any
*/
! if (estate->es_select_into)
CloseIntoRel(queryDesc);
/* do away with our snapshots */
--- 339,345 ----
/*
* Close the SELECT INTO relation if any
*/
! if (queryDesc->dest && queryDesc->dest->mydest == DestIntoRel)
CloseIntoRel(queryDesc);
/* do away with our snapshots */
***************
*** 624,629 ****
--- 624,630 ----
EState *estate = queryDesc->estate;
PlanState *planstate;
TupleDesc tupType;
+ bool select_into;
ListCell *l;
int i;
***************
*** 671,678 ****
}
estate->es_result_relations = resultRelInfos;
estate->es_num_result_relations = numResultRelations;
- /* es_result_relation_info is NULL except when within ModifyTable */
- estate->es_result_relation_info = NULL;
}
else
{
--- 672,677 ----
***************
*** 681,687 ****
*/
estate->es_result_relations = NULL;
estate->es_num_result_relations = 0;
- estate->es_result_relation_info = NULL;
}
/*
--- 680,685 ----
***************
*** 736,751 ****
}
/*
! * Detect whether we're doing SELECT INTO. If so, set the es_into_oids
! * flag appropriately so that the plan tree will be initialized with the
! * correct tuple descriptors. (Other SELECT INTO stuff comes later.)
*/
! estate->es_select_into = false;
! if (operation == CMD_SELECT && plannedstmt->intoClause != NULL)
! {
! estate->es_select_into = true;
! estate->es_into_oids = interpretOidsOption(plannedstmt->intoClause->options);
! }
/*
* Initialize the executor's tuple table to empty.
--- 734,742 ----
}
/*
! * Detect whether we're doing SELECT INTO.
*/
! select_into = (operation == CMD_SELECT && plannedstmt->intoClause != NULL);
/*
* Initialize the executor's tuple table to empty.
***************
*** 804,825 ****
tupType = ExecGetResultType(planstate);
/*
! * Initialize the junk filter if needed. SELECT queries need a
! * filter if there are any junk attrs in the top-level tlist.
*/
if (operation == CMD_SELECT)
{
bool junk_filter_needed = false;
! ListCell *tlist;
! foreach(tlist, plan->targetlist)
{
! TargetEntry *tle = (TargetEntry *) lfirst(tlist);
! if (tle->resjunk)
{
! junk_filter_needed = true;
! break;
}
}
--- 795,832 ----
tupType = ExecGetResultType(planstate);
/*
! * Initialize the junk filter if needed. SELECT queries need a filter
! * if there are any junk attrs in the top-level tlist, or if we're doing
! * SELECT INTO and the result type doesn't match the required hasOids
! * status.
*/
if (operation == CMD_SELECT)
{
bool junk_filter_needed = false;
! bool junk_filter_hasoid = false;
! if (select_into)
{
! /* output hasoid state must match the expected rel option */
! junk_filter_hasoid =
! interpretOidsOption(plannedstmt->intoClause->options);
! if (tupType->tdhasoid != junk_filter_hasoid)
! junk_filter_needed = true;
! }
! if (!junk_filter_needed)
! {
! ListCell *tlist;
!
! foreach(tlist, plan->targetlist)
{
! TargetEntry *tle = (TargetEntry *) lfirst(tlist);
!
! if (tle->resjunk)
! {
! junk_filter_needed = true;
! break;
! }
}
}
***************
*** 828,834 ****
JunkFilter *j;
j = ExecInitJunkFilter(planstate->plan->targetlist,
! tupType->tdhasoid,
ExecInitExtraTupleSlot(estate));
estate->es_junkFilter = j;
--- 835,841 ----
JunkFilter *j;
j = ExecInitJunkFilter(planstate->plan->targetlist,
! junk_filter_hasoid,
ExecInitExtraTupleSlot(estate));
estate->es_junkFilter = j;
***************
*** 847,853 ****
*
* If EXPLAIN, skip creating the "into" relation.
*/
! if (estate->es_select_into && !(eflags & EXEC_FLAG_EXPLAIN_ONLY))
OpenIntoRel(queryDesc);
}
--- 854,860 ----
*
* If EXPLAIN, skip creating the "into" relation.
*/
! if (select_into && !(eflags & EXEC_FLAG_EXPLAIN_ONLY))
OpenIntoRel(queryDesc);
}
***************
*** 1010,1072 ****
return rInfo;
}
- /*
- * ExecContextForcesOids
- *
- * This is pretty grotty: when doing INSERT, UPDATE, or SELECT INTO,
- * we need to ensure that result tuples have space for an OID iff they are
- * going to be stored into a relation that has OIDs. In other contexts
- * we are free to choose whether to leave space for OIDs in result tuples
- * (we generally don't want to, but we do if a physical-tlist optimization
- * is possible). This routine checks the plan context and returns TRUE if the
- * choice is forced, FALSE if the choice is not forced. In the TRUE case,
- * *hasoids is set to the required value.
- *
- * One reason this is ugly is that all plan nodes in the plan tree will emit
- * tuples with space for an OID, though we really only need the topmost node
- * to do so. However, node types like Sort don't project new tuples but just
- * return their inputs, and in those cases the requirement propagates down
- * to the input node. Eventually we might make this code smart enough to
- * recognize how far down the requirement really goes, but for now we just
- * make all plan nodes do the same thing if the top level forces the choice.
- *
- * We assume that if we are generating tuples for INSERT or UPDATE,
- * estate->es_result_relation_info is already set up to describe the target
- * relation. Note that in an UPDATE that spans an inheritance tree, some of
- * the target relations may have OIDs and some not. We have to make the
- * decisions on a per-relation basis as we initialize each of the subplans of
- * the ModifyTable node, so ModifyTable has to set es_result_relation_info
- * while initializing each subplan.
- *
- * SELECT INTO is even uglier, because we don't have the INTO relation's
- * descriptor available when this code runs; we have to look aside at a
- * flag set by InitPlan().
- */
- bool
- ExecContextForcesOids(PlanState *planstate, bool *hasoids)
- {
- ResultRelInfo *ri = planstate->state->es_result_relation_info;
-
- if (ri != NULL)
- {
- Relation rel = ri->ri_RelationDesc;
-
- if (rel != NULL)
- {
- *hasoids = rel->rd_rel->relhasoids;
- return true;
- }
- }
-
- if (planstate->state->es_select_into)
- {
- *hasoids = planstate->state->es_into_oids;
- return true;
- }
-
- return false;
- }
-
/* ----------------------------------------------------------------
* ExecEndPlan
*
--- 1017,1022 ----
***************
*** 1887,1898 ****
estate->es_output_cid = parentestate->es_output_cid;
estate->es_result_relations = parentestate->es_result_relations;
estate->es_num_result_relations = parentestate->es_num_result_relations;
- estate->es_result_relation_info = parentestate->es_result_relation_info;
/* es_trig_target_relations must NOT be copied */
estate->es_rowMarks = parentestate->es_rowMarks;
estate->es_instrument = parentestate->es_instrument;
- estate->es_select_into = parentestate->es_select_into;
- estate->es_into_oids = parentestate->es_into_oids;
/*
* The external param list is simply shared from parent. The internal
--- 1837,1845 ----
Index: src/backend/executor/execScan.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execScan.c,v
retrieving revision 1.47
diff -c -r1.47 execScan.c
*** src/backend/executor/execScan.c 26 Oct 2009 02:26:29 -0000 1.47
--- src/backend/executor/execScan.c 27 Nov 2009 17:21:56 -0000
***************
*** 23,29 ****
#include "utils/memutils.h"
! static bool tlist_matches_tupdesc(PlanState *ps, List *tlist, Index varno, TupleDesc tupdesc);
/*
--- 23,30 ----
#include "utils/memutils.h"
! static bool tlist_matches_tupdesc(List *tlist, Index varno, TupleDesc tupdesc,
! bool resulthasoid);
/*
***************
*** 235,263 ****
* the scan node, because the planner will preferentially generate a matching
* tlist.
*
! * ExecAssignScanType must have been called already.
*/
void
ExecAssignScanProjectionInfo(ScanState *node)
{
Scan *scan = (Scan *) node->ps.plan;
! if (tlist_matches_tupdesc(&node->ps,
! scan->plan.targetlist,
scan->scanrelid,
! node->ss_ScanTupleSlot->tts_tupleDescriptor))
node->ps.ps_ProjInfo = NULL;
else
! ExecAssignProjectionInfo(&node->ps,
! node->ss_ScanTupleSlot->tts_tupleDescriptor);
}
static bool
! tlist_matches_tupdesc(PlanState *ps, List *tlist, Index varno, TupleDesc tupdesc)
{
int numattrs = tupdesc->natts;
int attrno;
- bool hasoid;
ListCell *tlist_item = list_head(tlist);
/* Check the tlist attributes */
--- 236,265 ----
* the scan node, because the planner will preferentially generate a matching
* tlist.
*
! * ExecAssignScanType and ExecAssignResultType must have been called already.
*/
void
ExecAssignScanProjectionInfo(ScanState *node)
{
Scan *scan = (Scan *) node->ps.plan;
+ TupleDesc scandesc = node->ss_ScanTupleSlot->tts_tupleDescriptor;
+ TupleDesc resultdesc = node->ps.ps_ResultTupleSlot->tts_tupleDescriptor;
! if (tlist_matches_tupdesc(scan->plan.targetlist,
scan->scanrelid,
! scandesc,
! resultdesc->tdhasoid))
node->ps.ps_ProjInfo = NULL;
else
! ExecAssignProjectionInfo(&node->ps, scandesc);
}
static bool
! tlist_matches_tupdesc(List *tlist, Index varno, TupleDesc tupdesc,
! bool resulthasoid)
{
int numattrs = tupdesc->natts;
int attrno;
ListCell *tlist_item = list_head(tlist);
/* Check the tlist attributes */
***************
*** 301,311 ****
return false; /* tlist too long */
/*
! * If the plan context requires a particular hasoid setting, then that has
! * to match, too.
*/
! if (ExecContextForcesOids(ps, &hasoid) &&
! hasoid != tupdesc->tdhasoid)
return false;
return true;
--- 303,311 ----
return false; /* tlist too long */
/*
! * The hasOids state has to match the output rowtype, too.
*/
! if (tupdesc->tdhasoid != resulthasoid)
return false;
return true;
Index: src/backend/executor/execUtils.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/execUtils.c,v
retrieving revision 1.166
diff -c -r1.166 execUtils.c
*** src/backend/executor/execUtils.c 20 Nov 2009 20:38:10 -0000 1.166
--- src/backend/executor/execUtils.c 27 Nov 2009 17:21:56 -0000
***************
*** 113,119 ****
estate->es_result_relations = NULL;
estate->es_num_result_relations = 0;
- estate->es_result_relation_info = NULL;
estate->es_trig_target_relations = NIL;
estate->es_trig_tuple_slot = NULL;
--- 113,118 ----
***************
*** 132,139 ****
estate->es_lastoid = InvalidOid;
estate->es_instrument = false;
- estate->es_select_into = false;
- estate->es_into_oids = false;
estate->es_exprcontexts = NIL;
--- 131,136 ----
***************
*** 440,454 ****
bool hasoid;
TupleDesc tupDesc;
! if (ExecContextForcesOids(planstate, &hasoid))
! {
! /* context forces OID choice; hasoid is now set correctly */
! }
! else
! {
! /* given free choice, don't leave space for OIDs in result tuples */
! hasoid = false;
! }
/*
* ExecTypeFromTL needs the parse-time representation of the tlist, not a
--- 437,450 ----
bool hasoid;
TupleDesc tupDesc;
! /*
! * If the plan is a scan node on a relation that has OIDs, it would be
! * desirable to allow OIDs in the output rowtype, so that a physical
! * tlist optimization is possible. It would require some refactoring
! * to make that knowledge available here, however, so for the moment
! * just set hasoid = false always.
! */
! hasoid = false;
/*
* ExecTypeFromTL needs the parse-time representation of the tlist, not a
***************
*** 969,981 ****
* ----------------------------------------------------------------
*/
List *
! ExecInsertIndexTuples(TupleTableSlot *slot,
ItemPointer tupleid,
EState *estate,
bool is_vacuum_full)
{
List *result = NIL;
- ResultRelInfo *resultRelInfo;
int i;
int numIndices;
RelationPtr relationDescs;
--- 965,977 ----
* ----------------------------------------------------------------
*/
List *
! ExecInsertIndexTuples(ResultRelInfo *resultRelInfo,
! TupleTableSlot *slot,
ItemPointer tupleid,
EState *estate,
bool is_vacuum_full)
{
List *result = NIL;
int i;
int numIndices;
RelationPtr relationDescs;
***************
*** 988,994 ****
/*
* Get information from the result relation info structure.
*/
- resultRelInfo = estate->es_result_relation_info;
numIndices = resultRelInfo->ri_NumIndices;
relationDescs = resultRelInfo->ri_IndexRelationDescs;
indexInfoArray = resultRelInfo->ri_IndexRelationInfo;
--- 984,989 ----
Index: src/backend/executor/nodeModifyTable.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/executor/nodeModifyTable.c,v
retrieving revision 1.3
diff -c -r1.3 nodeModifyTable.c
*** src/backend/executor/nodeModifyTable.c 20 Nov 2009 20:38:10 -0000 1.3
--- src/backend/executor/nodeModifyTable.c 27 Nov 2009 17:21:56 -0000
***************
*** 158,169 ****
* ----------------------------------------------------------------
*/
static TupleTableSlot *
! ExecInsert(TupleTableSlot *slot,
TupleTableSlot *planSlot,
EState *estate)
{
HeapTuple tuple;
- ResultRelInfo *resultRelInfo;
Relation resultRelationDesc;
Oid newId;
List *recheckIndexes = NIL;
--- 158,169 ----
* ----------------------------------------------------------------
*/
static TupleTableSlot *
! ExecInsert(ResultRelInfo *resultRelInfo,
! TupleTableSlot *slot,
TupleTableSlot *planSlot,
EState *estate)
{
HeapTuple tuple;
Relation resultRelationDesc;
Oid newId;
List *recheckIndexes = NIL;
***************
*** 177,183 ****
/*
* get information on the (current) result relation
*/
- resultRelInfo = estate->es_result_relation_info;
resultRelationDesc = resultRelInfo->ri_RelationDesc;
/*
--- 177,182 ----
***************
*** 248,254 ****
* insert index entries for tuple
*/
if (resultRelInfo->ri_NumIndices > 0)
! recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self),
estate, false);
/* AFTER ROW INSERT Triggers */
--- 247,254 ----
* insert index entries for tuple
*/
if (resultRelInfo->ri_NumIndices > 0)
! recheckIndexes = ExecInsertIndexTuples(resultRelInfo,
! slot, &(tuple->t_self),
estate, false);
/* AFTER ROW INSERT Triggers */
***************
*** 272,283 ****
* ----------------------------------------------------------------
*/
static TupleTableSlot *
! ExecDelete(ItemPointer tupleid,
TupleTableSlot *planSlot,
EPQState *epqstate,
EState *estate)
{
- ResultRelInfo *resultRelInfo;
Relation resultRelationDesc;
HTSU_Result result;
ItemPointerData update_ctid;
--- 272,283 ----
* ----------------------------------------------------------------
*/
static TupleTableSlot *
! ExecDelete(ResultRelInfo *resultRelInfo,
! ItemPointer tupleid,
TupleTableSlot *planSlot,
EPQState *epqstate,
EState *estate)
{
Relation resultRelationDesc;
HTSU_Result result;
ItemPointerData update_ctid;
***************
*** 286,292 ****
/*
* get information on the (current) result relation
*/
- resultRelInfo = estate->es_result_relation_info;
resultRelationDesc = resultRelInfo->ri_RelationDesc;
/* BEFORE ROW DELETE Triggers */
--- 286,291 ----
***************
*** 415,428 ****
* ----------------------------------------------------------------
*/
static TupleTableSlot *
! ExecUpdate(ItemPointer tupleid,
TupleTableSlot *slot,
TupleTableSlot *planSlot,
EPQState *epqstate,
EState *estate)
{
HeapTuple tuple;
- ResultRelInfo *resultRelInfo;
Relation resultRelationDesc;
HTSU_Result result;
ItemPointerData update_ctid;
--- 414,427 ----
* ----------------------------------------------------------------
*/
static TupleTableSlot *
! ExecUpdate(ResultRelInfo *resultRelInfo,
! ItemPointer tupleid,
TupleTableSlot *slot,
TupleTableSlot *planSlot,
EPQState *epqstate,
EState *estate)
{
HeapTuple tuple;
Relation resultRelationDesc;
HTSU_Result result;
ItemPointerData update_ctid;
***************
*** 444,450 ****
/*
* get information on the (current) result relation
*/
- resultRelInfo = estate->es_result_relation_info;
resultRelationDesc = resultRelInfo->ri_RelationDesc;
/* BEFORE ROW UPDATE Triggers */
--- 443,448 ----
***************
*** 563,569 ****
* If it's a HOT update, we mustn't insert new index entries.
*/
if (resultRelInfo->ri_NumIndices > 0 && !HeapTupleIsHeapOnly(tuple))
! recheckIndexes = ExecInsertIndexTuples(slot, &(tuple->t_self),
estate, false);
/* AFTER ROW UPDATE Triggers */
--- 561,568 ----
* If it's a HOT update, we mustn't insert new index entries.
*/
if (resultRelInfo->ri_NumIndices > 0 && !HeapTupleIsHeapOnly(tuple))
! recheckIndexes = ExecInsertIndexTuples(resultRelInfo,
! slot, &(tuple->t_self),
estate, false);
/* AFTER ROW UPDATE Triggers */
***************
*** 645,650 ****
--- 644,650 ----
EState *estate = node->ps.state;
CmdType operation = node->operation;
PlanState *subplanstate;
+ ResultRelInfo *resultRelInfo;
JunkFilter *junkfilter;
TupleTableSlot *slot;
TupleTableSlot *planSlot;
***************
*** 660,676 ****
node->fireBSTriggers = false;
}
- /*
- * es_result_relation_info must point to the currently active result
- * relation. (Note we assume that ModifyTable nodes can't be nested.)
- * We want it to be NULL whenever we're not within ModifyTable, though.
- */
- estate->es_result_relation_info =
- estate->es_result_relations + node->mt_whichplan;
-
/* Preload local variables */
subplanstate = node->mt_plans[node->mt_whichplan];
! junkfilter = estate->es_result_relation_info->ri_junkFilter;
/*
* Fetch rows from subplan(s), and execute the required table modification
--- 660,669 ----
node->fireBSTriggers = false;
}
/* Preload local variables */
subplanstate = node->mt_plans[node->mt_whichplan];
! resultRelInfo = estate->es_result_relations + node->mt_whichplan;
! junkfilter = resultRelInfo->ri_junkFilter;
/*
* Fetch rows from subplan(s), and execute the required table modification
***************
*** 686,694 ****
node->mt_whichplan++;
if (node->mt_whichplan < node->mt_nplans)
{
- estate->es_result_relation_info++;
subplanstate = node->mt_plans[node->mt_whichplan];
! junkfilter = estate->es_result_relation_info->ri_junkFilter;
EvalPlanQualSetPlan(&node->mt_epqstate, subplanstate->plan);
continue;
}
--- 679,687 ----
node->mt_whichplan++;
if (node->mt_whichplan < node->mt_nplans)
{
subplanstate = node->mt_plans[node->mt_whichplan];
! resultRelInfo = estate->es_result_relations + node->mt_whichplan;
! junkfilter = resultRelInfo->ri_junkFilter;
EvalPlanQualSetPlan(&node->mt_epqstate, subplanstate->plan);
continue;
}
***************
*** 730,743 ****
switch (operation)
{
case CMD_INSERT:
! slot = ExecInsert(slot, planSlot, estate);
break;
case CMD_UPDATE:
! slot = ExecUpdate(tupleid, slot, planSlot,
&node->mt_epqstate, estate);
break;
case CMD_DELETE:
! slot = ExecDelete(tupleid, planSlot,
&node->mt_epqstate, estate);
break;
default:
--- 723,739 ----
switch (operation)
{
case CMD_INSERT:
! slot = ExecInsert(resultRelInfo,
! slot, planSlot, estate);
break;
case CMD_UPDATE:
! slot = ExecUpdate(resultRelInfo,
! tupleid, slot, planSlot,
&node->mt_epqstate, estate);
break;
case CMD_DELETE:
! slot = ExecDelete(resultRelInfo,
! tupleid, planSlot,
&node->mt_epqstate, estate);
break;
default:
***************
*** 750,764 ****
* the work on next call.
*/
if (slot)
- {
- estate->es_result_relation_info = NULL;
return slot;
- }
}
- /* Reset es_result_relation_info before exiting */
- estate->es_result_relation_info = NULL;
-
/*
* We're done, but fire AFTER STATEMENT triggers before exiting.
*/
--- 746,754 ----
***************
*** 813,832 ****
/*
* call ExecInitNode on each of the plans to be executed and save the
! * results into the array "mt_plans". Note we *must* set
! * estate->es_result_relation_info correctly while we initialize each
! * sub-plan; ExecContextForcesOids depends on that!
*/
- estate->es_result_relation_info = estate->es_result_relations;
i = 0;
foreach(l, node->plans)
{
subplan = (Plan *) lfirst(l);
mtstate->mt_plans[i] = ExecInitNode(subplan, estate, eflags);
- estate->es_result_relation_info++;
i++;
}
- estate->es_result_relation_info = NULL;
/* select first subplan */
mtstate->mt_whichplan = 0;
--- 803,817 ----
/*
* call ExecInitNode on each of the plans to be executed and save the
! * results into the array "mt_plans".
*/
i = 0;
foreach(l, node->plans)
{
subplan = (Plan *) lfirst(l);
mtstate->mt_plans[i] = ExecInitNode(subplan, estate, eflags);
i++;
}
/* select first subplan */
mtstate->mt_whichplan = 0;
***************
*** 921,951 ****
/*
* Initialize the junk filter(s) if needed. INSERT queries need a filter
! * if there are any junk attrs in the tlist. UPDATE and DELETE
! * always need a filter, since there's always a junk 'ctid' attribute
! * present --- no need to look first.
*
* If there are multiple result relations, each one needs its own junk
! * filter. Note multiple rels are only possible for UPDATE/DELETE, so we
! * can't be fooled by some needing a filter and some not.
*
* This section of code is also a convenient place to verify that the
* output of an INSERT or UPDATE matches the target table(s).
*/
{
bool junk_filter_needed = false;
switch (operation)
{
case CMD_INSERT:
! foreach(l, subplan->targetlist)
{
! TargetEntry *tle = (TargetEntry *) lfirst(l);
!
! if (tle->resjunk)
{
! junk_filter_needed = true;
! break;
}
}
break;
--- 906,958 ----
/*
* Initialize the junk filter(s) if needed. INSERT queries need a filter
! * if there are any junk attrs in the tlist, or if we have to add or
! * remove an OID column. UPDATE and DELETE always need a filter, since
! * there's always a junk 'ctid' attribute present --- no need to look
! * first. (For DELETE, we won't actually use the filter to filter tuples,
! * but we still need it for extracting the ctid attribute.)
*
* If there are multiple result relations, each one needs its own junk
! * filter.
*
* This section of code is also a convenient place to verify that the
* output of an INSERT or UPDATE matches the target table(s).
*/
+ resultRelInfo = estate->es_result_relations;
+ for (i = 0; i < nplans; i++)
{
bool junk_filter_needed = false;
+ bool junk_filter_hasoid = false;
+
+ subplan = mtstate->mt_plans[i]->plan;
+
+ if (operation == CMD_INSERT || operation == CMD_UPDATE)
+ {
+ TupleDesc tupType;
+
+ ExecCheckPlanOutput(resultRelInfo->ri_RelationDesc,
+ subplan->targetlist);
+ /* output hasoid state must match the rel */
+ junk_filter_hasoid = resultRelInfo->ri_RelationDesc->rd_att->tdhasoid;
+ tupType = ExecGetResultType(mtstate->mt_plans[i]);
+ if (tupType->tdhasoid != junk_filter_hasoid)
+ junk_filter_needed = true;
+ }
switch (operation)
{
case CMD_INSERT:
! if (!junk_filter_needed)
{
! foreach(l, subplan->targetlist)
{
! TargetEntry *tle = (TargetEntry *) lfirst(l);
!
! if (tle->resjunk)
! {
! junk_filter_needed = true;
! break;
! }
}
}
break;
***************
*** 960,997 ****
if (junk_filter_needed)
{
! resultRelInfo = estate->es_result_relations;
! for (i = 0; i < nplans; i++)
! {
! JunkFilter *j;
!
! subplan = mtstate->mt_plans[i]->plan;
! if (operation == CMD_INSERT || operation == CMD_UPDATE)
! ExecCheckPlanOutput(resultRelInfo->ri_RelationDesc,
! subplan->targetlist);
!
! j = ExecInitJunkFilter(subplan->targetlist,
! resultRelInfo->ri_RelationDesc->rd_att->tdhasoid,
! ExecInitExtraTupleSlot(estate));
! if (operation == CMD_UPDATE || operation == CMD_DELETE)
! {
! /* For UPDATE/DELETE, find the ctid junk attr now */
! j->jf_junkAttNo = ExecFindJunkAttribute(j, "ctid");
! if (!AttributeNumberIsValid(j->jf_junkAttNo))
! elog(ERROR, "could not find junk ctid column");
! }
!
! resultRelInfo->ri_junkFilter = j;
! resultRelInfo++;
}
}
! else
! {
! if (operation == CMD_INSERT)
! ExecCheckPlanOutput(estate->es_result_relations->ri_RelationDesc,
! subplan->targetlist);
! }
}
/*
--- 967,989 ----
if (junk_filter_needed)
{
! JunkFilter *j;
! j = ExecInitJunkFilter(subplan->targetlist,
! junk_filter_hasoid,
! ExecInitExtraTupleSlot(estate));
! if (operation == CMD_UPDATE || operation == CMD_DELETE)
! {
! /* For UPDATE/DELETE, find the ctid junk attr now */
! j->jf_junkAttNo = ExecFindJunkAttribute(j, "ctid");
! if (!AttributeNumberIsValid(j->jf_junkAttNo))
! elog(ERROR, "could not find junk ctid column");
}
+
+ resultRelInfo->ri_junkFilter = j;
}
!
! resultRelInfo++;
}
/*
Index: src/include/executor/executor.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/executor/executor.h,v
retrieving revision 1.163
diff -c -r1.163 executor.h
*** src/include/executor/executor.h 26 Oct 2009 02:26:41 -0000 1.163
--- src/include/executor/executor.h 27 Nov 2009 17:21:56 -0000
***************
*** 163,169 ****
CmdType operation,
bool doInstrument);
extern ResultRelInfo *ExecGetTriggerResultRel(EState *estate, Oid relid);
- extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids);
extern void ExecConstraints(ResultRelInfo *resultRelInfo,
TupleTableSlot *slot, EState *estate);
extern TupleTableSlot *EvalPlanQual(EState *estate, EPQState *epqstate,
--- 163,168 ----
***************
*** 319,325 ****
extern void ExecOpenIndices(ResultRelInfo *resultRelInfo);
extern void ExecCloseIndices(ResultRelInfo *resultRelInfo);
! extern List *ExecInsertIndexTuples(TupleTableSlot *slot, ItemPointer tupleid,
EState *estate, bool is_vacuum_full);
extern void RegisterExprContextCallback(ExprContext *econtext,
--- 318,325 ----
extern void ExecOpenIndices(ResultRelInfo *resultRelInfo);
extern void ExecCloseIndices(ResultRelInfo *resultRelInfo);
! extern List *ExecInsertIndexTuples(ResultRelInfo *resultRelInfo,
! TupleTableSlot *slot, ItemPointer tupleid,
EState *estate, bool is_vacuum_full);
extern void RegisterExprContextCallback(ExprContext *econtext,
Index: src/include/nodes/execnodes.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/nodes/execnodes.h,v
retrieving revision 1.212
diff -c -r1.212 execnodes.h
*** src/include/nodes/execnodes.h 20 Nov 2009 20:38:11 -0000 1.212
--- src/include/nodes/execnodes.h 27 Nov 2009 17:21:56 -0000
***************
*** 340,349 ****
/* If query can insert/delete tuples, the command ID to mark them with */
CommandId es_output_cid;
! /* Info about target table for insert/update/delete queries: */
ResultRelInfo *es_result_relations; /* array of ResultRelInfos */
int es_num_result_relations; /* length of array */
- ResultRelInfo *es_result_relation_info; /* currently active array elt */
/* Stuff used for firing triggers: */
List *es_trig_target_relations; /* trigger-only ResultRelInfos */
--- 340,348 ----
/* If query can insert/delete tuples, the command ID to mark them with */
CommandId es_output_cid;
! /* Info about target tables for insert/update/delete queries: */
ResultRelInfo *es_result_relations; /* array of ResultRelInfos */
int es_num_result_relations; /* length of array */
/* Stuff used for firing triggers: */
List *es_trig_target_relations; /* trigger-only ResultRelInfos */
***************
*** 365,372 ****
Oid es_lastoid; /* last oid processed (by INSERT) */
bool es_instrument; /* true requests runtime instrumentation */
- bool es_select_into; /* true if doing SELECT INTO */
- bool es_into_oids; /* true to generate OIDs in SELECT INTO */
List *es_exprcontexts; /* List of ExprContexts within EState */
--- 364,369 ----
I wrote:
> I think this is worth doing since it cleans up one of the grottier
> parts of executor initialization. The whole thing around
> ExecContextForcesOids was never pretty, and it's been the source of
> more than one bug if memory serves.
On further review there's a really serious stumbling block here.
ConsiderINSERT INTO t1 SELECT * FROM t2 UNION ALL SELECT * FROM t3
where the three tables all have the same user columns but t2 has
OIDs and t3 not (or vice versa). Without ExecContextForcesOids
or something very much like it, both scan nodes will think they
can return physical tuples. The output of the Append node will
therefore contain some tuples with OIDs and some without. Append
itself can't fix that since it doesn't project. In many queries
this would not matter --- but if we are inserting them directly
into t1 without any further filtering, it does matter.
I can imagine various ways around this, but it's not clear that
any of them are much less grotty than the code is now. In any
case this was just a marginal code cleanup idea and it doesn't
seem worth spending so much time on right now.
I'm going to go back to plan A: drop the es_result_relation_info
changes from the patch.
regards, tom lane
Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi> writes: > Attached is the latest version of the patch. I looked through this patch and concluded that it still needs a fair amount of work, so I'm bouncing it back for further work. 1. I thought we'd agreed at http://archives.postgresql.org/pgsql-hackers/2009-10/msg00558.php that the patch should support WITH on DML statements, egwith (some-query) insert into foo ... This might not take much more than grammar additions, but it's definitely lacking at the moment. 2. The handling of rules on DML WITH queries is far short of sufficient. AFAICT, what it's doing is rewriting the query, then taking the first or last element of the resulting query list as replacing the WITH query, and adding the rest of the list after or before the main query. This does not work at all for cases involving conditional DO INSTEAD rules, since there could be more than one element of the resulting query list that's responsible for delivering results depending on the runtime outcome of the condition. I don't think it works for unconditional DO INSTEAD either, since the rule producing output might not be the first or last one. And in any case it fails to satisfy the POLA in regards to the order of execution of DO ALSO queries relative to other WITH queries or the main query. I am not sure that it is possible to fix this without really drastic surgery on the rule mechanisms. Or maybe we ought to rethink what the representation of DML WITH queries is. Perhaps it would be acceptable to just throw ERROR_FEATURE_NOT_SUPPORTED when there are DO ALSO or conditional DO INSTEAD rules applying to the target of a DML WITH query. I wouldn't normally think that just blowing off such a thing meets the project's quality standards, but we all know that the current rule mechanism is in need of a ground-up redesign anyway. It's hard to justify putting a lot of work into making it work with DML WITH queries when we might be throwing it all out in the future. One thing that really does have to draw an error is that AFAIR the current rule feature doesn't enforce that a rewritten query produce the same type of output that the original would have. We just ship off whatever the results are to the client, and let it sort everything out. In a DML WITH query, though, I think we do have to insist that the rewritten query(s) still produce the same RETURNING rowtype as before. 3. I'm pretty unimpressed with the code added to ExecutePlan. It knows way more than it ought to about CTEs, and yet I don't think it's doing the right things anyway --- in particular, won't it run the "leader" CTE more than once if one CTE references another? I think it would be better if the PlannedStmt representation just told ExecutePlan what to do, rather than having all these heuristics inside ExecutePlan. (BTW, I also think it would work better if you had the CommandCounterIncrement at the bottom of the loop, after the subquery execution not before it. But I'm not sure it's safe for ExecutePlan to be modifying the snapshot it's handed anyway.) I wonder whether it would be practical to fix both #2 and #3 by having the representation of DML WITH queries look more like the representation of rule rewrite output --- that is, generate a list of top-level Queries not one Query with DML subqueries in its CTE list. The main thing that seems to be missing in order to allow that is for a Query to refer back to the output of a previous Query in the list. This doesn't seem tremendously hard at runtime --- it's just a tuplestore to keep around --- but I'm not clear what it ought to look like in terms of the parsetree representation. 4. As previously noted, the changes to avoid using es_result_relation_info are broken and need to be dropped from the patch. regards, tom lane
On Sat, Nov 28, 2009 at 11:59, Tom Lane <tgl@sss.pgh.pa.us> wrote: > 1. I thought we'd agreed at > http://archives.postgresql.org/pgsql-hackers/2009-10/msg00558.php > that the patch should support WITH on DML statements, eg > with (some-query) insert into foo ... > This might not take much more than grammar additions, but it's > definitely lacking at the moment. Hrm ? A few messages down you say SELECT should be a good start http://archives.postgresql.org/pgsql-hackers/2009-10/msg01081.php > 2. The handling of rules on DML WITH queries is far short of sufficient. Ick. > Perhaps it would be acceptable to just throw ERROR_FEATURE_NOT_SUPPORTED > when there are DO ALSO or conditional DO INSTEAD rules applying to the > target of a DML WITH query. +1 > 3. I'm pretty unimpressed with the code added to ExecutePlan. > I wonder whether it would be practical to fix both #2 and #3 by having the > representation of DML WITH queries look more like the representation of > rule rewrite output Interesting... This seems like the best solution ( assuming its workable ). It also looks like it might make #1 easier as well. However, I think the current approach does have some virtue in that I was surprised how little the patch was. Granted that is partly due to ExecutePlan knowing to much.
Tom Lane wrote: > 1. I thought we'd agreed at > http://archives.postgresql.org/pgsql-hackers/2009-10/msg00558.php > that the patch should support WITH on DML statements, eg > with (some-query) insert into foo ... > This might not take much more than grammar additions, but it's > definitely lacking at the moment. Ok, I added these. > One thing that really does have to draw an error is that AFAIR the current > rule feature doesn't enforce that a rewritten query produce the same type > of output that the original would have. We just ship off whatever the > results are to the client, and let it sort everything out. In a DML WITH > query, though, I think we do have to insist that the rewritten query(s) > still produce the same RETURNING rowtype as before. Agreed. > 3. I'm pretty unimpressed with the code added to ExecutePlan. It knows > way more than it ought to about CTEs, and yet I don't think it's doing the > right things anyway --- in particular, won't it run the "leader" CTE more > than once if one CTE references another? Yes. Are you suggesting something more intelligent to avoid scanning the CTE more than once or..? > I think it would be better if > the PlannedStmt representation just told ExecutePlan what to do, rather > than having all these heuristics inside ExecutePlan. Yup, seems like a better choice. > (BTW, I also think > it would work better if you had the CommandCounterIncrement at the bottom > of the loop, after the subquery execution not before it. But I'm not sure > it's safe for ExecutePlan to be modifying the snapshot it's handed anyway.) Agreed. I'm a bit lost here with the snapshot business; is doing this work in ExecutePlan() out of the question or is it just that what I'm doing is wrong? > 4. As previously noted, the changes to avoid using es_result_relation_info > are broken and need to be dropped from the patch. Done. I kept the logic for result relations to allow nested ModifyTable nodes, but I don't think it ever did the right thing with EvalPlanQual() and nested nodes. I'll have think about that. Regards, Marko Tiikkaja
Marko Tiikkaja <marko.tiikkaja@cs.helsinki.fi> writes:
> Tom Lane wrote:
>> (BTW, I also think
>> it would work better if you had the CommandCounterIncrement at the bottom
>> of the loop, after the subquery execution not before it. But I'm not sure
>> it's safe for ExecutePlan to be modifying the snapshot it's handed anyway.)
> Agreed. I'm a bit lost here with the snapshot business; is doing this
> work in ExecutePlan() out of the question or is it just that what I'm
> doing is wrong?
I think it's not a good idea for ExecutePlan to be scribbling on the
executor's input, and the provided snapshot is definitely an input.
It might accidentally fail to fail in the present system, but it would
always be a hazard. The only thing that I'd be comfortable with is
copying the snap and modifying the copy. This might be okay from a
performance standpoint if it's done at the bottom of the loop (ie,
only when you actually have at least one writable CTE). It would be
altogether cleaner though if the CommandCounterIncrement responsibility
were in the same place it is now, ie the caller of the executor. Which
could be possible if we restructure the rewriter/planner output as a
list of Queries instead of just one. I'm not currently sure how hard
that would be, though; it might not be a practical answer.
regards, tom lane
Tom Lane wrote: > It would be > altogether cleaner though if the CommandCounterIncrement responsibility > were in the same place it is now, ie the caller of the executor. Which > could be possible if we restructure the rewriter/planner output as a > list of Queries instead of just one. I'm not currently sure how hard > that would be, though; it might not be a practical answer. I'm trying to avoid doing this, at least for now. Regards, Marko Tiikkaja
Tom Lane wrote: > The only thing that I'd be comfortable with is > copying the snap and modifying the copy. I don't see an easy way to do that with the current code; CopySnapshot() is static and PushUpdatedSnapshot() seems to be a bit of a pain since it messes up some of the existing code which uses the active snapshot stack. Any ideas? Regards, Marko Tiikkaja
Marko Tiikkaja escribió: > Tom Lane wrote: > >The only thing that I'd be comfortable with is > >copying the snap and modifying the copy. > > I don't see an easy way to do that with the current code; > CopySnapshot() is static and PushUpdatedSnapshot() seems to be a bit > of a pain since it messes up some of the existing code which uses > the active snapshot stack. Any ideas? That API is rather new. Maybe we need a new entry point, say GetActiveSnapshotCopy or some such. -- Alvaro Herrera http://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support