Re: Patch for - Change FETCH/MOVE to use int8 - Mailing list pgsql-patches

From Alvaro Herrera
Subject Re: Patch for - Change FETCH/MOVE to use int8
Date
Msg-id 20060813221939.GA14441@alvh.no-ip.org
Whole thread Raw
In response to Patch for - Change FETCH/MOVE to use int8  (Dhanaraj M <Dhanaraj.M@Sun.COM>)
Responses Re: Patch for - Change FETCH/MOVE to use int8  (Tom Lane <tgl@sss.pgh.pa.us>)
Re: Patch for - Change FETCH/MOVE to use int8  (Dhanaraj M <Dhanaraj.M@Sun.COM>)
List pgsql-patches
Dhanaraj M wrote:


I had a quick look:

> ***************
> *** 209,215 ****
>
>       /* Return command status if wanted */
>       if (completionTag)
> !         snprintf(completionTag, COMPLETION_TAG_BUFSIZE, "%s %ld",
>                    stmt->ismove ? "MOVE" : "FETCH",
>                    nprocessed);
>   }
> --- 209,215 ----
>
>       /* Return command status if wanted */
>       if (completionTag)
> !         snprintf(completionTag, COMPLETION_TAG_BUFSIZE, "%s %lld",
>                    stmt->ismove ? "MOVE" : "FETCH",
>                    nprocessed);
>   }

You shouldn't be using %lld as it breaks on some platforms.
Use INT64_FORMAT instead.

> --- ./src/backend/parser/gram.y    Sun Aug 13 00:06:28 2006
> ***************
> *** 116,122 ****
>
>   %union
>   {
> !     int                    ival;
>       char                chr;
>       char                *str;
>       const char            *keyword;
> --- 116,122 ----
>
>   %union
>   {
> !     int64                ival;
>       char                chr;
>       char                *str;
>       const char            *keyword;

I don't think this is the right approach.  Maybe it would be reasonable
to add another arm to the %union instead, not sure.  The problem is the
amount of ugly casts you have to use below.  The scanner code seems to
think that a constant larger than the biggest int4 should be treated as
float, so I'm not sure why this would work anyway.


> ***************
> *** 767,773 ****
>       /*
>        * Force the queryDesc destination to the right thing.    This supports
>        * MOVE, for example, which will pass in dest = DestNone.  This is okay to
> !      * change as long as we do it on every fetch.  (The Executor must not
>        * assume that dest never changes.)
>        */
>       if (queryDesc)
> --- 767,773 ----
>       /*
>        * Force the queryDesc destination to the right thing.    This supports
>        * MOVE, for example, which will pass in dest = DestNone.  This is okay to
> !      * change as int64 as we do it on every fetch.  (The Executor must not
>        * assume that dest never changes.)
>        */
>       if (queryDesc)

Too enthusiastic about the search'n replace I think.

I stopped reading at this point.

--
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

pgsql-patches by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: [HACKERS] Custom variable class segmentation fault
Next
From: Tom Lane
Date:
Subject: Re: Patch for - Change FETCH/MOVE to use int8