Code review for bigint-LIMIT patch. Fix missed planner dependency,

eliminate unnecessary code, force initdb because stored rules change
(limit nodes are now supposed to be int8 not int4 expressions).
Update comments and error messages, which still all said 'integer'.

diff --git a/src/backend/executor/nodeLimit.c b/src/backend/executor/nodeLimit.c
index fbe0669f8a0c37160a4333d9e0f32e5b17980082..3a2e4c61e747b87b5c8c32aebde7704114bd4e14 100644 (file)
--- a/src/backend/executor/nodeLimit.c
+++ b/src/backend/executor/nodeLimit.c
@@ -23,7 +23,6 @@
 
 #include "executor/executor.h"
 #include "executor/nodeLimit.h"
-#include "catalog/pg_type.h"
 
 static void recompute_limits(LimitState *node);
 
@@ -227,24 +226,14 @@ recompute_limits(LimitState *node)
 {
        ExprContext *econtext = node->ps.ps_ExprContext;
        bool            isNull;
-       Oid type;
-  
+
        if (node->limitOffset)
        {
-               type = ((Const *) node->limitOffset->expr)->consttype;
-  
-               if (type == INT8OID)
-                       node->offset =
+               node->offset =
                        DatumGetInt64(ExecEvalExprSwitchContext(node->limitOffset,
                                                                                                        econtext,
                                                                                                        &isNull,
                                                                                                        NULL));
-               else
-                       node->offset = DatumGetInt32(ExecEvalExprSwitchContext(node->limitOffset,
-                                                                                                                                  econtext,
-                                                                                                                                  &isNull,
-                                                                                                                                  NULL));
-
                /* Interpret NULL offset as no offset */
                if (isNull)
                        node->offset = 0;
@@ -260,21 +249,11 @@ recompute_limits(LimitState *node)
        if (node->limitCount)
        {
                node->noCount = false;
-               type = ((Const *) node->limitCount->expr)->consttype;
- 
-               if (type == INT8OID)
-                       node->count =
+               node->count =
                        DatumGetInt64(ExecEvalExprSwitchContext(node->limitCount,
                                                                                                        econtext,
                                                                                                        &isNull,
                                                                                                        NULL));
-               else
-                       node->count = DatumGetInt32(ExecEvalExprSwitchContext(node->limitCount,
-                                                                                                                                 econtext,
-                                                                                                                                 &isNull,
-                                                                                                                                 NULL));
- 
-
                /* Interpret NULL count as no count (LIMIT ALL) */
                if (isNull)
                        node->noCount = true;

diff --git a/src/backend/optimizer/plan/planagg.c b/src/backend/optimizer/plan/planagg.c
index 03c1abca3a336cd8f11df028ebfb93ce5aeecef5..ac1226c187643d9dd063022ef72f57f319beab5f 100644 (file)
--- a/src/backend/optimizer/plan/planagg.c
+++ b/src/backend/optimizer/plan/planagg.c
@@ -459,9 +459,9 @@ make_agg_subplan(PlannerInfo *root, MinMaxAggInfo *info)
 
        /* set up LIMIT 1 */
        subparse->limitOffset = NULL;
-       subparse->limitCount = (Node *) makeConst(INT4OID, sizeof(int4),
-                                                                                         Int32GetDatum(1),
-                                                                                         false, true);
+       subparse->limitCount = (Node *) makeConst(INT8OID, sizeof(int64),
+                                                                                         Int64GetDatum(1),
+                                                                                         false, false /* not by val */);
 
        /*
         * Generate the plan for the subquery.  We already have a Path for the

diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 785b59d32ae5bc9ba675f509593383399aaf7b01..36c9fce69dee37479e11a0ae83017c124df4e8fc 100644 (file)
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -1129,7 +1129,6 @@ preprocess_limit(PlannerInfo *root, double tuple_fraction,
                        else
                        {
                                *offset_est = DatumGetInt64(((Const *) est)->constvalue);
-
                                if (*offset_est < 0)
                                        *offset_est = 0;        /* less than 0 is same as 0 */
                        }

diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index add229e23b3d5ea14549e00cf86d620b4e33d419..b224ef21075d4b49980515f5710cc03c916e6e5f 100644 (file)
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -1074,9 +1074,12 @@ transformWhereClause(ParseState *pstate, Node *clause,
 
 /*
  * transformLimitClause -
- *       Transform the expression and make sure it is of type integer.
+ *       Transform the expression and make sure it is of type bigint.
  *       Used for LIMIT and allied clauses.
  *
+ * Note: as of Postgres 8.2, LIMIT expressions are expected to yield int8,
+ * rather than int4 as before.
+ *
  * constructName does not affect the semantics, but is used in error messages
  */
 Node *
@@ -1090,7 +1093,7 @@ transformLimitClause(ParseState *pstate, Node *clause,
 
        qual = transformExpr(pstate, clause);
 
-       qual = coerce_to_integer64(pstate, qual, constructName);
+       qual = coerce_to_bigint(pstate, qual, constructName);
 
        /*
         * LIMIT can't refer to any vars or aggregates of the current query; we

diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c
index f57dafbc2b9e94df9c895f5d747c602d8ddd7fdf..5e13577c9dd4f17849d4f9dbaa89d526df84fe02 100644 (file)
--- a/src/backend/parser/parse_coerce.c
+++ b/src/backend/parser/parse_coerce.c
@@ -780,7 +780,8 @@ coerce_record_to_complex(ParseState *pstate, Node *node,
        return (Node *) rowexpr;
 }
 
-/* coerce_to_boolean()
+/*
+ * coerce_to_boolean()
  *             Coerce an argument of a construct that requires boolean input
  *             (AND, OR, NOT, etc).  Also check that input is not a set.
  *
@@ -819,8 +820,9 @@ coerce_to_boolean(ParseState *pstate, Node *node,
        return node;
 }
 
-/* coerce_to_integer()
- *             Coerce an argument of a construct that requires integer input
+/*
+ * coerce_to_integer()
+ *             Coerce an argument of a construct that requires integer input.
  *             Also check that input is not a set.
  *
  * Returns the possibly-transformed node tree.
@@ -857,10 +859,11 @@ coerce_to_integer(ParseState *pstate, Node *node,
 
        return node;
 }
- 
-/* coerce_to_integer64()
- *              Coerce an argument of a construct that requires integer input
- *              (LIMIT, OFFSET).  Also check that input is not a set.
+
+/*
+ * coerce_to_bigint()
+ *             Coerce an argument of a construct that requires int8 input.
+ *             Also check that input is not a set.
  *
  * Returns the possibly-transformed node tree.
  *
@@ -868,34 +871,35 @@ coerce_to_integer(ParseState *pstate, Node *node,
  * processing is wanted.
  */
 Node *
-coerce_to_integer64(ParseState *pstate, Node *node,
-                                       const char *constructName)
+coerce_to_bigint(ParseState *pstate, Node *node,
+                                const char *constructName)
 {
-       Oid     inputTypeId = exprType(node);
+       Oid                     inputTypeId = exprType(node);
 
        if (inputTypeId != INT8OID)
        {
                node = coerce_to_target_type(pstate, node, inputTypeId,
-                                                                        INT8OID, -1, COERCION_ASSIGNMENT,
+                                                                        INT8OID, -1,
+                                                                        COERCION_ASSIGNMENT,
                                                                         COERCE_IMPLICIT_CAST);
                if (node == NULL)
-                               ereport(ERROR,
-                                                 (errcode(ERRCODE_DATATYPE_MISMATCH),
-                                         /* translator: first %s is name of a SQL construct, eg LIMIT */
-                                                        errmsg("argument of %s must be type integer, not type %s",
-                                                                       constructName, format_type_be(inputTypeId))));
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_DATATYPE_MISMATCH),
+                       /* translator: first %s is name of a SQL construct, eg LIMIT */
+                                  errmsg("argument of %s must be type bigint, not type %s",
+                                                 constructName, format_type_be(inputTypeId))));
        }
 
        if (expression_returns_set(node))
                ereport(ERROR,
-                                 (errcode(ERRCODE_DATATYPE_MISMATCH),
-                 /* translator: %s is name of a SQL construct, eg LIMIT */
-                                          errmsg("argument of %s must not return a set",
-                                                         constructName)));
+                               (errcode(ERRCODE_DATATYPE_MISMATCH),
+               /* translator: %s is name of a SQL construct, eg LIMIT */
+                                errmsg("argument of %s must not return a set",
+                                               constructName)));
 
        return node;
 }
