Do honest transformation and preprocessing of LIMIT/OFFSET clauses,
authorTom Lane
Thu, 3 Jul 2003 19:07:54 +0000 (19:07 +0000)
committerTom Lane
Thu, 3 Jul 2003 19:07:54 +0000 (19:07 +0000)
instead of the former kluge whereby gram.y emitted already-transformed
expressions.  This is needed so that Params appearing in these clauses
actually work correctly.  I suppose some might claim that the side effect
of 'SELECT ... LIMIT 2+2' working is a new feature, but I say this is
a bug fix.

src/backend/optimizer/plan/planner.c
src/backend/optimizer/util/clauses.c
src/backend/parser/analyze.c
src/backend/parser/gram.y
src/backend/parser/parse_clause.c
src/backend/parser/parse_coerce.c
src/include/parser/parse_clause.h
src/include/parser/parse_coerce.h

index f55d0cad5d18c697501897ae9d18ee928a23960a..d058a44bb0d2d4570bb22f70adbc8c5a69c60439 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.155 2003/06/16 02:03:37 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v 1.156 2003/07/03 19:07:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -47,7 +47,8 @@
 #define EXPRKIND_QUAL  0
 #define EXPRKIND_TARGET    1
 #define EXPRKIND_RTFUNC    2
-#define EXPRKIND_ININFO    3
+#define EXPRKIND_LIMIT 3
+#define EXPRKIND_ININFO    4
 
 
 static Node *preprocess_expression(Query *parse, Node *expr, int kind);
@@ -232,6 +233,11 @@ subquery_planner(Query *parse, double tuple_fraction)
    parse->havingQual = preprocess_expression(parse, parse->havingQual,
                                              EXPRKIND_QUAL);
 
+   parse->limitOffset = preprocess_expression(parse, parse->limitOffset,
+                                              EXPRKIND_LIMIT);
+   parse->limitCount = preprocess_expression(parse, parse->limitCount,
+                                             EXPRKIND_LIMIT);
+
    parse->in_info_list = (List *)
        preprocess_expression(parse, (Node *) parse->in_info_list,
                              EXPRKIND_ININFO);
index b3dd818e0ac3db0f5d69f60552a350881a38dd34..d35cbee92e7c263c509cfcf9a600a35cd330237c 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.145 2003/07/03 16:33:07 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/optimizer/util/clauses.c,v 1.146 2003/07/03 19:07:25 tgl Exp $
  *
  * HISTORY
  *   AUTHOR            DATE            MAJOR EVENT
@@ -2390,6 +2390,10 @@ query_tree_walker(Query *query,
        return true;
    if (walker(query->havingQual, context))
        return true;
+   if (walker(query->limitOffset, context))
+       return true;
+   if (walker(query->limitCount, context))
+       return true;
    if (walker(query->in_info_list, context))
        return true;
    foreach(rt, query->rtable)
@@ -2863,6 +2867,8 @@ query_tree_mutator(Query *query,
    MUTATE(query->jointree, query->jointree, FromExpr *);
    MUTATE(query->setOperations, query->setOperations, Node *);
    MUTATE(query->havingQual, query->havingQual, Node *);
+   MUTATE(query->limitOffset, query->limitOffset, Node *);
+   MUTATE(query->limitCount, query->limitCount, Node *);
    MUTATE(query->in_info_list, query->in_info_list, List *);
    FastListInit(&newrt);
    foreach(rt, query->rtable)
index 99efae80721125ec78d5d97c172c64f8689df608..90818b153b8376a0d6be32d8a4bf7a03b0b5610a 100644 (file)
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.277 2003/06/25 04:19:24 momjian Exp $
+ * $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.278 2003/07/03 19:07:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -459,7 +459,7 @@ transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt)
    qry->distinctClause = NIL;
 
    /* fix where clause */
-   qual = transformWhereClause(pstate, stmt->whereClause);
+   qual = transformWhereClause(pstate, stmt->whereClause, "WHERE");
 
    /* done building the range table and jointree */
    qry->rtable = pstate->p_rtable;
@@ -1588,7 +1588,8 @@ transformIndexStmt(ParseState *pstate, IndexStmt *stmt)
        /* no to join list, yes to namespace */
        addRTEtoQuery(pstate, rte, false, true);
 
-       stmt->whereClause = transformWhereClause(pstate, stmt->whereClause);
+       stmt->whereClause = transformWhereClause(pstate, stmt->whereClause,
+                                                "WHERE");
    }
 
    /* take care of any index expressions */
