Re: [Fwd: Re: Patch for - Change LIMIT/OFFSET to use int8]

Patch applied. Thanks.

It had quite a number of tab/space alignment problems that I fixed.

---------------------------------------------------------------------------

Dhanaraj M wrote:
> I sent this patch already.
> Can somebody verify this patch?
>
> Thanks
> Dhanaraj

-- Start of included mail From: Dhanaraj M <Dhanaraj(dot)M(at)Sun(dot)COM>

> Date: Wed, 12 Jul 2006 01:06:13 +0530
> Subject: Re: [PATCHES] Patch for - Change LIMIT/OFFSET to use int8
> To: pgsql-patches(at)postgresql(dot)org

> I have made the changes appropriately. The regression tests passed.
> Since I do not have enough resources, I could not test for a large number.
> It works for a small table. If anybody tests for int8 value, it is
> appreciated.
> Also, it gives the following error msg, when the input exceeds the int8
> limit.
>
> ERROR: bigint out of range
>
> I attach the patch. Pl. check it.
> Thanks
> Dhanaraj
>
> Tom Lane wrote:
>
> >Dhanaraj M <Dhanaraj(dot)M(at)Sun(dot)COM> writes:
> >
> >
> >>I attach the patch for the following TODO item.
> >> SQL COMMAND
> >> * Change LIMIT/OFFSET to use int8
> >>
> >>
> >
> >This can't possibly be correct. It doesn't even change the field types
> >in struct LimitState, for example. You've missed half a dozen places
> >in the planner that would need work, too.
> >
> > regards, tom lane
> >
> >---------------------------(end of broadcast)---------------------------
> >TIP 1: if posting/reading through Usenet, please send an appropriate
> > subscribe-nomail command to majordomo(at)postgresql(dot)org so that your
> > message can get through to the mailing list cleanly
> >
> >
>

