Thread: [8.4] Updated WITH clause patch (non-recursive)
Attached is an updated version of Greg Stark's patch to add support for the non-recursive variant of the SQL99 WITH clause[1]. I haven't looked at the actual functionality of the patch yet (which is quite trivial) -- I just fixed up bitrot and the like. I also removed support for RECURSIVE and the search/cycle clause, along with their associated keywords -- the current patch doesn't approach anything close to adding support for the non-recursive case, so it seems like a net loss to add additional keywords for no gain in functionality. Remaining work is to review the guts of the patch (which shouldn't take long), and write documentation and regression tests. I'm personally hoping to see this get into the tree fairly early in the 8.4 cycle, pending discussion of course. -Neil [1] http://archives.postgresql.org/pgsql-patches/2007-03/msg00139.php http://archives.postgresql.org/pgsql-patches/2007-04/msg00055.php
Attachment
"Neil Conway" <neilc@samurai.com> writes: > Remaining work is to review the guts of the patch (which shouldn't take > long), and write documentation and regression tests. I'm personally > hoping to see this get into the tree fairly early in the 8.4 cycle, > pending discussion of course. Note that as it stands it directly inlines the subquery into the query everywhere you use it. So if the user was hoping to save database work by avoiding duplicate subqueries he or she may be disappointed. On the other hand inlining it can allow the planner to produce better plans. Tom's feeling at the time was that even though it was providing something from the standard, it wasn't actually allowing the user to do anything he couldn't before. If it doesn't provide any additional expressive capabilities then I think he didn't like taking "with" as a reserved word. I still hope to do recursive queries for 8.4 so I don't have strong feelings for this part either way. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production Tuning
On Sun, 2008-01-27 at 09:17 +0000, Gregory Stark wrote: > Tom's feeling at the time was that even though it was providing something from > the standard, it wasn't actually allowing the user to do anything he couldn't > before. I think this feature has value: (1) This is SQL-standard syntax (and not even wacko syntax, at that!), and there is merit in implementing it on those grounds alone. (2) It is supported by DB2, MS SQL and Oracle, so there is a further compatibility argument to be made. (3) It avoids the need to repeat subqueries multiple times in the main query, which can make queries more concise. Defining subqueries outside the main SELECT body can also have readability advantages. > If it doesn't provide any additional expressive capabilities then I > think he didn't like taking "with" as a reserved word. Note that we can make WITH a type_func_name_keyword, rather than a full-on reserved_keyword, which reduces the force of this argument slightly. If we're going to implement recursive queries eventually (which we almost surely will, whether in 8.4 or a future release), we'll need to make WITH more reserved at some point anyway, so I don't see much to be gained in the long run by delaying it. -Neil
Hello On 27/01/2008, Neil Conway <neilc@samurai.com> wrote: > On Sun, 2008-01-27 at 09:17 +0000, Gregory Stark wrote: > > Tom's feeling at the time was that even though it was providing something from > > the standard, it wasn't actually allowing the user to do anything he couldn't > > before. > > I think this feature has value: > +1 I thing so is better commit smaller pieces more often than one time one big patch. Nine months long feature freeze time is enough. Regards Pavel Stehule > (1) This is SQL-standard syntax (and not even wacko syntax, at that!), > and there is merit in implementing it on those grounds alone. > > (2) It is supported by DB2, MS SQL and Oracle, so there is a further > compatibility argument to be made. > > (3) It avoids the need to repeat subqueries multiple times in the main > query, which can make queries more concise. Defining subqueries outside > the main SELECT body can also have readability advantages. > > > If it doesn't provide any additional expressive capabilities then I > > think he didn't like taking "with" as a reserved word. > > Note that we can make WITH a type_func_name_keyword, rather than a > full-on reserved_keyword, which reduces the force of this argument > slightly. > > If we're going to implement recursive queries eventually (which we > almost surely will, whether in 8.4 or a future release), we'll need to > make WITH more reserved at some point anyway, so I don't see much to be > gained in the long run by delaying it. > > -Neil > > > > ---------------------------(end of broadcast)--------------------------- > TIP 4: Have you searched our list archives? > > http://archives.postgresql.org >
Gregory Stark <stark@enterprisedb.com> writes:
> I still hope to do recursive queries for 8.4 so I don't have strong feelings
> for this part either way.
One question that hasn't been asked is whether this patch is likely to
help, or to get in the way, for a more full-fledged implementation.
I don't recall at the moment if Greg has a credible design sketch for
the remaining work, but it might be a good idea to review that before
deciding.
regards, tom lane
Neil Conway <neilc@samurai.com> writes:
> (1) This is SQL-standard syntax (and not even wacko syntax, at that!),
> and there is merit in implementing it on those grounds alone.
> (2) It is supported by DB2, MS SQL and Oracle, so there is a further
> compatibility argument to be made.
Both of the above arguments hold water only if we implement compatible
*semantics*, not merely syntax, so I find them unconvincing at this
stage.
> (3) It avoids the need to repeat subqueries multiple times in the main
> query, which can make queries more concise. Defining subqueries outside
> the main SELECT body can also have readability advantages.
Views fix that too.
>> If it doesn't provide any additional expressive capabilities then I
>> think he didn't like taking "with" as a reserved word.
> If we're going to implement recursive queries eventually (which we
> almost surely will, whether in 8.4 or a future release), we'll need to
> make WITH more reserved at some point anyway, so I don't see much to be
> gained in the long run by delaying it.
The point is that when you break people's apps you'll be able to point
to some real increment in functionality to justify it. With the patch
as it stands you'd essentially be saying "we're going to cause you pain
now for benefit later", which is a hard selling proposition.
I'm not opposed to applying this patch if it's an incremental step along
a clearly defined path to full WITH support in 8.4. I'm less eager to
put it in if there's not a plan and a commitment to make that happen.
regards, tom lane
On Sun, 2008-01-27 at 12:36 -0500, Tom Lane wrote:
> Both of the above arguments hold water only if we implement compatible
> *semantics*, not merely syntax, so I find them unconvincing at this
> stage.
How are the semantics of the proposed patch incompatible with the SQL
spec or the implementations in other systems? The proposed patch is a
*subset* of the functionality in the SQL spec, but it isn't incompatible
with it as far as I know (recursive and non-recursive WITH are distinct
features).
> > (3) It avoids the need to repeat subqueries multiple times in the main
> > query, which can make queries more concise. Defining subqueries outside
> > the main SELECT body can also have readability advantages.
>
> Views fix that too.
Sure, if you're willing to resort to DDL, and lose most of the
conciseness / readability gain.
> The point is that when you break people's apps you'll be able to point
> to some real increment in functionality to justify it.
If your application uses an identifier that is a reserved word in SQL-92
and in pretty much all major databases, I don't think you have much
cause for grievance when it becomes a reserved word in Postgres -- the
writing has been on the wall for a while. Do you have any reason to
think that "WITH" is a particularly common table or column name, by the
way?
Note also the keywords.c hack in 8.3 for the WITH keyword means that
pg_dump already treats WITH as a reserved word, so most dumps should
load without changes.
> With the patch as it stands you'd essentially be saying "we're going
> to cause you pain now for benefit later", which is a hard selling
> proposition.
Again, the readability + compatibility arguments are non-zero benefits,
IMHO. But your argument is essentially a public-relations one ("it will
look bad if..."), which I don't find very convincing. If we explain that
WITH is a reserved word per SQL spec and is part of the planned support
for recursive queries (whether in 8.4 or later), I can't see very many
users being annoyed.
(Compare that with the irritation we may well see from the removal of
implicit casts in 8.3, which will break *far* more applications, for a
benefit that many users will no doubt find rather hard to observe.)
-Neil
On Jan 27, 2008 8:13 PM, Neil Conway <neilc@samurai.com> wrote: > (Compare that with the irritation we may well see from the removal of > implicit casts in 8.3, which will break *far* more applications, for a > benefit that many users will no doubt find rather hard to observe.) It's a bit off-topic but I was thinking the same *before* porting a real application to 8.3. There are cases where it's just annoying to not have the casts anymore but I find also a bunch of broken behaviours (interval > int for example) which I'm quite happy to detect and fix. But I'm pretty sure we'll have a lot of feedback on this, probably mostly negative at first. -- Guillaume
Neil Conway wrote: > On Sun, 2008-01-27 at 12:36 -0500, Tom Lane wrote: >> Both of the above arguments hold water only if we implement >> compatible *semantics*, not merely syntax, so I find them >> unconvincing at this stage. > > How are the semantics of the proposed patch incompatible with the SQL > spec or the implementations in other systems? The proposed patch is > a *subset* of the functionality in the SQL spec, but it isn't > incompatible with it as far as I know (recursive and non-recursive > WITH are distinct features). An implementation of WITH that inlines the subquery instead of executing it only once (if appropriate) might not be incompatible with the SQL spec, but it might very well turn out to be incompatible with other major DBMSes from a practical point of view. If people use non-recursive WITH as a replacement for constructs like CREATE TEMPORARY TABLE temp AS SELECT ... ; SELECT ... FROM temp, ... ; , and not merely to increase readability, they won't gain anything from an inlining WITH implementation. This, BTW, is the reason that the C++ standard specifies the runtime complexity (in big-O-notation) for things like vector/list/hash lookups, instead of just specifying the interface. regards, Florian Pflug
"Neil Conway" <neilc@samurai.com> writes:
> Remaining work is to review the guts of the patch (which shouldn't take
> long), and write documentation and regression tests. I'm personally
> hoping to see this get into the tree fairly early in the 8.4 cycle,
> pending discussion of course.
Looking back at this I've realized (putting aside whether we want to apply the
patch as is which is another question) that to get the CTEs materialized so
they perform the way a user might expect them to would actually require the
same infrastructure that recursive queries will require.
Basically what I think we really want down the line is for something like:
WITH (select * from complex_view) AS x
SELECT *
FROM x
JOIN x as x2 ON (x.id=x2.id2)
to run the view once, materialize the results and then join the resulting data
with itself. At least that's what the user is likely expecting. Now it may be
that we have a better plan by inlining the two calls which in an ideal world
we would go ahead and try as well. But it's more likely that users would write
the WITH clause because they specifically want to avoid re-evaluating a
complex subquery.
To do this though we would need the same capability that recursive queries
would need. Namely the ability to have a single tuplestore with multiple
readers reading from different positions in the tuplestore.
So what I'm imagining doing is to add a flag to the RelOptInfo (Alternatively
we could create a new rtekind, RTE_CTE, but that would require most sites
which check for RTE_SUBQUERY to check for that as well).
Then (I think) in create_subqueryscan_plan we would have to check for this
flag and introduce the Memoize node I previously mentioned. That's basically a
Materialize node which keeps track of its position within the tuplestore in
its own state. It would also have to introuduce the one-time node with the
Materialize node which the Memoize would point to. I'm getting a bit vague
here as I haven't entirely absorbed how one-time plans work.
That would allow the query above to, for example, generate something like:
InitPlan
-> Memoize x (cost=0.00..34.00 rows=2400 width=4)
-> Seq scan on complex_view (cost=0.00..34.00 rows=2400 width=4)
Merge Join (cost=337.50..781.50 rows=28800 width=8)
Merge Cond: (x.id = x2.id)
-> Sort (cost=168.75..174.75 rows=2400 width=4)
Sort Key: x.id
-> MemoizeRead x (cost=0.00..34.00 rows=2400 width=4)
-> Sort (cost=168.75..174.75 rows=2400 width=4)
Sort Key: x2.id
-> MemoizeRead x x2 (cost=0.00..34.00 rows=2400 width=4)
Does this sound like the right track? Should I be doing this at the RelOptInfo
level or at some point higher up? Do I misunderstand anything about how
InitPlan is handled?
Other ideas: it might be interesting to note that we're sorting the same
Memoize node twice and push that down into the initial plan. Or somehow to
check whether it wouldn't be faster to just inline the memoized node directly
because perhaps there's a path available which would work for this read of it.
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's On-Demand Production Tuning
Hi,
From: Neil Conway <neilc@samurai.com>
Subject: [PATCHES] [8.4] Updated WITH clause patch (non-recursive)
Date: Sat, 26 Jan 2008 23:58:40 -0800
> Attached is an updated version of Greg Stark's patch to add support for
> the non-recursive variant of the SQL99 WITH clause[1].
I found a bug with the following SQL.
postgres=# WITH x AS (SELECT 1), y AS (SELECT 2)
SELECT * FROM x UNION ALL SELECT * FROM y;
ERROR: relation "x" does not exist
Attached patch transforms WITH clause in transformSetOperationStmt().
It works correctly with the attached patch.
postgres=# WITH x AS (SELECT 1), y AS (SELECT 2)
SELECT * FROM x UNION ALL SELECT * FROM y;
?column?
----------
1
2
(2 rows)
Regards,
--
Yoshiyuki Asaba
y-asaba@sraoss.co.jp
Index: src/backend/nodes/copyfuncs.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v
retrieving revision 1.390
diff -c -r1.390 copyfuncs.c
*** src/backend/nodes/copyfuncs.c 21 Mar 2008 22:41:48 -0000 1.390
--- src/backend/nodes/copyfuncs.c 25 Mar 2008 04:18:06 -0000
***************
*** 1939,1944 ****
--- 1939,1945 ----
COPY_NODE_FIELD(limitOffset);
COPY_NODE_FIELD(limitCount);
COPY_NODE_FIELD(lockingClause);
+ COPY_NODE_FIELD(with_cte_list);
COPY_SCALAR_FIELD(op);
COPY_SCALAR_FIELD(all);
COPY_NODE_FIELD(larg);
Index: src/backend/nodes/equalfuncs.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/nodes/equalfuncs.c,v
retrieving revision 1.320
diff -c -r1.320 equalfuncs.c
*** src/backend/nodes/equalfuncs.c 21 Mar 2008 22:41:48 -0000 1.320
--- src/backend/nodes/equalfuncs.c 25 Mar 2008 04:18:07 -0000
***************
*** 821,826 ****
--- 821,827 ----
COMPARE_NODE_FIELD(limitOffset);
COMPARE_NODE_FIELD(limitCount);
COMPARE_NODE_FIELD(lockingClause);
+ COMPARE_NODE_FIELD(with_cte_list);
COMPARE_SCALAR_FIELD(op);
COMPARE_SCALAR_FIELD(all);
COMPARE_NODE_FIELD(larg);
Index: src/backend/nodes/outfuncs.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/nodes/outfuncs.c,v
retrieving revision 1.324
diff -c -r1.324 outfuncs.c
*** src/backend/nodes/outfuncs.c 21 Mar 2008 22:41:48 -0000 1.324
--- src/backend/nodes/outfuncs.c 25 Mar 2008 04:18:08 -0000
***************
*** 1599,1604 ****
--- 1599,1605 ----
WRITE_NODE_FIELD(limitOffset);
WRITE_NODE_FIELD(limitCount);
WRITE_NODE_FIELD(lockingClause);
+ WRITE_NODE_FIELD(with_cte_list);
WRITE_ENUM_FIELD(op, SetOperation);
WRITE_BOOL_FIELD(all);
WRITE_NODE_FIELD(larg);
Index: src/backend/parser/analyze.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/analyze.c,v
retrieving revision 1.371
diff -c -r1.371 analyze.c
*** src/backend/parser/analyze.c 1 Jan 2008 19:45:50 -0000 1.371
--- src/backend/parser/analyze.c 25 Mar 2008 04:18:09 -0000
***************
*** 688,693 ****
--- 688,696 ----
/* make FOR UPDATE/FOR SHARE info available to addRangeTableEntry */
pstate->p_locking_clause = stmt->lockingClause;
+ /* process the WITH clause (pull CTEs into the pstate's ctenamespace) */
+ transformWithClause(pstate, stmt->with_cte_list);
+
/* process the FROM clause */
transformFromClause(pstate, stmt->fromClause);
***************
*** 1021,1026 ****
--- 1024,1032 ----
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("SELECT FOR UPDATE/SHARE is not allowed with UNION/INTERSECT/EXCEPT")));
+ /* process the WITH clause (pull CTEs into the pstate's ctenamespace) */
+ transformWithClause(pstate, stmt->with_cte_list);
+
/*
* Recursively transform the components of the tree.
*/
Index: src/backend/parser/gram.y
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.610
diff -c -r2.610 gram.y
*** src/backend/parser/gram.y 21 Mar 2008 22:41:48 -0000 2.610
--- src/backend/parser/gram.y 25 Mar 2008 04:18:16 -0000
***************
*** 103,109 ****
static SelectStmt *findLeftmostSelect(SelectStmt *node);
static void insertSelectOptions(SelectStmt *stmt,
List *sortClause, List *lockingClause,
! Node *limitOffset, Node *limitCount);
static Node *makeSetOp(SetOperation op, bool all, Node *larg, Node *rarg);
static Node *doNegate(Node *n, int location);
static void doNegateFloat(Value *v);
--- 103,110 ----
static SelectStmt *findLeftmostSelect(SelectStmt *node);
static void insertSelectOptions(SelectStmt *stmt,
List *sortClause, List *lockingClause,
! Node *limitOffset, Node *limitCount,
! List *with_cte_list);
static Node *makeSetOp(SetOperation op, bool all, Node *larg, Node *rarg);
static Node *doNegate(Node *n, int location);
static void doNegateFloat(Value *v);
***************
*** 358,363 ****
--- 359,367 ----
%type <ival> document_or_content
%type <boolean> xml_whitespace_option
+ %type <node> common_table_expression
+ %type <list> with_cte_list cte_list
+
/*
* If you make any token changes, update the keyword table in
***************
*** 6170,6190 ****
| select_clause sort_clause
{
insertSelectOptions((SelectStmt *) $1, $2, NIL,
! NULL, NULL);
$$ = $1;
}
| select_clause opt_sort_clause for_locking_clause opt_select_limit
{
insertSelectOptions((SelectStmt *) $1, $2, $3,
! list_nth($4, 0), list_nth($4, 1));
$$ = $1;
}
| select_clause opt_sort_clause select_limit opt_for_locking_clause
{
insertSelectOptions((SelectStmt *) $1, $2, $4,
! list_nth($3, 0), list_nth($3, 1));
$$ = $1;
}
;
select_clause:
--- 6174,6225 ----
| select_clause sort_clause
{
insertSelectOptions((SelectStmt *) $1, $2, NIL,
! NULL, NULL, NIL);
$$ = $1;
}
| select_clause opt_sort_clause for_locking_clause opt_select_limit
{
insertSelectOptions((SelectStmt *) $1, $2, $3,
! list_nth($4, 0), list_nth($4, 1),
! NIL);
$$ = $1;
}
| select_clause opt_sort_clause select_limit opt_for_locking_clause
{
insertSelectOptions((SelectStmt *) $1, $2, $4,
! list_nth($3, 0), list_nth($3, 1),
! NIL);
$$ = $1;
}
+ | with_cte_list simple_select
+ {
+ insertSelectOptions((SelectStmt *) $2,
+ NULL, NIL,
+ NULL, NULL,
+ $1);
+ $$ = $2;
+ }
+ | with_cte_list select_clause sort_clause
+ {
+ insertSelectOptions((SelectStmt *) $2, $3, NIL,
+ NULL, NULL,
+ $1);
+ $$ = $2;
+ }
+ | with_cte_list select_clause opt_sort_clause for_locking_clause opt_select_limit
+ {
+ insertSelectOptions((SelectStmt *) $2, $3, $4,
+ list_nth($5, 0), list_nth($5, 1),
+ $1);
+ $$ = $2;
+ }
+ | with_cte_list select_clause opt_sort_clause select_limit opt_for_locking_clause
+ {
+ insertSelectOptions((SelectStmt *) $2, $3, $5,
+ list_nth($4, 0), list_nth($4, 1),
+ $1);
+ $$ = $2;
+ }
;
select_clause:
***************
*** 6245,6250 ****
--- 6280,6318 ----
}
;
+ /*
+ * ANSI standard WITH clause looks like:
+ *
+ * WITH [ RECURSIVE ] <query name> [ (<column>,...) ]
+ * AS (query) [ SEARCH or CYCLE clause ]
+ *
+ * We don't currently support RECURSIVE, or the SEARCH or CYCLE clause.
+ */
+ with_cte_list:
+ WITH cte_list
+ {
+ $$ = $2;
+ }
+ ;
+
+ cte_list:
+ common_table_expression { $$ = list_make1($1); }
+ | cte_list ',' common_table_expression { $$ = lappend($1, $3); }
+ ;
+
+ common_table_expression: name opt_name_list AS select_with_parens
+ {
+ RangeSubselect *n = makeNode(RangeSubselect);
+
+ n->subquery = $4;
+ n->alias = makeNode(Alias);
+ n->alias->aliasname = $1;
+ n->alias->colnames = $2;
+
+ $$ = (Node *) n;
+ }
+ ;
+
into_clause:
INTO OptTempTableName
{
***************
*** 9239,9245 ****
| VIEW
| VOLATILE
| WHITESPACE_P
- | WITH
| WITHOUT
| WORK
| WRITE
--- 9307,9312 ----
***************
*** 9421,9426 ****
--- 9488,9494 ----
| USING
| WHEN
| WHERE
+ | WITH
;
***************
*** 9680,9687 ****
static void
insertSelectOptions(SelectStmt *stmt,
List *sortClause, List *lockingClause,
! Node *limitOffset, Node *limitCount)
{
/*
* Tests here are to reject constructs like
* (SELECT foo ORDER BY bar) ORDER BY baz
--- 9748,9758 ----
static void
insertSelectOptions(SelectStmt *stmt,
List *sortClause, List *lockingClause,
! Node *limitOffset, Node *limitCount,
! List *with_cte_list)
{
+ Assert(IsA(stmt, SelectStmt));
+
/*
* Tests here are to reject constructs like
* (SELECT foo ORDER BY bar) ORDER BY baz
***************
*** 9712,9717 ****
--- 9783,9796 ----
errmsg("multiple LIMIT clauses not allowed")));
stmt->limitCount = limitCount;
}
+ if (with_cte_list)
+ {
+ if (stmt->with_cte_list)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("multiple WITH clauses not allowed")));
+ stmt->with_cte_list = with_cte_list;
+ }
}
static Node *
Index: src/backend/parser/parse_clause.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/parse_clause.c,v
retrieving revision 1.169
diff -c -r1.169 parse_clause.c
*** src/backend/parser/parse_clause.c 15 Feb 2008 17:19:46 -0000 1.169
--- src/backend/parser/parse_clause.c 25 Mar 2008 04:18:16 -0000
***************
*** 68,73 ****
--- 68,112 ----
/*
+ * transformWithClause -
+ * Transform the list of WITH clause "common table expressions" into
+ * Query nodes.
+ *
+ * We need to add the name of the common table expression to a list that is
+ * used later to find them. But we do _not_ add the table itself to the current
+ * namespace because that would implicitly join all of them which isn't right.
+ */
+ void
+ transformWithClause(ParseState *pstate, List *with_cte_list)
+ {
+ ListCell *lc;
+
+ foreach(lc, with_cte_list)
+ {
+ RangeSubselect *cte = lfirst(lc);
+ RangeSubselect *new_cte;
+ Query *query;
+
+ query = parse_sub_analyze(cte->subquery, pstate);
+
+ /* Same checks that FROM does on subqueries XXX refactor? */
+ if (query->commandType != CMD_SELECT ||
+ query->utilityStmt != NULL)
+ elog(ERROR, "expected SELECT query from subquery in WITH");
+ if (query->intoClause)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("subquery in WITH cannot have SELECT INTO")));
+
+ new_cte = makeNode(RangeSubselect);
+ new_cte->subquery = (Node*) query;
+ new_cte->alias = copyObject(cte->alias);
+
+ pstate->p_ctenamespace = lappend(pstate->p_ctenamespace, new_cte);
+ }
+ }
+
+ /*
* transformFromClause -
* Process the FROM clause and add items to the query's range table,
* joinlist, and namespaces.
***************
*** 410,415 ****
--- 449,503 ----
return rte;
}
+ /*
+ * transformRangeCTE --- transform a RangeVar which references a common table
+ * expression (ie, a sub-SELECT defined in a WITH clause)
+ */
+ static RangeTblEntry *
+ transformRangeCTE(ParseState *pstate, RangeVar *n, RangeSubselect *r)
+ {
+ RangeTblEntry *rte;
+
+ /*
+ * Unlike transformRangeSubselect we do not have to worry about:
+ *
+ * . checking for an alias because the grammar for WITH always gives us an
+ * alias
+ *
+ * . transforming the subquery as transformWithClause has already done that
+ * and the RangeSubselect contains the query tree, not the raw parse tree
+ *
+ * . checking for lateral references since WITH subqueries have their own
+ * scope. Since they were transformed prior to any range table entries
+ * being created in our pstate they were all planned with a fresh copy of
+ * our empty pstate (unless we're in a subquery already of course).
+ */
+
+ /*
+ * This is a kluge for now. Effectively we're inlining all the WITH
+ * clauses which isn't what we want to do
+ */
+
+ /*
+ * One tricky bit. We potentially have two aliases here. The WITH clause
+ * always specifies a relation alias and may or may not specify column
+ * aliases. The rangevar also may or may not specify a relation alias
+ * and may or may not specify column aliases.
+ */
+
+ Alias *a = copyObject(r->alias);
+ if (n->alias && n->alias->aliasname)
+ a->aliasname = n->alias->aliasname;
+ if (n->alias && n->alias->colnames)
+ a->colnames = n->alias->colnames;
+
+ /*
+ * OK, build an RTE for the subquery.
+ */
+ rte = addRangeTableEntryForSubquery(pstate, (Query*) r->subquery, a, true);
+
+ return rte;
+ }
/*
* transformRangeSubselect --- transform a sub-SELECT appearing in FROM
***************
*** 590,600 ****
if (IsA(n, RangeVar))
{
/* Plain relation reference */
RangeTblRef *rtr;
! RangeTblEntry *rte;
int rtindex;
! rte = transformTableEntry(pstate, (RangeVar *) n);
/* assume new rte is at end */
rtindex = list_length(pstate->p_rtable);
Assert(rte == rt_fetch(rtindex, pstate->p_rtable));
--- 678,715 ----
if (IsA(n, RangeVar))
{
/* Plain relation reference */
+ RangeVar *rv = (RangeVar *) n;
RangeTblRef *rtr;
! RangeTblEntry *rte = NULL;
int rtindex;
! if (!rv->schemaname)
! {
! /*
! * We have to check if this is a reference to a common table
! * expression (ie subquery defined in the WITH clause). Either
! * in this query or any parent query.
! */
! ParseState *ps;
! ListCell *lc;
!
! for (ps = pstate; ps; ps = ps->parentParseState)
! {
! foreach(lc, ps->p_ctenamespace)
! {
! RangeSubselect *r = (RangeSubselect *) lfirst(lc);
! if (strcmp(rv->relname, r->alias->aliasname) == 0)
! {
! rte = transformRangeCTE(pstate, rv, r);
! break;
! }
! }
! }
! }
!
! if (!rte)
! rte = transformTableEntry(pstate, rv);
!
/* assume new rte is at end */
rtindex = list_length(pstate->p_rtable);
Assert(rte == rt_fetch(rtindex, pstate->p_rtable));
Index: src/include/nodes/parsenodes.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/nodes/parsenodes.h,v
retrieving revision 1.361
diff -c -r1.361 parsenodes.h
*** src/include/nodes/parsenodes.h 21 Mar 2008 22:41:48 -0000 1.361
--- src/include/nodes/parsenodes.h 25 Mar 2008 04:18:19 -0000
***************
*** 771,776 ****
--- 771,777 ----
/*
* These fields are used only in upper-level SelectStmts.
*/
+ List *with_cte_list; /* List of Common Table Expressions (ie WITH clause) */
SetOperation op; /* type of set op */
bool all; /* ALL specified? */
struct SelectStmt *larg; /* left child */
Index: src/include/parser/parse_clause.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/parser/parse_clause.h,v
retrieving revision 1.49
diff -c -r1.49 parse_clause.h
*** src/include/parser/parse_clause.h 1 Jan 2008 19:45:58 -0000 1.49
--- src/include/parser/parse_clause.h 25 Mar 2008 04:18:19 -0000
***************
*** 16,21 ****
--- 16,22 ----
#include "parser/parse_node.h"
+ extern void transformWithClause(ParseState *pstate, List *with_cte_list);
extern void transformFromClause(ParseState *pstate, List *frmList);
extern int setTargetTable(ParseState *pstate, RangeVar *relation,
bool inh, bool alsoSource, AclMode requiredPerms);
Index: src/include/parser/parse_node.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/parser/parse_node.h,v
retrieving revision 1.53
diff -c -r1.53 parse_node.h
*** src/include/parser/parse_node.h 1 Jan 2008 19:45:58 -0000 1.53
--- src/include/parser/parse_node.h 25 Mar 2008 04:18:19 -0000
***************
*** 58,63 ****
--- 58,71 ----
* of ParseStates, only the topmost ParseState contains paramtype info; but
* we copy the p_variableparams flag down to the child nodes for speed in
* coerce_type.
+ *
+ * [1] Note that p_ctenamespace is a namespace for "relations" but distinct
+ * from p_relnamespace. p_ctenamespace is a list of relations that can be
+ * referred to in a FROM or JOIN clause (in addition to normal tables and
+ * views). p_relnamespace is the list of relations which already have been
+ * listed in such clauses and therefore can be referred to in qualified
+ * variable references. Also, note that p_ctenamespace is a list of
+ * RangeSubselects, not a list of range table entries.
*/
typedef struct ParseState
{
***************
*** 68,73 ****
--- 76,82 ----
* node's fromlist) */
List *p_relnamespace; /* current namespace for relations */
List *p_varnamespace; /* current namespace for columns */
+ List *p_ctenamespace; /* current namespace for common table expressions [1] */
Oid *p_paramtypes; /* OIDs of types for $n parameter symbols */
int p_numparams; /* allocated size of p_paramtypes[] */
int p_next_resno; /* next targetlist resno to assign */
There is little support for adding this patch without the recursive part, so I added the URLs for this thread to the TODO list under recursive queries. --------------------------------------------------------------------------- Neil Conway wrote: > Attached is an updated version of Greg Stark's patch to add support for > the non-recursive variant of the SQL99 WITH clause[1]. I haven't looked > at the actual functionality of the patch yet (which is quite trivial) -- > I just fixed up bitrot and the like. I also removed support for > RECURSIVE and the search/cycle clause, along with their associated > keywords -- the current patch doesn't approach anything close to adding > support for the non-recursive case, so it seems like a net loss to add > additional keywords for no gain in functionality. > > Remaining work is to review the guts of the patch (which shouldn't take > long), and write documentation and regression tests. I'm personally > hoping to see this get into the tree fairly early in the 8.4 cycle, > pending discussion of course. > > -Neil > > [1] http://archives.postgresql.org/pgsql-patches/2007-03/msg00139.php > http://archives.postgresql.org/pgsql-patches/2007-04/msg00055.php [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 5: don't forget to increase your free space map settings -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. +