Re: [8.4] Updated WITH clause patch (non-recursive)

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 */