> *** ./src/backend/executor/nodeLimit.c.orig Tue Jul 11 22:31:51 2006
> --- ./src/backend/executor/nodeLimit.c Wed Jul 12 00:46:11 2006
> ***************
> *** 23,28 ****
> --- 23,29 ----
>
> #include "executor/executor.h"
> #include "executor/nodeLimit.h"
> + #include "catalog/pg_type.h"
>
> static void recompute_limits(LimitState *node);
>
> ***************
> *** 226,239 ****
> {
> ExprContext *econtext = node->ps.ps_ExprContext;
> bool isNull;
>
> - if (node->limitOffset)
> - {
> - node->offset =
> - DatumGetInt32(ExecEvalExprSwitchContext(node->limitOffset,
> - econtext,
> - &isNull,
> - NULL));
> /* Interpret NULL offset as no offset */
> if (isNull)
> node->offset = 0;
> --- 227,251 ----
> {
> ExprContext *econtext = node->ps.ps_ExprContext;
> bool isNull;
> + Oid type;
> +
> + if (node->limitOffset)
> + {
> + type = ((Const *) node->limitOffset->expr)->consttype;
> +
> + if(type == INT8OID)
> + 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;
> ***************
> *** 249,259 ****
> if (node->limitCount)
> {
> node->noCount = false;
> ! node->count =
> ! DatumGetInt32(ExecEvalExprSwitchContext(node->limitCount,
> ! econtext,
> ! &isNull,
> ! NULL));
> /* Interpret NULL count as no count (LIMIT ALL) */
> if (isNull)
> node->noCount = true;
> --- 261,282 ----
> if (node->limitCount)
> {
> node->noCount = false;
> ! type = ((Const *) node->limitCount->expr)->consttype;
> !
> ! if(type == INT8OID)
> ! 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;
> *** ./src/backend/optimizer/plan/createplan.c.orig Tue Jul 11 22:32:31 2006
> --- ./src/backend/optimizer/plan/createplan.c Tue Jul 11 22:50:33 2006
> ***************
> *** 2865,2871 ****
> */
> Limit *
> make_limit(Plan *lefttree, Node *limitOffset, Node *limitCount,
> ! int offset_est, int count_est)
> {
> Limit *node = makeNode(Limit);
> Plan *plan = &node->plan;
> --- 2865,2871 ----
> */
> Limit *
> make_limit(Plan *lefttree, Node *limitOffset, Node *limitCount,
> ! int64 offset_est, int64 count_est)
> {
> Limit *node = makeNode(Limit);
> Plan *plan = &node->plan;
> *** ./src/backend/optimizer/plan/planner.c.orig Tue Jul 11 22:32:55 2006
> --- ./src/backend/optimizer/plan/planner.c Wed Jul 12 00:47:50 2006
> ***************
> *** 61,67 ****
> static Plan *grouping_planner(PlannerInfo *root, double tuple_fraction);
> static double preprocess_limit(PlannerInfo *root,
> double tuple_fraction,
> ! int *offset_est, int *count_est);
> static bool choose_hashed_grouping(PlannerInfo *root, double tuple_fraction,
> Path *cheapest_path, Path *sorted_path,
> double dNumGroups, AggClauseCounts *agg_counts);
> --- 61,67 ----
> static Plan *grouping_planner(PlannerInfo *root, double tuple_fraction);
> static double preprocess_limit(PlannerInfo *root,
> double tuple_fraction,
> ! int64 *offset_est, int64 *count_est);
> static bool choose_hashed_grouping(PlannerInfo *root, double tuple_fraction,
> Path *cheapest_path, Path *sorted_path,
> double dNumGroups, AggClauseCounts *agg_counts);
> ***************
> *** 633,640 ****
> {
> Query *parse = root->parse;
> List *tlist = parse->targetList;
> ! int offset_est = 0;
> ! int count_est = 0;
> Plan *result_plan;
> List *current_pathkeys;
> List *sort_pathkeys;
> --- 633,640 ----
> {
> Query *parse = root->parse;
> List *tlist = parse->targetList;
> ! int64 offset_est = 0;
> ! int64 count_est = 0;
> Plan *result_plan;
> List *current_pathkeys;
> List *sort_pathkeys;
> ***************
> *** 1082,1088 ****
> */
> static double
> preprocess_limit(PlannerInfo *root, double tuple_fraction,
> ! int *offset_est, int *count_est)
> {
> Query *parse = root->parse;
> Node *est;
> --- 1082,1088 ----
> */
> static double
> preprocess_limit(PlannerInfo *root, double tuple_fraction,
> ! int64 *offset_est, int64 *count_est)
> {
> Query *parse = root->parse;
> Node *est;
> ***************
> *** 1107,1113 ****
> }
> else
> {
> ! *count_est = DatumGetInt32(((Const *) est)->constvalue);
> if (*count_est <= 0)
> *count_est = 1; /* force to at least 1 */
> }
> --- 1107,1114 ----
> }
> else
> {
> ! *count_est = DatumGetInt64(((Const *) est)->constvalue);
> !
> if (*count_est <= 0)
> *count_est = 1; /* force to at least 1 */
> }
> ***************
> *** 1130,1136 ****
> }
> else
> {
> ! *offset_est = DatumGetInt32(((Const *) est)->constvalue);
> if (*offset_est < 0)
> *offset_est = 0; /* less than 0 is same as 0 */
> }
> --- 1131,1138 ----
> }
> else
> {
> ! *offset_est = DatumGetInt64(((Const *) est)->constvalue);
> !
> if (*offset_est < 0)
> *offset_est = 0; /* less than 0 is same as 0 */
> }
> *** ./src/backend/parser/parse_clause.c.orig Tue Jul 11 22:33:21 2006
> --- ./src/backend/parser/parse_clause.c Tue Jul 11 22:56:17 2006
> ***************
> *** 1091,1097 ****
>
> qual = transformExpr(pstate, clause);
>
> ! qual = coerce_to_integer(pstate, qual, constructName);
>
> /*
> * LIMIT can't refer to any vars or aggregates of the current query; we
> --- 1091,1097 ----
>
> qual = transformExpr(pstate, clause);
>
> ! qual = coerce_to_integer64(pstate, qual, constructName);
>
> /*
> * LIMIT can't refer to any vars or aggregates of the current query; we
> *** ./src/backend/parser/parse_coerce.c.orig Tue Jul 11 22:33:41 2006
> --- ./src/backend/parser/parse_coerce.c Wed Jul 12 00:58:57 2006
> ***************
> *** 822,828 ****
>
> /* 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.
> *
> --- 822,828 ----
>
> /* 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.
> *
> ***************
> *** 858,865 ****
>
> return node;
> }
>
> -
> /* select_common_type()
> * Determine the common supertype of a list of input expression types.
> * This is used for determining the output type of CASE and UNION
> --- 858,905 ----
>
> 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.
> + *
> + * 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_integer64(ParseState *pstate, Node *node,
> + const char *constructName)
> +
> + {
> + Oid inputTypeId = exprType(node);
> +
> + if (inputTypeId != INT8OID)
> + {
> + node = coerce_to_target_type(pstate, node, inputTypeId,
> + 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))));
> + }
> +
> + 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)));
> +
> + return node;
> + }
> +
>
> /* select_common_type()
> * Determine the common supertype of a list of input expression types.
> * This is used for determining the output type of CASE and UNION
> *** ./src/include/nodes/execnodes.h.orig Tue Jul 11 22:34:08 2006
> --- ./src/include/nodes/execnodes.h Tue Jul 11 22:59:43 2006
> ***************
> *** 1328,1338 ****
> PlanState ps; /* its first field is NodeTag */
> ExprState *limitOffset; /* OFFSET parameter, or NULL if none */
> ExprState *limitCount; /* COUNT parameter, or NULL if none */
> ! long offset; /* current OFFSET value */
> ! long count; /* current COUNT, if any */
> bool noCount; /* if true, ignore count */
> LimitStateCond lstate; /* state machine status, as above */
> ! long position; /* 1-based index of last tuple returned */
> TupleTableSlot *subSlot; /* tuple last obtained from subplan */
> } LimitState;
>
> --- 1328,1338 ----
> PlanState ps; /* its first field is NodeTag */
> ExprState *limitOffset; /* OFFSET parameter, or NULL if none */
> ExprState *limitCount; /* COUNT parameter, or NULL if none */
> ! int64 offset; /* current OFFSET value */
> ! int64 count; /* current COUNT, if any */
> bool noCount; /* if true, ignore count */
> LimitStateCond lstate; /* state machine status, as above */
> ! int64 position; /* 1-based index of last tuple returned */
> TupleTableSlot *subSlot; /* tuple last obtained from subplan */
> } LimitState;
>
> *** ./src/include/optimizer/planmain.h.orig Tue Jul 11 22:36:57 2006
> --- ./src/include/optimizer/planmain.h Tue Jul 11 23:00:30 2006
> ***************
> *** 55,61 ****
> extern Plan *materialize_finished_plan(Plan *subplan);
> extern Unique *make_unique(Plan *lefttree, List *distinctList);
> extern Limit *make_limit(Plan *lefttree, Node *limitOffset, Node *limitCount,
> ! int offset_est, int count_est);
> extern SetOp *make_setop(SetOpCmd cmd, Plan *lefttree,
> List *distinctList, AttrNumber flagColIdx);
> extern Result *make_result(List *tlist, Node *resconstantqual, Plan *subplan);
> --- 55,61 ----
> extern Plan *materialize_finished_plan(Plan *subplan);
> extern Unique *make_unique(Plan *lefttree, List *distinctList);
> extern Limit *make_limit(Plan *lefttree, Node *limitOffset, Node *limitCount,
> ! int64 offset_est, int64 count_est);
> extern SetOp *make_setop(SetOpCmd cmd, Plan *lefttree,
> List *distinctList, AttrNumber flagColIdx);
> extern Result *make_result(List *tlist, Node *resconstantqual, Plan *subplan);
> *** ./src/include/parser/parse_coerce.h.orig Tue Jul 11 22:37:23 2006
> --- ./src/include/parser/parse_coerce.h Tue Jul 11 23:01:07 2006
> ***************
> *** 58,64 ****
> 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,
> Oid targetTypeId,
> --- 58,66 ----
> 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 Oid select_common_type(List *typeids, const char *context);
> extern Node *coerce_to_common_type(ParseState *pstate, Node *node,
> Oid targetTypeId,
-- End of included mail.

>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
>
> http://www.postgresql.org/docs/faq

--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +