Date: Wed, 12 Jul 2006 01:06:13 +0530 From: Dhanaraj M Subject: Re: [PATCHES] Patch for - Change LIMIT/OFFSET to use int8 In-reply-to: <29360.1151334585@sss.pgh.pa.us> To: pgsql-patches@postgresql.org Message-id: <44B3FDAD.1030102@sun.com> MIME-version: 1.0 Content-type: multipart/mixed; boundary="Boundary_(ID_ox0qjGrmqUP9JwpNgv9xTQ)" X-Accept-Language: en-us, en References: <449FD6DA.5040808@sun.com> <29360.1151334585@sss.pgh.pa.us> User-Agent: Mozilla Thunderbird 1.0.2 (X11/20050323) This is a multi-part message in MIME format. --Boundary_(ID_ox0qjGrmqUP9JwpNgv9xTQ) Content-type: text/plain; format=flowed; charset=ISO-8859-1 Content-transfer-encoding: 7BIT 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 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@postgresql.org so that your > message can get through to the mailing list cleanly > > --Boundary_(ID_ox0qjGrmqUP9JwpNgv9xTQ) Content-type: text/plain; name=limit-offset_patch.txt Content-transfer-encoding: 7BIT Content-disposition: inline; filename=limit-offset_patch.txt *** ./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, --Boundary_(ID_ox0qjGrmqUP9JwpNgv9xTQ)--