-  
+
 
 /* select_common_type()
  *             Determine the common supertype of a list of input expression types.

diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 627dcd9b80886e8e68cd9728dd94876c56805521..23ebc3644a235544495d5666832a95ccb58df622 100644 (file)
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
  */
 
 /*                                                     yyyymmddN */
-#define CATALOG_VERSION_NO     200607241
+#define CATALOG_VERSION_NO     200607261
 
 #endif

diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 14499f4566ba47eb9fa36d9812e3ebe8d0e6754c..594e5f236829af16215b69d1808f3e3ab031ff21 100644 (file)
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -112,8 +112,8 @@ typedef struct Query
 
        List       *sortClause;         /* a list of SortClause's */
 
-       Node       *limitOffset;        /* # of result tuples to skip */
-       Node       *limitCount;         /* # of result tuples to return */
+       Node       *limitOffset;        /* # of result tuples to skip (int8 expr) */
+       Node       *limitCount;         /* # of result tuples to return (int8 expr) */
 
        List       *rowMarks;           /* a list of RowMarkClause's */
 

diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 69f305bfd71bcf67b6cb43e5566151024e1cea82..bf135bdc0840571de7146577626dbdb55ba06c47 100644 (file)
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -463,6 +463,9 @@ typedef struct SetOp
 
 /* ----------------
  *             limit node
+ *
+ * Note: as of Postgres 8.2, the offset and count expressions are expected
+ * to yield int8, rather than int4 as before.
  * ----------------
  */
 typedef struct Limit

diff --git a/src/include/parser/parse_coerce.h b/src/include/parser/parse_coerce.h
index 184791e7ee67dd498cdaf6ee8be8087947143f1b..72404b17d398861a87451778a7ad2612e0979781 100644 (file)
--- a/src/include/parser/parse_coerce.h
+++ b/src/include/parser/parse_coerce.h
@@ -57,9 +57,9 @@ 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 Node *coerce_to_integer64(ParseState *pstate, Node *node,
-                                    const char *constructName);
- 
+extern Node *coerce_to_bigint(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,
                                          Oid targetTypeId,