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

2006-07-25 Thread Bruce Momjian 

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 <[EMAIL PROTECTED]>

> Date: Wed, 12 Jul 2006 01:06:13 +0530
> Subject: Re: [PATCHES] Patch for - Change LIMIT/OFFSET to use int8
> To: pgsql-patches@postgresql.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 <[EMAIL PROTECTED]> 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 PROTECTED] 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.cWed 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;
>   boolisNull;
>   
> - 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;
>   boolisNull;
> + 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->

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

2006-07-24 Thread Dhanaraj M 

I sent this patch already.
Can somebody verify this patch?

Thanks
Dhanaraj
--- Begin Message ---

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 PROTECTED]> 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 PROTECTED] 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;
boolisNull;
  
-   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;
boolisNull;
+   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,
!