Re: [PATCHES] Patch for - Change FETCH/MOVE to use int8
Patch applied. Thanks. I had to convert a lot of whitespace to tabs. It wasn't just the whitespace, but whitespace that was 8 spaces. I assume you are reading our code using an 8-space tab. Please see the developer's FAQ and try to use tabs in future patches. Thanks. --- Dhanaraj M wrote: Hi Alvaro Thanks for your valuable suggestions. I made the changes as suggested earlier. Please review again and comment on this. I like to make changes if it is required. ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Patch for - Change FETCH/MOVE to use int8
bruce wrote:
Patch applied. Thanks.
I had to convert a lot of whitespace to tabs. It wasn't just the
whitespace, but whitespace that was 8 spaces. I assume you are reading
our code using an 8-space tab. Please see the developer's FAQ and try
to use tabs in future patches. Thanks.
I have reverted this patch. It was causing crashes in the ecpg
regression tests. I think the problem was in scan.l:
+ /* For Fetch/Move stmt, convert the string into int64
value */
+ if (strcmp(yylval.keyword, fetch) == 0 ||
+ strcmp(yylval.keyword, move) == 0)
+ {
+ int64 int64Val;
+ errno = 0;
This is code that was in the 'integer' section. Why did you think you
could compare a non-assigned yylval at this stage? Anyway, this isn't
going into 8.2.
---
Dhanaraj M wrote:
Hi Alvaro
Thanks for your valuable suggestions.
I made the changes as suggested earlier.
Please review again and comment on this.
I like to make changes if it is required.
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
--
Bruce Momjian [EMAIL PROTECTED]
EnterpriseDBhttp://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Patch for - Change FETCH/MOVE to use int8
Hi Alvaro
Thanks for your valuable suggestions.
I made the changes as suggested earlier.
Please review again and comment on this.
I like to make changes if it is required.
*** ./src/backend/commands/portalcmds.c.orig Sat Aug 12 23:04:54 2006
--- ./src/backend/commands/portalcmds.c Fri Aug 18 22:52:05 2006
***
*** 176,183
char *completionTag)
{
Portal portal;
! long nprocessed;
/*
* Disallow empty-string cursor name (conflicts with protocol-level
* unnamed portal).
--- 176,183
char *completionTag)
{
Portal portal;
! int64 nprocessed;
/*
* Disallow empty-string cursor name (conflicts with protocol-level
* unnamed portal).
***
*** 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 INT64_FORMAT,
stmt-ismove ? MOVE : FETCH,
nprocessed);
}
*** ./src/backend/executor/spi.c.orig Sat Aug 12 23:04:55 2006
--- ./src/backend/executor/spi.c Fri Aug 18 02:14:20 2006
***
*** 45,51
static void _SPI_error_callback(void *arg);
! static void _SPI_cursor_operation(Portal portal, bool forward, long count,
DestReceiver *dest);
static _SPI_plan *_SPI_copy_plan(_SPI_plan *plan, int location);
--- 45,51
static void _SPI_error_callback(void *arg);
! static void _SPI_cursor_operation(Portal portal, bool forward, int64 count,
DestReceiver *dest);
static _SPI_plan *_SPI_copy_plan(_SPI_plan *plan, int location);
***
*** 980,986
* Fetch rows in a cursor
*/
void
! SPI_cursor_fetch(Portal portal, bool forward, long count)
{
_SPI_cursor_operation(portal, forward, count,
CreateDestReceiver(DestSPI, NULL));
--- 980,986
* Fetch rows in a cursor
*/
void
! SPI_cursor_fetch(Portal portal, bool forward, int64 count)
{
_SPI_cursor_operation(portal, forward, count,
CreateDestReceiver(DestSPI, NULL));
***
*** 994,1000
* Move in a cursor
*/
void
! SPI_cursor_move(Portal portal, bool forward, long count)
{
_SPI_cursor_operation(portal, forward, count, None_Receiver);
}
--- 994,1000
* Move in a cursor
*/
void
! SPI_cursor_move(Portal portal, bool forward, int64 count)
{
_SPI_cursor_operation(portal, forward, count, None_Receiver);
}
***
*** 1611,1620
* Do a FETCH or MOVE in a cursor
*/
static void
! _SPI_cursor_operation(Portal portal, bool forward, long count,
DestReceiver *dest)
{
! long nfetched;
/* Check that the portal is valid */
if (!PortalIsValid(portal))
--- 1611,1620
* Do a FETCH or MOVE in a cursor
*/
static void
! _SPI_cursor_operation(Portal portal, bool forward, int64 count,
DestReceiver *dest)
{
! int64 nfetched;
/* Check that the portal is valid */
if (!PortalIsValid(portal))
*** ./src/backend/parser/gram.y.orig Fri Aug 18 23:37:43 2006
--- ./src/backend/parser/gram.y Fri Aug 18 01:12:58 2006
***
*** 117,122
--- 117,123
%union
{
intival;
+ int64i64val;
charchr;
char*str;
const char *keyword;
***
*** 323,328
--- 324,330
%type boolean opt_varying opt_timezone
%type ival Iconst SignedIconst
+ %type i64val SignedI64const
%type str Sconst comment_text
%type str RoleId opt_granted_by opt_boolean ColId_or_Sconst
%type list var_list var_list_or_default
***
*** 446,451
--- 448,454
/* Special token types, not actually keywords - see the lex file */
%token str IDENT FCONST SCONST BCONST XCONST Op
%token ival ICONST PARAM
+ %token i64val I64CONST
/* precedence: lowest to highest */
%nonassoc SET/* see relation_expr_opt_alias */
***
*** 3334,3339
--- 3337,3363
n-howMany = $1;
$$ = (Node *)n;
}
+ | ABSOLUTE_P SignedI64const
+ {
+ FetchStmt *n = makeNode(FetchStmt);
+ n-direction = FETCH_ABSOLUTE;
+ n-howMany = $2;
+ $$ = (Node *)n;
+ }
+ | RELATIVE_P SignedI64const
+ {
+ FetchStmt *n = makeNode(FetchStmt);
+ n-direction = FETCH_RELATIVE;
+ n-howMany = $2;
+ $$ = (Node *)n;
+ }
+ | SignedI64const
+ {
+
[PATCHES] Patch for - Change FETCH/MOVE to use int8
This patch is for the following TODO item.
SQL command:
-/Change LIMIT/OFFSET and FETCH/MOVE to use int8
/Since the limit/offset patch is already applied,
this patch is meant for Fetch/Move query.
I have tested the patch and it works for int64 values.
Please verify this.
Thanks
Dhanaraj
/
/
*** ./src/backend/commands/portalcmds.c.orig Sat Aug 12 23:04:54 2006
--- ./src/backend/commands/portalcmds.c Sat Aug 12 23:04:53 2006
***
*** 176,183
char *completionTag)
{
Portal portal;
! long nprocessed;
/*
* Disallow empty-string cursor name (conflicts with protocol-level
* unnamed portal).
--- 176,183
char *completionTag)
{
Portal portal;
! int64 nprocessed;
/*
* Disallow empty-string cursor name (conflicts with protocol-level
* unnamed portal).
***
*** 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);
}
*** ./src/backend/parser/gram.y.orig Sat Aug 12 23:04:57 2006
--- ./src/backend/parser/gram.y Sun Aug 13 00:06:28 2006
***
*** 116,122
%union
{
! int ival;
charchr;
char*str;
const char *keyword;
--- 116,122
%union
{
! int64ival;
charchr;
char*str;
const char *keyword;
***
*** 1180,1192
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg(INTERVAL(%d) precision must not be negative,
! $3)));
if ($3 MAX_INTERVAL_PRECISION)
{
ereport(WARNING,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg(INTERVAL(%d) precision reduced to maximum allowed, %d,
! $3, MAX_INTERVAL_PRECISION)));
$3 = MAX_INTERVAL_PRECISION;
}
--- 1180,1192
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg(INTERVAL(%d) precision must not be negative,
! (int)$3)));
if ($3 MAX_INTERVAL_PRECISION)
{
ereport(WARNING,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg(INTERVAL(%d) precision reduced to maximum allowed, %d,
! (int)$3, MAX_INTERVAL_PRECISION)));
$3 = MAX_INTERVAL_PRECISION;
}
***
*** 2620,2626
ICONST
{
char buf[64];
! snprintf(buf, sizeof(buf), %d, $1);
$$ = makeString(pstrdup(buf));
}
| FCONST{ $$ = makeString($1); }
--- 2620,2626
ICONST
{
char buf[64];
! snprintf(buf, sizeof(buf), %d, (int)$1);
$$ = makeString(pstrdup(buf));
}
| FCONST{ $$ = makeString($1); }
***
*** 6281,6293
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg(INTERVAL(%d) precision must not be negative,
! $3)));
if ($3 MAX_INTERVAL_PRECISION)
{
ereport(WARNING,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg(INTERVAL(%d) precision reduced to maximum allowed, %d,
! $3, MAX_INTERVAL_PRECISION)));
$3 = MAX_INTERVAL_PRECISION;
}
$$-typmod = INTERVAL_TYPMOD($3, $5);
--- 6281,6293
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg(INTERVAL(%d) precision must not be negative,
! (int)$3)));
if ($3 MAX_INTERVAL_PRECISION)
{
ereport(WARNING,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg(INTERVAL(%d) precision reduced to maximum allowed, %d,
! (int)$3, MAX_INTERVAL_PRECISION)));
$3 = MAX_INTERVAL_PRECISION;
}
$$-typmod = INTERVAL_TYPMOD($3, $5);
***
*** 6408,6419
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg(NUMERIC precision %d must be between 1 and %d,
! $2, NUMERIC_MAX_PRECISION)));
if ($4 0 || $4 $2)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg(NUMERIC scale %d must be between 0 and precision %d,
! $4, $2)));
$$ = (($2 16) | $4) + VARHDRSZ;
}
--- 6408,6419
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg(NUMERIC precision %d must be between 1 and %d,
! (int)$2, NUMERIC_MAX_PRECISION)));
if ($4 0 || $4 $2)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg(NUMERIC scale %d must be between 0 and precision %d,
! (int)$4, (int)$2)));
$$ = (($2 16) | $4) + VARHDRSZ;
}
***
*** 6423,6429
Re: [PATCHES] Patch for - Change FETCH/MOVE to use int8
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;
charchr;
char*str;
const char *keyword;
--- 116,122
%union
{
! int64 ival;
charchr;
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 Herrerahttp://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?
http://www.postgresql.org/docs/faq
Re: [PATCHES] Patch for - Change FETCH/MOVE to use int8
Alvaro Herrera [EMAIL PROTECTED] writes: 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. I'm not sure that I see the point of this at all. ISTM the entire reason for using a cursor is that you're going to fetch the results in bite-size pieces. I don't see the current Postgres source code surviving into the era where 2G rows is considered bite-size ;-) I thought the int8-LIMIT patch was equally pointless, btw, but at least it was not very invasive. This one is not passing the minimum usefulness-to-ugliness ratio for me. 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
Re: [PATCHES] Patch for - Change FETCH/MOVE to use int8
Tom Lane wrote: Alvaro Herrera [EMAIL PROTECTED] writes: 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. I'm not sure that I see the point of this at all. ISTM the entire reason for using a cursor is that you're going to fetch the results in bite-size pieces. I don't see the current Postgres source code surviving into the era where 2G rows is considered bite-size ;-) Think MOVE to a specific section of the cursor 2gig. I can see that happening. -- Bruce Momjian [EMAIL PROTECTED] EnterpriseDBhttp://www.enterprisedb.com + If your life is a hard drive, Christ can be your backup. + ---(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
Re: [PATCHES] Patch for - Change FETCH/MOVE to use int8
Bruce Momjian [EMAIL PROTECTED] writes: Tom Lane wrote: I'm not sure that I see the point of this at all. ISTM the entire reason for using a cursor is that you're going to fetch the results in bite-size pieces. I don't see the current Postgres source code surviving into the era where 2G rows is considered bite-size ;-) Think MOVE to a specific section of the cursor 2gig. I can see that happening. Yeah, and by the time it happens you'll have gotten bored and found something else to do. With no support in the system for random access to a cursor result, this is just about as useless as the FETCH case. In any case I agree with Alvaro's comment: the way to support int8 in a FETCH/MOVE command is not to try to convert the entire rest of the grammar to int8 instead of int4 as its native datatype. regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
