From a998a692475845e10ba12a24c5af44b22737bd50 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 26 Jul 2006 19:31:51 +0000 Subject: [PATCH] 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'. --- src/backend/executor/nodeLimit.c | 29 +++-------------- src/backend/optimizer/plan/planagg.c | 8 ++--- src/backend/optimizer/plan/planner.c | 3 +- src/backend/parser/parse_clause.c | 9 ++++-- src/backend/parser/parse_coerce.c | 48 +++++++++++++++------------- src/include/catalog/catversion.h | 4 +-- src/include/nodes/parsenodes.h | 6 ++-- src/include/nodes/plannodes.h | 5 ++- src/include/parser/parse_coerce.h | 8 ++--- 9 files changed, 54 insertions(+), 66 deletions(-) diff --git a/src/backend/executor/nodeLimit.c b/src/backend/executor/nodeLimit.c index 99c474b161..935b59a722 100644 --- a/src/backend/executor/nodeLimit.c +++ b/src/backend/executor/nodeLimit.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/nodeLimit.c,v 1.26 2006/07/26 00:34:48 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/executor/nodeLimit.c,v 1.27 2006/07/26 19:31:50 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -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 30814a7e75..39e77897c0 100644 --- a/src/backend/optimizer/plan/planagg.c +++ b/src/backend/optimizer/plan/planagg.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/plan/planagg.c,v 1.18 2006/07/14 14:52:20 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/plan/planagg.c,v 1.19 2006/07/26 19:31:50 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -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 5545da4978..c02f6e195c 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.204 2006/07/26 00:34:48 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.205 2006/07/26 19:31:50 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -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 2b95722530..d34529c74e 100644 --- a/src/backend/parser/parse_clause.c +++ b/src/backend/parser/parse_clause.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/parser/parse_clause.c,v 1.154 2006/07/26 00:34:48 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/parser/parse_clause.c,v 1.155 2006/07/26 19:31:51 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -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 6aa04b7b82..010c704c68 100644 --- a/src/backend/parser/parse_coerce.c +++ b/src/backend/parser/parse_coerce.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/parser/parse_coerce.c,v 2.142 2006/07/26 00:34:48 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/parser/parse_coerce.c,v 2.143 2006/07/26 19:31:51 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -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 93ce0809e0..bcb2232e0b 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -37,7 +37,7 @@ * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/catalog/catversion.h,v 1.340 2006/07/25 03:51:21 tgl Exp $ + * $PostgreSQL: pgsql/src/include/catalog/catversion.h,v 1.341 2006/07/26 19:31:51 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -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 9198dadf5a..c44ba954d8 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.317 2006/07/13 16:49:19 momjian Exp $ + * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.318 2006/07/26 19:31:51 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -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 59425e7229..78a472342b 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/nodes/plannodes.h,v 1.83 2006/03/05 15:58:57 momjian Exp $ + * $PostgreSQL: pgsql/src/include/nodes/plannodes.h,v 1.84 2006/07/26 19:31:51 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -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 a5afff496c..84d119df76 100644 --- a/src/include/parser/parse_coerce.h +++ b/src/include/parser/parse_coerce.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2006, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/parser/parse_coerce.h,v 1.64 2006/07/26 00:34:48 momjian Exp $ + * $PostgreSQL: pgsql/src/include/parser/parse_coerce.h,v 1.65 2006/07/26 19:31:51 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -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,