Re: [HACKERS] [GENERAL] Insert result does not match record count
Did this every go any further? I wrote some data transformation script at work, and after seeing with count -2017657667 (and similar) in my scripts log I got a bit worried. seeing else where were folks just run a full on count(*) later to check counts but that is even MORE time and I was thinking it was a psycopg2 problem, but seems there are issues with the internal counters in pg as well for tracking large changes. thanks, Mark On Sun, Feb 2, 2014 at 9:12 AM, Tom Lane t...@sss.pgh.pa.us wrote: Vik Fearing vik.fear...@dalibo.com writes: Without re-doing the work, my IRC logs show that I was bothered by this in src/backend/tcop/postgres.c: max_rows = pq_getmsgint(input_message, 4); I needed to change max_rows to int64 which meant I had to change pq_getmsgint to pq_getmsgint64 which made me a little worried. As well you should be, because we are *not* doing that. That would be a guaranteed-incompatible protocol change. Fortunately, I don't see any functional need for widening the row-limit field in execute messages; how likely is it that someone wants to fetch exactly 3 billion rows? The practical use-cases for nonzero row limits generally involve fetching a bufferload worth of data at a time, so that the restriction to getting no more than INT_MAX rows at once is several orders of magnitude away from being a problem. The same goes for internal uses of row limits, which makes it questionable whether it's worth changing the width of ExecutorRun's count parameter, which is what I assume you were on about here. But in any case, if we did that we'd not try to reflect it as far as here, because the message format specs can't change. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GENERAL] Insert result does not match record count
On 02/01/2014 02:26 AM, Bruce Momjian wrote:
On Sat, Feb 1, 2014 at 02:25:16AM +0100, Vik Fearing wrote:
OK, thanks for the feedback. I understand now. The contents of the
string will potentially have a larger integer, but the byte length of
the string in the wire protocol doesn't change.
Let's wait for Vik to reply and I think we can move forward.
Unfortunately, I just did some cleanup last week and removed that
branch. Had I waited a bit more I still would have had all the work I
had done. I'll see how quickly I can redo it to get to the part where I
got scared of what I was doing.
It will have to wait until next week though; I am currently at FOSDEM.
OK, thanks. I thought it only required passing the int64 around until
it got into the string passed to the client. The original patch is in
the email archives if you want it.
The original patch didn't have much in the way of actual work done,
unfortunately.
Without re-doing the work, my IRC logs show that I was bothered by this
in src/backend/tcop/postgres.c:
case 'E':/* execute */
{
const char *portal_name;
intmax_rows;
forbidden_in_wal_sender(firstchar);
/* Set statement_timestamp() */
SetCurrentStatementStartTimestamp();
portal_name = pq_getmsgstring(input_message);
max_rows = pq_getmsgint(input_message, 4);
pq_getmsgend(input_message);
exec_execute_message(portal_name, max_rows);
}
break;
I needed to change max_rows to int64 which meant I had to change
pq_getmsgint to pq_getmsgint64 which made me a little worried. I was
already overwhelmed by how much code I was changing and this one made me
reconsider.
If it's just a n00b thing, I guess I can pick this back up for 9.5.
--
Vik
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GENERAL] Insert result does not match record count
Vik Fearing vik.fear...@dalibo.com writes: Without re-doing the work, my IRC logs show that I was bothered by this in src/backend/tcop/postgres.c: max_rows = pq_getmsgint(input_message, 4); I needed to change max_rows to int64 which meant I had to change pq_getmsgint to pq_getmsgint64 which made me a little worried. As well you should be, because we are *not* doing that. That would be a guaranteed-incompatible protocol change. Fortunately, I don't see any functional need for widening the row-limit field in execute messages; how likely is it that someone wants to fetch exactly 3 billion rows? The practical use-cases for nonzero row limits generally involve fetching a bufferload worth of data at a time, so that the restriction to getting no more than INT_MAX rows at once is several orders of magnitude away from being a problem. The same goes for internal uses of row limits, which makes it questionable whether it's worth changing the width of ExecutorRun's count parameter, which is what I assume you were on about here. But in any case, if we did that we'd not try to reflect it as far as here, because the message format specs can't change. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GENERAL] Insert result does not match record count
On Wed, Jul 24, 2013 at 08:08:32PM +0200, Andres Freund wrote:
On 2013-07-24 13:48:23 -0400, Tom Lane wrote:
Vik Fearing vik.fear...@dalibo.com writes:
Also worth mentioning is bug #7766.
http://www.postgresql.org/message-id/e1tlli5-0007tr...@wrigleys.postgresql.org
Yeah, did you read that whole thread? The real issue here is going to
be whether client-side code falls over on wider-than-32-bit counts.
We can fix the backend and be pretty sure that we've found all the
relevant places inside it, but we'll just be exporting the issue.
I did look at libpq and noted that it doesn't seem to have any internal
problem, because it returns the count to callers as a string (!).
But what do you think are the odds that callers are using code that
won't overflow? I'd bet on finding atoi() or suchlike in a lot of
callers. Even if they thought to use strtoul(), unsigned long is
not necessarily 64 bits wide.
Application code that relies on the values already has problems though
since the returned values are pretty bogus now. Including the fact that
it can return 0 as the number of modified rows which is checked for more
frequently than the actual number IME...
So I think client code that uses simplistic stuff like atoi isn't worse
off afterwards since the values will be about as bogus. I am more
worried about code that does range checks like java's string conversion
routines...
I think fixing this for 9.4 is fine, but due to the compat issues I
think it's to late for 9.3.
Where are we on this? There was a posted patch, attached, but Vik
Fearing said it was insufficent and he was working on a new one:
http://www.postgresql.org/message-id/51eff67a.7020...@dalibo.com
--
Bruce Momjian br...@momjian.ushttp://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
*** a/src/backend/commands/createas.c
--- b/src/backend/commands/createas.c
***
*** 172,178 ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString,
/* save the rowcount if we're given a completionTag to fill */
if (completionTag)
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! SELECT %u, queryDesc-estate-es_processed);
/* and clean up */
ExecutorFinish(queryDesc);
--- 172,178
/* save the rowcount if we're given a completionTag to fill */
if (completionTag)
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! SELECT UINT64_FORMAT, queryDesc-estate-es_processed);
/* and clean up */
ExecutorFinish(queryDesc);
*** a/src/backend/tcop/pquery.c
--- b/src/backend/tcop/pquery.c
***
*** 195,201 ProcessQuery(PlannedStmt *plan,
{
case CMD_SELECT:
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! SELECT %u, queryDesc-estate-es_processed);
break;
case CMD_INSERT:
if (queryDesc-estate-es_processed == 1)
--- 195,201
{
case CMD_SELECT:
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! SELECT UINT64_FORMAT, queryDesc-estate-es_processed);
break;
case CMD_INSERT:
if (queryDesc-estate-es_processed == 1)
***
*** 203,217 ProcessQuery(PlannedStmt *plan,
else
lastOid = InvalidOid;
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! INSERT %u %u, lastOid, queryDesc-estate-es_processed);
break;
case CMD_UPDATE:
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! UPDATE %u, queryDesc-estate-es_processed);
break;
case CMD_DELETE:
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! DELETE %u, queryDesc-estate-es_processed);
break;
default:
strcpy(completionTag, ???);
--- 203,217
else
lastOid = InvalidOid;
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! INSERT %u UINT64_FORMAT, lastOid, queryDesc-estate-es_processed);
break;
case CMD_UPDATE:
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! UPDATE UINT64_FORMAT, queryDesc-estate-es_processed);
break;
case CMD_DELETE:
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! DELETE UINT64_FORMAT, queryDesc-estate-es_processed);
break;
default:
strcpy(completionTag, ???);
*** a/src/include/nodes/execnodes.h
--- b/src/include/nodes/execnodes.h
***
*** 375,381 typedef struct EState
List *es_rowMarks; /* List of ExecRowMarks */
! uint32 es_processed; /* # of tuples processed */
Oid es_lastoid; /* last oid processed (by INSERT) */
int es_top_eflags; /* eflags passed to ExecutorStart */
--- 375,381
List *es_rowMarks; /* List of ExecRowMarks */
! uint64 es_processed; /* # of tuples processed */
Oid es_lastoid; /* last oid processed (by INSERT) */
int es_top_eflags; /* eflags passed to ExecutorStart */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To
Re: [HACKERS] [GENERAL] Insert result does not match record count
On 01/31/2014 06:19 PM, Bruce Momjian wrote: On Wed, Jul 24, 2013 at 08:08:32PM +0200, Andres Freund wrote: On 2013-07-24 13:48:23 -0400, Tom Lane wrote: Vik Fearing vik.fear...@dalibo.com writes: Also worth mentioning is bug #7766. http://www.postgresql.org/message-id/e1tlli5-0007tr...@wrigleys.postgresql.org Yeah, did you read that whole thread? The real issue here is going to be whether client-side code falls over on wider-than-32-bit counts. We can fix the backend and be pretty sure that we've found all the relevant places inside it, but we'll just be exporting the issue. I did look at libpq and noted that it doesn't seem to have any internal problem, because it returns the count to callers as a string (!). But what do you think are the odds that callers are using code that won't overflow? I'd bet on finding atoi() or suchlike in a lot of callers. Even if they thought to use strtoul(), unsigned long is not necessarily 64 bits wide. Application code that relies on the values already has problems though since the returned values are pretty bogus now. Including the fact that it can return 0 as the number of modified rows which is checked for more frequently than the actual number IME... So I think client code that uses simplistic stuff like atoi isn't worse off afterwards since the values will be about as bogus. I am more worried about code that does range checks like java's string conversion routines... I think fixing this for 9.4 is fine, but due to the compat issues I think it's to late for 9.3. Where are we on this? There was a posted patch, attached, but Vik Fearing said it was insufficent and he was working on a new one: http://www.postgresql.org/message-id/51eff67a.7020...@dalibo.com Unfortunately, I gave up on it as being over my head when I noticed I was changing the protocol itself. I should have notified the list so someone else could have taken over. -- Vik -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GENERAL] Insert result does not match record count
On Fri, Jan 31, 2014 at 06:34:27PM +0100, Vik Fearing wrote: Application code that relies on the values already has problems though since the returned values are pretty bogus now. Including the fact that it can return 0 as the number of modified rows which is checked for more frequently than the actual number IME... So I think client code that uses simplistic stuff like atoi isn't worse off afterwards since the values will be about as bogus. I am more worried about code that does range checks like java's string conversion routines... I think fixing this for 9.4 is fine, but due to the compat issues I think it's to late for 9.3. Where are we on this? There was a posted patch, attached, but Vik Fearing said it was insufficent and he was working on a new one: http://www.postgresql.org/message-id/51eff67a.7020...@dalibo.com Unfortunately, I gave up on it as being over my head when I noticed I was changing the protocol itself. I should have notified the list so someone else could have taken over. OK, so that brings up a good question. Can we change the protocol for this without causing major breakage? Tom seems to indicate that it can be done for 9.4, but I thought protocol breakage was a major issue. Are we really changing the wire protocol here, or just the type of string we can pass back to the interface? I know the libpq API we give to clients is a string so it is OK. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GENERAL] Insert result does not match record count
Bruce Momjian br...@momjian.us writes: On Fri, Jan 31, 2014 at 06:34:27PM +0100, Vik Fearing wrote: Unfortunately, I gave up on it as being over my head when I noticed I was changing the protocol itself. I should have notified the list so someone else could have taken over. OK, so that brings up a good question. Can we change the protocol for this without causing major breakage? Tom seems to indicate that it can be done for 9.4, but I thought protocol breakage was a major issue. Are we really changing the wire protocol here, or just the type of string we can pass back to the interface? What I said about it upthread was this is effectively a protocol change, albeit a pretty minor one, so I can't see back-patching it. The discussion in bug #7766 shows that some client-side code is likely to need fixing, and that such fixing might actually be nontrivial for them. So changing this in a minor release is clearly a bad idea. But I don't have a problem with widening the counters in a major release where we can document it as a potential compatibility issue. I took a quick look and noted that CMDSTATUS_LEN and COMPLETION_TAG_BUFSIZE are set to 64, and have been for quite a long time, so command status string buffer sizes should not be a problem. I think we probably just need to widen es_processed and touch related code. Not sure what else Vik saw that needed doing. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GENERAL] Insert result does not match record count
On Fri, Jan 31, 2014 at 04:38:21PM -0500, Tom Lane wrote: Bruce Momjian br...@momjian.us writes: On Fri, Jan 31, 2014 at 06:34:27PM +0100, Vik Fearing wrote: Unfortunately, I gave up on it as being over my head when I noticed I was changing the protocol itself. I should have notified the list so someone else could have taken over. OK, so that brings up a good question. Can we change the protocol for this without causing major breakage? Tom seems to indicate that it can be done for 9.4, but I thought protocol breakage was a major issue. Are we really changing the wire protocol here, or just the type of string we can pass back to the interface? What I said about it upthread was this is effectively a protocol change, albeit a pretty minor one, so I can't see back-patching it. The discussion in bug #7766 shows that some client-side code is likely to need fixing, and that such fixing might actually be nontrivial for them. So changing this in a minor release is clearly a bad idea. But I don't have a problem with widening the counters in a major release where we can document it as a potential compatibility issue. I took a quick look and noted that CMDSTATUS_LEN and COMPLETION_TAG_BUFSIZE are set to 64, and have been for quite a long time, so command status string buffer sizes should not be a problem. I think we probably just need to widen es_processed and touch related code. Not sure what else Vik saw that needed doing. OK, thanks for the feedback. I understand now. The contents of the string will potentially have a larger integer, but the byte length of the string in the wire protocol doesn't change. Let's wait for Vik to reply and I think we can move forward. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GENERAL] Insert result does not match record count
On 01/31/2014 10:56 PM, Bruce Momjian wrote: On Fri, Jan 31, 2014 at 04:38:21PM -0500, Tom Lane wrote: Bruce Momjian br...@momjian.us writes: On Fri, Jan 31, 2014 at 06:34:27PM +0100, Vik Fearing wrote: Unfortunately, I gave up on it as being over my head when I noticed I was changing the protocol itself. I should have notified the list so someone else could have taken over. OK, so that brings up a good question. Can we change the protocol for this without causing major breakage? Tom seems to indicate that it can be done for 9.4, but I thought protocol breakage was a major issue. Are we really changing the wire protocol here, or just the type of string we can pass back to the interface? What I said about it upthread was this is effectively a protocol change, albeit a pretty minor one, so I can't see back-patching it. The discussion in bug #7766 shows that some client-side code is likely to need fixing, and that such fixing might actually be nontrivial for them. So changing this in a minor release is clearly a bad idea. But I don't have a problem with widening the counters in a major release where we can document it as a potential compatibility issue. I took a quick look and noted that CMDSTATUS_LEN and COMPLETION_TAG_BUFSIZE are set to 64, and have been for quite a long time, so command status string buffer sizes should not be a problem. I think we probably just need to widen es_processed and touch related code. Yes. Not sure what else Vik saw that needed doing. Quite a lot, actually. It seemed to me at the time to be a pretty big rabbit hole. OK, thanks for the feedback. I understand now. The contents of the string will potentially have a larger integer, but the byte length of the string in the wire protocol doesn't change. Let's wait for Vik to reply and I think we can move forward. Unfortunately, I just did some cleanup last week and removed that branch. Had I waited a bit more I still would have had all the work I had done. I'll see how quickly I can redo it to get to the part where I got scared of what I was doing. It will have to wait until next week though; I am currently at FOSDEM. -- Vik -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GENERAL] Insert result does not match record count
On Sat, Feb 1, 2014 at 02:25:16AM +0100, Vik Fearing wrote: OK, thanks for the feedback. I understand now. The contents of the string will potentially have a larger integer, but the byte length of the string in the wire protocol doesn't change. Let's wait for Vik to reply and I think we can move forward. Unfortunately, I just did some cleanup last week and removed that branch. Had I waited a bit more I still would have had all the work I had done. I'll see how quickly I can redo it to get to the part where I got scared of what I was doing. It will have to wait until next week though; I am currently at FOSDEM. OK, thanks. I thought it only required passing the int64 around until it got into the string passed to the client. The original patch is in the email archives if you want it. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GENERAL] Insert result does not match record count
On 07/22/2013 06:20 PM, Jeff Janes wrote:
On Fri, Jul 19, 2013 at 3:20 PM, Natalie Wenz nataliew...@ebureau.com wrote:
Hi all,
I am moving some data from one table to another in 9.2.4, and keep seeing
this strange scenario:
insert into newtable select data from oldtable where proc_date = x and
proc_date y;
INSERT 0 78551642
select count(*) from newtable where proc_date = x and proc_date y;
count
---
4373518938
It looks to me like the status report is 32 bits and overflowed.
4,373,518,938 - 2^32 = 78,551,642
Attached is a small patch that should fix the problem.
Vik
*** a/src/backend/commands/createas.c
--- b/src/backend/commands/createas.c
***
*** 172,178 ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString,
/* save the rowcount if we're given a completionTag to fill */
if (completionTag)
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! SELECT %u, queryDesc-estate-es_processed);
/* and clean up */
ExecutorFinish(queryDesc);
--- 172,178
/* save the rowcount if we're given a completionTag to fill */
if (completionTag)
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! SELECT UINT64_FORMAT, queryDesc-estate-es_processed);
/* and clean up */
ExecutorFinish(queryDesc);
*** a/src/backend/tcop/pquery.c
--- b/src/backend/tcop/pquery.c
***
*** 195,201 ProcessQuery(PlannedStmt *plan,
{
case CMD_SELECT:
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! SELECT %u, queryDesc-estate-es_processed);
break;
case CMD_INSERT:
if (queryDesc-estate-es_processed == 1)
--- 195,201
{
case CMD_SELECT:
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! SELECT UINT64_FORMAT, queryDesc-estate-es_processed);
break;
case CMD_INSERT:
if (queryDesc-estate-es_processed == 1)
***
*** 203,217 ProcessQuery(PlannedStmt *plan,
else
lastOid = InvalidOid;
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! INSERT %u %u, lastOid, queryDesc-estate-es_processed);
break;
case CMD_UPDATE:
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! UPDATE %u, queryDesc-estate-es_processed);
break;
case CMD_DELETE:
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! DELETE %u, queryDesc-estate-es_processed);
break;
default:
strcpy(completionTag, ???);
--- 203,217
else
lastOid = InvalidOid;
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! INSERT %u UINT64_FORMAT, lastOid, queryDesc-estate-es_processed);
break;
case CMD_UPDATE:
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! UPDATE UINT64_FORMAT, queryDesc-estate-es_processed);
break;
case CMD_DELETE:
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
! DELETE UINT64_FORMAT, queryDesc-estate-es_processed);
break;
default:
strcpy(completionTag, ???);
*** a/src/include/nodes/execnodes.h
--- b/src/include/nodes/execnodes.h
***
*** 375,381 typedef struct EState
List *es_rowMarks; /* List of ExecRowMarks */
! uint32 es_processed; /* # of tuples processed */
Oid es_lastoid; /* last oid processed (by INSERT) */
int es_top_eflags; /* eflags passed to ExecutorStart */
--- 375,381
List *es_rowMarks; /* List of ExecRowMarks */
! uint64 es_processed; /* # of tuples processed */
Oid es_lastoid; /* last oid processed (by INSERT) */
int es_top_eflags; /* eflags passed to ExecutorStart */
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GENERAL] Insert result does not match record count
On 07/24/2013 04:04 PM, Vik Fearing wrote: On 07/22/2013 06:20 PM, Jeff Janes wrote: On Fri, Jul 19, 2013 at 3:20 PM, Natalie Wenz nataliew...@ebureau.com wrote: Hi all, I am moving some data from one table to another in 9.2.4, and keep seeing this strange scenario: insert into newtable select data from oldtable where proc_date = x and proc_date y; INSERT 0 78551642 select count(*) from newtable where proc_date = x and proc_date y; count --- 4373518938 It looks to me like the status report is 32 bits and overflowed. 4,373,518,938 - 2^32 = 78,551,642 Attached is a small patch that should fix the problem. It would seem this was not as thorough as it should have been and there's a lot more to do. I'm working on a better patch. Also worth mentioning is bug #7766. http://www.postgresql.org/message-id/e1tlli5-0007tr...@wrigleys.postgresql.org Vik -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GENERAL] Insert result does not match record count
Vik Fearing vik.fear...@dalibo.com writes: Also worth mentioning is bug #7766. http://www.postgresql.org/message-id/e1tlli5-0007tr...@wrigleys.postgresql.org Yeah, did you read that whole thread? The real issue here is going to be whether client-side code falls over on wider-than-32-bit counts. We can fix the backend and be pretty sure that we've found all the relevant places inside it, but we'll just be exporting the issue. I did look at libpq and noted that it doesn't seem to have any internal problem, because it returns the count to callers as a string (!). But what do you think are the odds that callers are using code that won't overflow? I'd bet on finding atoi() or suchlike in a lot of callers. Even if they thought to use strtoul(), unsigned long is not necessarily 64 bits wide. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GENERAL] Insert result does not match record count
On 2013-07-24 13:48:23 -0400, Tom Lane wrote: Vik Fearing vik.fear...@dalibo.com writes: Also worth mentioning is bug #7766. http://www.postgresql.org/message-id/e1tlli5-0007tr...@wrigleys.postgresql.org Yeah, did you read that whole thread? The real issue here is going to be whether client-side code falls over on wider-than-32-bit counts. We can fix the backend and be pretty sure that we've found all the relevant places inside it, but we'll just be exporting the issue. I did look at libpq and noted that it doesn't seem to have any internal problem, because it returns the count to callers as a string (!). But what do you think are the odds that callers are using code that won't overflow? I'd bet on finding atoi() or suchlike in a lot of callers. Even if they thought to use strtoul(), unsigned long is not necessarily 64 bits wide. Application code that relies on the values already has problems though since the returned values are pretty bogus now. Including the fact that it can return 0 as the number of modified rows which is checked for more frequently than the actual number IME... So I think client code that uses simplistic stuff like atoi isn't worse off afterwards since the values will be about as bogus. I am more worried about code that does range checks like java's string conversion routines... I think fixing this for 9.4 is fine, but due to the compat issues I think it's to late for 9.3. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [GENERAL] Insert result does not match record count
Andres Freund and...@2ndquadrant.com writes: I think fixing this for 9.4 is fine, but due to the compat issues I think it's to late for 9.3. Agreed -- this is effectively a protocol change, albeit a pretty minor one, so I can't see back-patching it. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
