-----------------------------------
Problem I'm trying to solve:
For partitioned tables, make it possible to use RETURNING clause on INSERT
INTO
together with DO INSTEAD rule
[ Note - wherever I say INSERT I also mean UPDATE and DELETE ]
-----------------------------------
Current behaviour :
An INSERT which has a RETURNING clause and which is to be rewritten based on
a rule will be accepted if the rule is an "unconditional DO INSTEAD".
In general I believe "unconditional" means "no WHERE clause", but in
practice
if the rule is of the form
CREATE RULE insert_part_history as ON INSERT to history \
DO INSTEAD SELECT history_insert_partitioned(NEW) returning NEW.id
this is treated as conditional and the query is rejected.
Testcase:
A table T is partitioned and has two or more columns, one of which
is an id column declared as
id bigint DEFAULT nextval('history_id_seq'::regclass) NOT NULL
and the application issues
"INSERT into history (column-list which excludes id) values (....)
RETURNING id"
I can get the re-direction of the INSERT *without* RETURNING to work using
either trigger or rule, in which the trigger/rule invokes a procedure, but
whichever way I do it, I could not get this RETURNING clause to work.
For a trigger,
the INSERT ... RETURNING was accepted but returned no rows, (as I would
expect), and for the RULE, the INSERT ... RETURNING was rejected with :
ERROR: cannot perform INSERT RETURNING on relation "history"
HINT: You need an unconditional ON INSERT DO INSTEAD rule with a
RETURNING clause.
but this hint was not much help, since :
For a rule,
CREATE RULE insert_part_history as ON INSERT to history \
DO INSTEAD SELECT history_insert_partitioned(NEW) returning NEW.id
ERROR: syntax error at or near "returning"
LINE 1: ...DO INSTEAD SELECT history_insert_partitioned(NEW) returning ...
Here the function history_insert_partitioned is something like
CREATE FUNCTION history_insert_partitioned( NEW public.history) RETURNS
BIGINT AS $$
DECLARE
...
BEGIN
...
< acccess NEW fields e.g. timestamp>
< construct partitioned table name>
< EXECUTE 'INSERT INTO ' partitioned table
...
RETURN history_id;
END;
$$
LANGUAGE plpgsql
-----------------------------------
Some references to discussion of this requirement :
. http://wiki.postgresql.org/wiki/Todo
item "Make it possible to use RETURNING together with conditional DO
INSTEAD rules,
such as for partitioning setups"
. http://archives.postgresql.org/pgsql-general/2012-06/msg00377.php
. http://archives.postgresql.org/pgsql-general/2010-12/msg00542.php
.
http://acodapella.blogspot.it/2011/06/hibernate-postgresql-table-partitioning.html
-----------------------------------
Proposal:
. I propose to extend the rule system to recognize cases where the INSERT
query specifies
RETURNING and the rule promises to return a row, and to then permit this
query to run
and return the expected row. In effect, to widen the definition of
"unconditional"
to handle cases such as my testcase.
. One comment is that all the infrastructure for returning one row from the
re-written query
is already present in the code, and the non-trivial question is how to
ensure the
new design is safe in preventing any rewrite that actually would not
return a row.
. In this patch, I have chosen to make use of the LIMIT clause -
I add a side-effect implication to a LIMIT clause when it occurs in a
rewrite of an INSERT
to mean "this rule will return a row".
So, with my patch, same testcase, same function
history_insert_partitioned and new rule
CREATE RULE insert_part_history as ON INSERT to history \
DO INSTEAD SELECT history_insert_partitioned(NEW) LIMIT 1
the INSERT is accepted and returns the id.
This use of LIMIT clause is probably contentious but I wished to avoid
introducing new
SQL syntax, and the LIMIT clause does have a connotation of returning
rows.
-----------------------------------
I attach patch based on clone of postgresql.git as of yesterday (120619-145751
EST)
I have tested the patch with INSERT and UPDATE (not tested with DELETE but
should work).
The patch is not expected to be final but just to show how I did it.
John
--- src/backend/optimizer/plan/planner.c.orig 2012-06-19 14:59:21.264574275 -0400
+++ src/backend/optimizer/plan/planner.c 2012-06-19 15:10:54.776590758 -0400
@@ -226,7 +226,8 @@ standard_planner(Query *parse, int curso
result->commandType = parse->commandType;
result->queryId = parse->queryId;
- result->hasReturning = (parse->returningList != NIL);
+ result->returning_one = parse->returning_one;
+ result->hasReturning = ( (parse->returningList != NIL) || parse->returning_one );
result->hasModifyingCTE = parse->hasModifyingCTE;
result->canSetTag = parse->canSetTag;
result->transientPlan = glob->transientPlan;
--- src/backend/rewrite/rewriteHandler.c.orig 2012-06-19 14:59:21.324574277 -0400
+++ src/backend/rewrite/rewriteHandler.c 2012-06-19 15:14:44.080596207 -0400
@@ -1734,6 +1734,8 @@ CopyAndAddInvertedQual(Query *parsetree,
* (must be initialized to FALSE)
* *qual_product - filled with modified original query if any qualified
* INSTEAD rule is found (must be initialized to NULL)
+ * *limit_rows_ptr - hint that this rule will return rows :
+ * any value in range [ 1 , 23 ] indicates yes
* Return value:
* list of rule actions adjusted for use with this query
*
@@ -1753,11 +1755,16 @@ fireRules(Query *parsetree,
List *locks,
bool *instead_flag,
bool *returning_flag,
- Query **qual_product)
+ Query **qual_product
+ ,int64 *limit_rows_ptr
+ )
{
List *results = NIL;
ListCell *l;
+ if (limit_rows_ptr)
+ *limit_rows_ptr = -1;
+
foreach(l, locks)
{
RewriteRule *rule_lock = (RewriteRule *) lfirst(l);
@@ -1810,6 +1817,22 @@ fireRules(Query *parsetree,
{
Query *rule_action = lfirst(r);
+ /* check for rule which is a SELECT with LIMIT clause -
+ ** if so, inform caller of the value (if const) or type of the limit
+ */
+ if ( (limit_rows_ptr)
+ && (rule_action->commandType == CMD_SELECT)
+ && (rule_action->limitCount)
+ )
+ {
+ if (nodeTag(rule_action->limitCount) == T_Const) {
+ *limit_rows_ptr = DatumGetInt64(((Const *)(rule_action->limitCount))->constvalue);
+ }
+ else if (nodeTag(rule_action->limitCount) == T_FuncExpr) {
+ *limit_rows_ptr = ((int64)((FuncExpr *)(rule_action->limitCount))->funcresulttype);
+ }
+ }
+
if (rule_action->commandType == CMD_NOTHING)
continue;
@@ -1850,6 +1873,7 @@ RewriteQuery(Query *parsetree, List *rew
* clauses. (We have to do this first because the WITH clauses may get
* copied into rule actions below.)
*/
+ parsetree->returning_one = false;
foreach(lc1, parsetree->cteList)
{
CommonTableExpr *cte = (CommonTableExpr *) lfirst(lc1);
@@ -1922,7 +1946,10 @@ RewriteQuery(Query *parsetree, List *rew
RangeTblEntry *rt_entry;
Relation rt_entry_relation;
List *locks;
+ int64 limit_rows; /* has either the const value or the datatype of a limit value */
+ /* which is a hint that this action will return specified number of rows */
+ limit_rows = -1;
result_relation = parsetree->resultRelation;
Assert(result_relation != 0);
rt_entry = rt_fetch(result_relation, parsetree->rtable);
@@ -2002,7 +2029,9 @@ RewriteQuery(Query *parsetree, List *rew
locks,
&instead,
&returning,
- &qual_product);
+ &qual_product
+ ,&limit_rows
+ );
/*
* If we got any product queries, recursively rewrite them --- but
@@ -2044,14 +2073,28 @@ RewriteQuery(Query *parsetree, List *rew
/*
* If there is an INSTEAD, and the original query has a RETURNING, we
- * have to have found a RETURNING in the rule(s), else fail. (Because
- * DefineQueryRewrite only allows RETURNING in unconditional INSTEAD
- * rules, there's no need to worry whether the substituted RETURNING
- * will actually be executed --- it must be.)
- */
- if ((instead || qual_product != NULL) &&
- parsetree->returningList &&
- !returning)
+ * have to have found in the rule(s) :
+ * EITHER a RETURNING
+ * OR a hint that this action will return specified number of rows
+ * else fail.
+ * (Because DefineQueryRewrite only allows RETURNING in unconditional
+ * INSTEAD rules, there's no need to worry whether the substituted
+ * RETURNING will actually be executed --- it must be.)
+ */
+ if ( (instead || qual_product != NULL)
+ && (parsetree->returningList)
+ && (!returning)
+ )
+ {
+ /* a value of limit_rows in the range 1 - 23 is a good hint
+ ** that query will return specified number of rows
+ */
+ if ( (limit_rows >= 1)
+ && (limit_rows <= 23)
+ )
+ {
+ parsetree->returning_one = true;
+ } else
{
switch (event)
{
@@ -2082,6 +2125,7 @@ RewriteQuery(Query *parsetree, List *rew
break;
}
}
+ }
heap_close(rt_entry_relation, NoLock);
}
@@ -2165,6 +2209,7 @@ QueryRewrite(Query *parsetree)
bool foundOriginalQuery;
Query *lastInstead;
+ parsetree->returning_one = false;
/*
* This function is only applied to top-level original queries
*/
@@ -2184,6 +2229,7 @@ QueryRewrite(Query *parsetree)
* Apply all the RIR rules on each query
*
* This is also a handy place to mark each query with the original queryId
+ * and with returning_one
*/
results = NIL;
foreach(l, querylist)
@@ -2193,6 +2239,7 @@ QueryRewrite(Query *parsetree)
query = fireRIRrules(query, NIL, false);
query->queryId = input_query_id;
+ query->returning_one = parsetree->returning_one;
results = lappend(results, query);
}
@@ -2233,9 +2280,13 @@ QueryRewrite(Query *parsetree)
else
{
Assert(!query->canSetTag);
- if (query->commandType == origCmdType &&
- (query->querySource == QSRC_INSTEAD_RULE ||
- query->querySource == QSRC_QUAL_INSTEAD_RULE))
+ if ( ( (query->commandType == origCmdType)
+ || query->returning_one
+ )
+ && ( (query->querySource == QSRC_INSTEAD_RULE)
+ || (query->querySource == QSRC_QUAL_INSTEAD_RULE)
+ )
+ )
lastInstead = query;
}
}
--- src/backend/nodes/copyfuncs.c.orig 2012-06-19 14:59:21.256574276 -0400
+++ src/backend/nodes/copyfuncs.c 2012-06-19 15:10:54.776590758 -0400
@@ -93,6 +93,7 @@ _copyPlannedStmt(const PlannedStmt *from
COPY_NODE_FIELD(relationOids);
COPY_NODE_FIELD(invalItems);
COPY_SCALAR_FIELD(nParamExec);
+ COPY_SCALAR_FIELD(returning_one);
return newnode;
}
@@ -2434,6 +2435,7 @@ _copyQuery(const Query *from)
COPY_NODE_FIELD(rowMarks);
COPY_NODE_FIELD(setOperations);
COPY_NODE_FIELD(constraintDeps);
+ COPY_SCALAR_FIELD(returning_one);
return newnode;
}
--- src/include/nodes/plannodes.h.orig 2012-06-19 14:59:21.496574281 -0400
+++ src/include/nodes/plannodes.h 2012-06-19 15:10:54.812590759 -0400
@@ -43,6 +43,8 @@ typedef struct PlannedStmt
bool hasModifyingCTE; /* has insert|update|delete in WITH? */
+ bool returning_one; /* inform portal that any intermediate result is to be returned -
+ when a RETURNING was rewritten */
bool canSetTag; /* do I set the command result tag? */
bool transientPlan; /* redo plan when TransactionXmin changes? */
--- src/include/nodes/parsenodes.h.orig 2012-06-19 14:59:21.492574281 -0400
+++ src/include/nodes/parsenodes.h 2012-06-19 15:10:54.816590759 -0400
@@ -105,6 +105,8 @@ typedef struct Query
uint32 queryId; /* query identifier (can be set by plugins) */
+ bool returning_one; /* inform portal that any intermediate result is to be returned -
+ when a RETURNING was rewritten */
bool canSetTag; /* do I set the command result tag? */
Node *utilityStmt; /* non-null if this is DECLARE CURSOR or a
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
