Marko Tiikkaja <marko.tiikk...@cs.helsinki.fi> writes:
>> Here's the patch. It's the same as the stuff in writeable CTE patches,
>> but I added regression tests.
> Whoops. The reference section in docs still had some traces of writeable
> CTEs. Updated patch attached.
I spent some time playing with this but concluded that it's not
committable. I ran into two significant problems:
1. In an INSERT statement, it's already possible to attach a WITH to
the contained statement, ie
INSERT INTO foo WITH ... SELECT ...
INSERT INTO foo WITH ... VALUES (...)
and the patch wasn't doing anything nice with the case where one tries
to put WITH at both places:
WITH ... INSERT INTO foo WITH ... VALUES (...)
(The SELECT case actually works, mostly, but the VALUES one doesn't.)
I thought about just concat'ing the two WITH lists but this introduces
various strange corner cases; in particular when one list is marked
RECURSIVE and the other isn't there's no way to avoid surprising
behavior. However, since the option for an inner WITH already does
everything you would want, we could just forget about adding outer WITH
for INSERT. The attached modified patch does that.
2. None of the cases play nicely with NEW or OLD references in a rule.
For example,
regression=# create temp table x(f1 int);
CREATE TABLE
regression=# create temp table y(f2 int);
CREATE TABLE
regression=# create rule r2 as on update to x do instead
with t as (select old.*) update y set f2 = t.f1 from t;
CREATE RULE
regression=# update x set f1 = f1+1;
ERROR: bogus local parameter passed to WITH query
regression=#
I don't see any very nice way to fix this. The problem is that the
NEW or OLD reference is treated as though it were a relation of the
main query (the UPDATE in this case), which is something that's not
valid to reference in a WITH query. I'm afraid that it might not
be possible to fix it without significant changes in the way rules
work (and consequent compatibility issues).
We could possibly put in some hack to disallow OLD/NEW references in
the WITH queries, but that got past my threshold of ugliness, so
I'm not going to commit it without further discussion.
Attached is the patch as I had it before giving up (sans documentation
since I'd not gotten to that yet). The main other change from what
you submitted was adding ruleutils.c support.
regards, tom lane
Index: src/backend/nodes/copyfuncs.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v
retrieving revision 1.461
diff -c -r1.461 copyfuncs.c
*** src/backend/nodes/copyfuncs.c 12 Feb 2010 17:33:20 -0000 1.461
--- src/backend/nodes/copyfuncs.c 13 Feb 2010 00:49:41 -0000
***************
*** 2279,2284 ****
--- 2279,2285 ----
COPY_NODE_FIELD(usingClause);
COPY_NODE_FIELD(whereClause);
COPY_NODE_FIELD(returningList);
+ COPY_NODE_FIELD(withClause);
return newnode;
}
***************
*** 2293,2298 ****
--- 2294,2300 ----
COPY_NODE_FIELD(whereClause);
COPY_NODE_FIELD(fromClause);
COPY_NODE_FIELD(returningList);
+ COPY_NODE_FIELD(withClause);
return newnode;
}
Index: src/backend/nodes/equalfuncs.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v
retrieving revision 1.382
diff -c -r1.382 equalfuncs.c
*** src/backend/nodes/equalfuncs.c 12 Feb 2010 17:33:20 -0000 1.382
--- src/backend/nodes/equalfuncs.c 13 Feb 2010 00:49:41 -0000
***************
*** 899,904 ****
--- 899,905 ----
COMPARE_NODE_FIELD(usingClause);
COMPARE_NODE_FIELD(whereClause);
COMPARE_NODE_FIELD(returningList);
+ COMPARE_NODE_FIELD(withClause);
return true;
}
***************
*** 911,916 ****
--- 912,918 ----
COMPARE_NODE_FIELD(whereClause);
COMPARE_NODE_FIELD(fromClause);
COMPARE_NODE_FIELD(returningList);
+ COMPARE_NODE_FIELD(withClause);
return true;
}
Index: src/backend/parser/analyze.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/analyze.c,v
retrieving revision 1.401
diff -c -r1.401 analyze.c
*** src/backend/parser/analyze.c 12 Feb 2010 22:48:56 -0000 1.401
--- src/backend/parser/analyze.c 13 Feb 2010 00:49:41 -0000
***************
*** 282,287 ****
--- 282,294 ----
qry->commandType = CMD_DELETE;
+ /* process the WITH clause independently of all else */
+ if (stmt->withClause)
+ {
+ qry->hasRecursive = stmt->withClause->recursive;
+ qry->cteList = transformWithClause(pstate, stmt->withClause);
+ }
+
/* set up range table with just the result rel */
qry->resultRelation = setTargetTable(pstate, stmt->relation,
interpretInhOption(stmt->relation->inhOpt),
***************
*** 380,386 ****
* Must get write lock on INSERT target table before scanning SELECT, else
* we will grab the wrong kind of initial lock if the target table is also
* mentioned in the SELECT part. Note that the target table is not added
! * to the joinlist or namespace.
*/
qry->resultRelation = setTargetTable(pstate, stmt->relation,
false, false, ACL_INSERT);
--- 387,394 ----
* Must get write lock on INSERT target table before scanning SELECT, else
* we will grab the wrong kind of initial lock if the target table is also
* mentioned in the SELECT part. Note that the target table is not added
! * to the joinlist or namespace (and hence it won't affect processing
! * of the contained statement).
*/
qry->resultRelation = setTargetTable(pstate, stmt->relation,
false, false, ACL_INSERT);
***************
*** 1730,1735 ****
--- 1738,1750 ----
qry->commandType = CMD_UPDATE;
pstate->p_is_update = true;
+ /* process the WITH clause independently of all else */
+ if (stmt->withClause)
+ {
+ qry->hasRecursive = stmt->withClause->recursive;
+ qry->cteList = transformWithClause(pstate, stmt->withClause);
+ }
+
qry->resultRelation = setTargetTable(pstate, stmt->relation,
interpretInhOption(stmt->relation->inhOpt),
true,
Index: src/backend/parser/gram.y
===================================================================
RCS file: /cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.708
diff -c -r2.708 gram.y
*** src/backend/parser/gram.y 12 Feb 2010 17:33:20 -0000 2.708
--- src/backend/parser/gram.y 13 Feb 2010 00:49:41 -0000
***************
*** 429,435 ****
%type <boolean> xml_whitespace_option
%type <node> common_table_expr
! %type <with> with_clause
%type <list> cte_list
%type <list> window_clause window_definition_list opt_partition_clause
--- 429,435 ----
%type <boolean> xml_whitespace_option
%type <node> common_table_expr
! %type <with> with_clause opt_with_clause
%type <list> cte_list
%type <list> window_clause window_definition_list opt_partition_clause
***************
*** 7141,7154 ****
*
*****************************************************************************/
! DeleteStmt: DELETE_P FROM relation_expr_opt_alias
using_clause where_or_current_clause returning_clause
{
DeleteStmt *n = makeNode(DeleteStmt);
! n->relation = $3;
! n->usingClause = $4;
! n->whereClause = $5;
! n->returningList = $6;
$$ = (Node *)n;
}
;
--- 7141,7155 ----
*
*****************************************************************************/
! DeleteStmt: opt_with_clause DELETE_P FROM relation_expr_opt_alias
using_clause where_or_current_clause returning_clause
{
DeleteStmt *n = makeNode(DeleteStmt);
! n->relation = $4;
! n->usingClause = $5;
! n->whereClause = $6;
! n->returningList = $7;
! n->withClause = $1;
$$ = (Node *)n;
}
;
***************
*** 7203,7220 ****
*
*****************************************************************************/
! UpdateStmt: UPDATE relation_expr_opt_alias
SET set_clause_list
from_clause
where_or_current_clause
returning_clause
{
UpdateStmt *n = makeNode(UpdateStmt);
! n->relation = $2;
! n->targetList = $4;
! n->fromClause = $5;
! n->whereClause = $6;
! n->returningList = $7;
$$ = (Node *)n;
}
;
--- 7204,7222 ----
*
*****************************************************************************/
! UpdateStmt: opt_with_clause UPDATE relation_expr_opt_alias
SET set_clause_list
from_clause
where_or_current_clause
returning_clause
{
UpdateStmt *n = makeNode(UpdateStmt);
! n->relation = $3;
! n->targetList = $5;
! n->fromClause = $6;
! n->whereClause = $7;
! n->returningList = $8;
! n->withClause = $1;
$$ = (Node *)n;
}
;
***************
*** 7556,7561 ****
--- 7558,7569 ----
}
;
+ opt_with_clause:
+ with_clause { $$ = $1; }
+ | /*EMPTY*/ { $$ = NULL; }
+ ;
+
+
into_clause:
INTO OptTempTableName
{
Index: src/backend/utils/adt/ruleutils.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/utils/adt/ruleutils.c,v
retrieving revision 1.321
diff -c -r1.321 ruleutils.c
*** src/backend/utils/adt/ruleutils.c 12 Feb 2010 17:33:20 -0000 1.321
--- src/backend/utils/adt/ruleutils.c 13 Feb 2010 00:49:42 -0000
***************
*** 3351,3356 ****
--- 3351,3359 ----
RangeTblEntry *rte;
ListCell *l;
+ /* Insert the WITH clause if given */
+ get_with_clause(query, context);
+
/*
* Start the query with UPDATE relname SET
*/
***************
*** 3432,3437 ****
--- 3435,3443 ----
StringInfo buf = context->buf;
RangeTblEntry *rte;
+ /* Insert the WITH clause if given */
+ get_with_clause(query, context);
+
/*
* Start the query with DELETE FROM relname
*/
Index: src/include/nodes/parsenodes.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/nodes/parsenodes.h,v
retrieving revision 1.429
diff -c -r1.429 parsenodes.h
*** src/include/nodes/parsenodes.h 12 Feb 2010 17:33:20 -0000 1.429
--- src/include/nodes/parsenodes.h 13 Feb 2010 00:49:42 -0000
***************
*** 906,911 ****
--- 906,912 ----
List *usingClause; /* optional using clause for more tables */
Node *whereClause; /* qualifications */
List *returningList; /* list of expressions to return */
+ WithClause *withClause; /* WITH clause */
} DeleteStmt;
/* ----------------------
***************
*** 920,925 ****
--- 921,927 ----
Node *whereClause; /* qualifications */
List *fromClause; /* optional from clause for more tables */
List *returningList; /* list of expressions to return */
+ WithClause *withClause; /* WITH clause */
} UpdateStmt;
/* ----------------------
Index: src/test/regress/expected/with.out
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/expected/with.out,v
retrieving revision 1.13
diff -c -r1.13 with.out
*** src/test/regress/expected/with.out 22 Nov 2009 05:20:41 -0000 1.13
--- src/test/regress/expected/with.out 13 Feb 2010 00:49:42 -0000
***************
*** 738,743 ****
--- 738,820 ----
(54 rows)
--
+ -- WITH on top of a DML statement
+ --
+ CREATE TEMPORARY TABLE y (a INTEGER);
+ INSERT INTO y SELECT generate_series(1, 10);
+ INSERT INTO y
+ WITH t AS (
+ SELECT a FROM y
+ )
+ SELECT a+20 FROM t RETURNING *;
+ a
+ ----
+ 21
+ 22
+ 23
+ 24
+ 25
+ 26
+ 27
+ 28
+ 29
+ 30
+ (10 rows)
+
+ WITH t AS (
+ SELECT a FROM y
+ )
+ UPDATE y SET a = y.a-10 FROM t WHERE y.a > 20 AND t.a = y.a RETURNING y.a;
+ a
+ ----
+ 11
+ 12
+ 13
+ 14
+ 15
+ 16
+ 17
+ 18
+ 19
+ 20
+ (10 rows)
+
+ WITH RECURSIVE t(a) AS (
+ SELECT 11
+ UNION ALL
+ SELECT a+1 FROM t WHERE a < 50
+ )
+ DELETE FROM y USING t WHERE t.a = y.a RETURNING y.a;
+ a
+ ----
+ 11
+ 12
+ 13
+ 14
+ 15
+ 16
+ 17
+ 18
+ 19
+ 20
+ (10 rows)
+
+ SELECT * FROM y;
+ a
+ ----
+ 1
+ 2
+ 3
+ 4
+ 5
+ 6
+ 7
+ 8
+ 9
+ 10
+ (10 rows)
+
+ --
-- error cases
--
-- INTERSECT
***************
*** 774,781 ****
ERROR: recursive reference to query "x" must not appear within its non-recursive term
LINE 1: WITH RECURSIVE x(n) AS (SELECT n FROM x UNION ALL SELECT 1)
^
- CREATE TEMPORARY TABLE y (a INTEGER);
- INSERT INTO y SELECT generate_series(1, 10);
-- LEFT JOIN
WITH RECURSIVE x(n) AS (SELECT a FROM y WHERE a = 1
UNION ALL
--- 851,856 ----
Index: src/test/regress/sql/with.sql
===================================================================
RCS file: /cvsroot/pgsql/src/test/regress/sql/with.sql,v
retrieving revision 1.11
diff -c -r1.11 with.sql
*** src/test/regress/sql/with.sql 9 Sep 2009 03:32:52 -0000 1.11
--- src/test/regress/sql/with.sql 13 Feb 2010 00:49:42 -0000
***************
*** 339,344 ****
--- 339,371 ----
SELECT * FROM z;
--
+ -- WITH on top of a DML statement
+ --
+
+ CREATE TEMPORARY TABLE y (a INTEGER);
+ INSERT INTO y SELECT generate_series(1, 10);
+
+ INSERT INTO y
+ WITH t AS (
+ SELECT a FROM y
+ )
+ SELECT a+20 FROM t RETURNING *;
+
+ WITH t AS (
+ SELECT a FROM y
+ )
+ UPDATE y SET a = y.a-10 FROM t WHERE y.a > 20 AND t.a = y.a RETURNING y.a;
+
+ WITH RECURSIVE t(a) AS (
+ SELECT 11
+ UNION ALL
+ SELECT a+1 FROM t WHERE a < 50
+ )
+ DELETE FROM y USING t WHERE t.a = y.a RETURNING y.a;
+
+ SELECT * FROM y;
+
+ --
-- error cases
--
***************
*** 364,372 ****
WITH RECURSIVE x(n) AS (SELECT n FROM x UNION ALL SELECT 1)
SELECT * FROM x;
- CREATE TEMPORARY TABLE y (a INTEGER);
- INSERT INTO y SELECT generate_series(1, 10);
-
-- LEFT JOIN
WITH RECURSIVE x(n) AS (SELECT a FROM y WHERE a = 1
--- 391,396 ----
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
