Subject: Re: [PATCHES] Patch for - Change LIMIT/OFFSET to use int8
From: Dhanaraj M <[email-masked]>
To: [email-masked]
Date: Wed, 12 Jul 2006 01:06:13 +0530

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 <[email-masked]> 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 [email-masked] 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,