@@ -1699,7 +1700,8 @@ transformRuleStmt(ParseState *pstate, RuleStmt *stmt,
    }
 
    /* take care of the where clause */
-   stmt->whereClause = transformWhereClause(pstate, stmt->whereClause);
+   stmt->whereClause = transformWhereClause(pstate, stmt->whereClause,
+                                            "WHERE");
 
    if (length(pstate->p_rtable) != 2)  /* naughty, naughty... */
        elog(ERROR, "Rule WHERE condition may not contain references to other relations");
@@ -1891,13 +1893,14 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt)
    markTargetListOrigins(pstate, qry->targetList);
 
    /* transform WHERE */
-   qual = transformWhereClause(pstate, stmt->whereClause);
+   qual = transformWhereClause(pstate, stmt->whereClause, "WHERE");
 
    /*
     * Initial processing of HAVING clause is just like WHERE clause.
     * Additional work will be done in optimizer/plan/planner.c.
     */
-   qry->havingQual = transformWhereClause(pstate, stmt->havingClause);
+   qry->havingQual = transformWhereClause(pstate, stmt->havingClause,
+                                          "HAVING");
 
    /*
     * Transform sorting/grouping stuff.  Do ORDER BY first because both
@@ -1918,8 +1921,10 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt)
                                                  qry->targetList,
                                                  &qry->sortClause);
 
-   qry->limitOffset = stmt->limitOffset;
-   qry->limitCount = stmt->limitCount;
+   qry->limitOffset = transformLimitClause(pstate, stmt->limitOffset,
+                                           "OFFSET");
+   qry->limitCount = transformLimitClause(pstate, stmt->limitCount,
+                                          "LIMIT");
 
    qry->rtable = pstate->p_rtable;
    qry->jointree = makeFromExpr(pstate->p_joinlist, qual);
@@ -2124,8 +2129,10 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
    if (tllen != length(qry->targetList))
        elog(ERROR, "ORDER BY on a UNION/INTERSECT/EXCEPT result must be on one of the result columns");
 
-   qry->limitOffset = limitOffset;
-   qry->limitCount = limitCount;
+   qry->limitOffset = transformLimitClause(pstate, limitOffset,
+                                           "OFFSET");
+   qry->limitCount = transformLimitClause(pstate, limitCount,
+                                          "LIMIT");
 
    qry->rtable = pstate->p_rtable;
    qry->jointree = makeFromExpr(pstate->p_joinlist, NULL);
@@ -2376,7 +2383,7 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt)
 
    qry->targetList = transformTargetList(pstate, stmt->targetList);
 
-   qual = transformWhereClause(pstate, stmt->whereClause);
+   qual = transformWhereClause(pstate, stmt->whereClause, "WHERE");
 
    qry->rtable = pstate->p_rtable;
    qry->jointree = makeFromExpr(pstate->p_joinlist, qual);
index d8485a2d4d7e229594a1db5f9a813cb56b3b79a4..0ef919c38ae5405b80209128dc4195dfd545d953 100644 (file)
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/parser/gram.y,v 2.425 2003/07/03 16:33:37 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/parser/gram.y,v 2.426 2003/07/03 19:07:36 tgl Exp $
  *
  * HISTORY
  *   AUTHOR            DATE            MAJOR EVENT
@@ -4585,12 +4585,13 @@ select_limit:
            | OFFSET select_offset_value
                { $$ = makeList2($2, NULL); }
            | LIMIT select_limit_value ',' select_offset_value
-               /* Disabled because it was too confusing, bjm 2002-02-18 */
-               { elog(ERROR,
-                   "LIMIT #,# syntax not supported.\n\tUse separate LIMIT and OFFSET clauses."); }
+               {
+                   /* Disabled because it was too confusing, bjm 2002-02-18 */
+                   elog(ERROR,
+                        "LIMIT #,# syntax not supported.\n\tUse separate LIMIT and OFFSET clauses.");
+               }
        ;
 
-
 opt_select_limit:
            select_limit                            { $$ = $1; }
            | /* EMPTY */
@@ -4598,67 +4599,18 @@ opt_select_limit:
        ;
 
 select_limit_value:
-           Iconst
-               {
-                   Const   *n = makeNode(Const);
-
-                   if ($1 < 0)
-                       elog(ERROR, "LIMIT must not be negative");
-
-                   n->consttype    = INT4OID;
-                   n->constlen     = sizeof(int4);
-                   n->constvalue   = Int32GetDatum($1);
-                   n->constisnull  = FALSE;
-                   n->constbyval   = TRUE;
-                   $$ = (Node *)n;
-               }
+           a_expr                                  { $$ = $1; }
            | ALL
                {
                    /* LIMIT ALL is represented as a NULL constant */
-                   Const   *n = makeNode(Const);
-
-                   n->consttype    = INT4OID;
-                   n->constlen     = sizeof(int4);
-                   n->constvalue   = (Datum) 0;
-                   n->constisnull  = TRUE;
-                   n->constbyval   = TRUE;
-                   $$ = (Node *)n;
-               }
-           | PARAM
-               {
-                   Param   *n = makeNode(Param);
-
-                   n->paramkind    = PARAM_NUM;
-                   n->paramid      = $1;
-                   n->paramtype    = INT4OID;
+                   A_Const *n = makeNode(A_Const);
+                   n->val.type = T_Null;
                    $$ = (Node *)n;
                }
        ;
 
 select_offset_value:
-           Iconst
-               {
-                   Const   *n = makeNode(Const);
-
-                   if ($1 < 0)
-                       elog(ERROR, "OFFSET must not be negative");
-
-                   n->consttype    = INT4OID;
-                   n->constlen     = sizeof(int4);
-                   n->constvalue   = Int32GetDatum($1);
-                   n->constisnull  = FALSE;
-                   n->constbyval   = TRUE;
-                   $$ = (Node *)n;
-               }
-           | PARAM
-               {
-                   Param   *n = makeNode(Param);
-
-                   n->paramkind    = PARAM_NUM;
-                   n->paramid      = $1;
-                   n->paramtype    = INT4OID;
-                   $$ = (Node *)n;
-               }
+           a_expr                                  { $$ = $1; }
        ;
 
 /*
index 4d08704da66ce4a90c41969f0acab55390bf771c..9094a14ae53b0f07fe9c75311aaaf5b696a39d63 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.116 2003/06/16 02:03:37 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/parser/parse_clause.c,v 1.117 2003/07/03 19:07:45 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -317,10 +317,7 @@ transformJoinOnClause(ParseState *pstate, JoinExpr *j,
    save_namespace = pstate->p_namespace;
    pstate->p_namespace = makeList2(j->larg, j->rarg);
 
-   /* This part is just like transformWhereClause() */
-   result = transformExpr(pstate, j->quals);
-
-   result = coerce_to_boolean(pstate, result, "JOIN/ON");
+   result = transformWhereClause(pstate, j->quals, "JOIN/ON");
 
    pstate->p_namespace = save_namespace;
 
@@ -945,10 +942,38 @@ buildMergedJoinVar(ParseState *pstate, JoinType jointype,
 
 /*
  * transformWhereClause -
- *   transforms the qualification and make sure it is of type Boolean
+ *   Transform the qualification and make sure it is of type boolean.
+ *   Used for WHERE and allied clauses.
+ *
+ * constructName does not affect the semantics, but is used in error messages
+ */
+Node *
+transformWhereClause(ParseState *pstate, Node *clause,
+                    const char *constructName)
+{
+   Node       *qual;
+
+   if (clause == NULL)
+       return NULL;
+
+   qual = transformExpr(pstate, clause);
+
+   qual = coerce_to_boolean(pstate, qual, constructName);
+
+   return qual;
+}
+
+
+/*
+ * transformLimitClause -
+ *   Transform the expression and make sure it is of type integer.
+ *   Used for LIMIT and allied clauses.
+ *
+ * constructName does not affect the semantics, but is used in error messages
  */
 Node *
-transformWhereClause(ParseState *pstate, Node *clause)
+transformLimitClause(ParseState *pstate, Node *clause,
+                    const char *constructName)
 {
    Node       *qual;
 
@@ -957,7 +982,31 @@ transformWhereClause(ParseState *pstate, Node *clause)
 
    qual = transformExpr(pstate, clause);
 
-   qual = coerce_to_boolean(pstate, qual, "WHERE");
+   qual = coerce_to_integer(pstate, qual, constructName);
+
+   /*
+    * LIMIT can't refer to any vars or aggregates of the current query;
+    * we don't allow subselects either (though that case would at least
+    * be sensible)
+    */
+   if (contain_vars_of_level(qual, 0))
+   {
+       /* translator: %s is name of a SQL construct, eg LIMIT */
+       elog(ERROR, "argument of %s must not contain variables",
+            constructName);
+   }
+   if (checkExprHasAggs(qual))
+   {
+       /* translator: %s is name of a SQL construct, eg LIMIT */
+       elog(ERROR, "argument of %s must not contain aggregates",
+            constructName);
+   }
+   if (contain_subplans(qual))
+   {
+       /* translator: %s is name of a SQL construct, eg LIMIT */
+       elog(ERROR, "argument of %s must not contain subselects",
+            constructName);
+   }
 
    return qual;
 }
index 977780fecc857adc8ca6a9a40bf1a2637b10fe11..1544f94cf2e602b996eab23dbe55390ddb3b5b1f 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/parser/parse_coerce.c,v 2.102 2003/07/01 19:10:53 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/parser/parse_coerce.c,v 2.103 2003/07/03 19:07:48 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -564,7 +564,7 @@ coerce_to_boolean(ParseState *pstate, Node *node,
        if (node == NULL)
        {
            /* translator: first %s is name of a SQL construct, eg WHERE */
-           elog(ERROR, "Argument of %s must be type boolean, not type %s",
+           elog(ERROR, "argument of %s must be type boolean, not type %s",
                 constructName, format_type_be(inputTypeId));
        }
    }
@@ -572,7 +572,46 @@ coerce_to_boolean(ParseState *pstate, Node *node,
    if (expression_returns_set(node))
    {
        /* translator: %s is name of a SQL construct, eg WHERE */
-       elog(ERROR, "Argument of %s must not be a set function",
+       elog(ERROR, "argument of %s must not be a set function",
+            constructName);
+   }
+
+   return node;
+}
+
+/* coerce_to_integer()
+ *     Coerce an argument of a construct that requires integer input
+ *     (LIMIT, OFFSET, etc).  Also check that input is not a set.
+ *
+ * Returns the possibly-transformed node tree.
+ *
+ * As with coerce_type, pstate may be NULL if no special unknown-Param
+ * processing is wanted.
+ */
+Node *
+coerce_to_integer(ParseState *pstate, Node *node,
+                 const char *constructName)
+{
+   Oid         inputTypeId = exprType(node);
+
+   if (inputTypeId != INT4OID)
+   {
+       node = coerce_to_target_type(pstate, node, inputTypeId,
+                                    INT4OID, -1,
+                                    COERCION_ASSIGNMENT,
+                                    COERCE_IMPLICIT_CAST);
+       if (node == NULL)
+       {
+           /* translator: first %s is name of a SQL construct, eg LIMIT */
+           elog(ERROR, "argument of %s must be type integer, not type %s",
+                constructName, format_type_be(inputTypeId));
+       }
+   }
+
+   if (expression_returns_set(node))
+   {
+       /* translator: %s is name of a SQL construct, eg LIMIT */
+       elog(ERROR, "argument of %s must not be a set function",
             constructName);
    }
 
index c2a3b2dc77b8068860ad0091b4884a888a173cc1..6f4f87a8b6675eefc15ab094a57e006fa8d39ac6 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: parse_clause.h,v 1.32 2003/06/16 02:03:38 tgl Exp $
+ * $Id: parse_clause.h,v 1.33 2003/07/03 19:07:53 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -21,7 +21,10 @@ extern int setTargetTable(ParseState *pstate, RangeVar *relation,
               bool inh, bool alsoSource);
 extern bool interpretInhOption(InhOption inhOpt);
 
-extern Node *transformWhereClause(ParseState *pstate, Node *where);
+extern Node *transformWhereClause(ParseState *pstate, Node *clause,
+                                 const char *constructName);
+extern Node *transformLimitClause(ParseState *pstate, Node *clause,
+                                 const char *constructName);
 extern List *transformGroupClause(ParseState *pstate, List *grouplist,
                     List *targetlist, List *sortClause);
 extern List *transformSortClause(ParseState *pstate, List *orderlist,
index 5facbc7d7d4fa43a8866718cd494f4eb8b0aa9ae..88d7b614bb9dd020bf3cf301a958c730be6933e7 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $Id: parse_coerce.h,v 1.52 2003/07/01 19:10:53 tgl Exp $
+ * $Id: parse_coerce.h,v 1.53 2003/07/03 19:07:54 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -54,6 +54,8 @@ extern Node *coerce_to_domain(Node *arg, Oid baseTypeId, Oid typeId,
 
 extern Node *coerce_to_boolean(ParseState *pstate, Node *node,
                               const char *constructName);
+extern Node *coerce_to_integer(ParseState *pstate, Node *node,
+                              const char *constructName);
 
 extern Oid select_common_type(List *typeids, const char *context);
 extern Node *coerce_to_common_type(ParseState *pstate, Node *node,