subject:"\[HACKERS\] pl\/pgSQL, get diagnostics and big data"

Re: [HACKERS] pl/pgSQL, get diagnostics and big data

2016-03-12 Thread Tom Lane

I wrote:
> 2. As I was just complaining to -hackers, plpython plperl and pltcl
> all now contain attempts to pass uint64 values (from SPI_processed)
> into language-specific functions.  We need to figure out whether
> that will overflow and whether it's worth doing something about.

I fixed this along the lines suggested by Salvador Fandino, viz convert
to the language's equivalent of "double" if it wouldn't fit in int
or long respectively.  Tcl turns out to have a native int64 type
("WideInt") so that was slightly less messy.

I've pushed this so we can get some buildfarm testing, but it wouldn't
be a bad idea for someone to review the committed patch.  Chasing all
the dependencies was tedious and I'm still not real sure I found them
all.

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] pl/pgSQL, get diagnostics and big data

2016-03-12 Thread Tom Lane

Petr Jelinek  writes:
> On 12/03/16 04:30, Tom Lane wrote:
>> 1. I found two places (marked XXX in this patch) that are using strtoul()
>> to parse a tuple count back out of a command tag.  That won't do anymore.
>> pg_stat_statements has a messy hack for the same problem (look for
>> HAVE_STRTOULL), which is probably what we want to do, but not by
>> copy-and-pasting #ifdef HAVE_STRTOULL into multiple places.  I'd be
>> inclined to provide a utility function "pg_strtouint64" or some such
>> to encapsulate that.  (numutils.c might be a good place for it.)

> Hmm, I thought that solution is not really portable for 64bit numbers 
> and only is allowed in pg_stat_statements because worst case it will cut 
> the number to 32bit int and misreport but won't break anything there. 
> For example windows IIRC need _strtoui64 for this.

OK, we can use _strtoui64() on Windows.

> I once wrote (well copy-pasted from BDS + some #define wrappers) 
> portable version of that, see the 0004 and bottom of 0003 in 
> http://www.postgresql.org/message-id/557d9ded.2080...@2ndquadrant.com (I 
> think at minimum what the 0003 does in c.h is needed).

Meh.  That seems like pretty substantial overkill.  Given that we assume
platforms have working 64-bit support these days, what's the probability
that they don't have an appropriate strtoXXX function?  Or on one that
doesn't, that anyone will ever try to run >4G-tuple results through the
relevant code paths?

For the moment I'm just going to do this:

uint64
pg_strtouint64(const char *str, char **endptr, int base)
{
#ifdef WIN32
return _strtoui64(str, endptr, base);
#elif defined(HAVE_STRTOULL) && SIZEOF_LONG < 8
return strtoull(str, endptr, base);
#else
return strtoul(str, endptr, base);
#endif
}

If there ever seems to be any practical value in improving it, we
can do that later.

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] pl/pgSQL, get diagnostics and big data

2016-03-11 Thread Petr Jelinek


On 12/03/16 04:30, Tom Lane wrote:


1. I found two places (marked XXX in this patch) that are using strtoul()
to parse a tuple count back out of a command tag.  That won't do anymore.
pg_stat_statements has a messy hack for the same problem (look for
HAVE_STRTOULL), which is probably what we want to do, but not by
copy-and-pasting #ifdef HAVE_STRTOULL into multiple places.  I'd be
inclined to provide a utility function "pg_strtouint64" or some such
to encapsulate that.  (numutils.c might be a good place for it.)



Hmm, I thought that solution is not really portable for 64bit numbers 
and only is allowed in pg_stat_statements because worst case it will cut 
the number to 32bit int and misreport but won't break anything there. 
For example windows IIRC need _strtoui64 for this.


I once wrote (well copy-pasted from BDS + some #define wrappers) 
portable version of that, see the 0004 and bottom of 0003 in 
http://www.postgresql.org/message-id/557d9ded.2080...@2ndquadrant.com (I 
think at minimum what the 0003 does in c.h is needed).


--
  Petr Jelinek  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] pl/pgSQL, get diagnostics and big data

2016-03-11 Thread Tom Lane

I wrote:
> I'll take it from here, unless I find bigger issues.

Hmm ... so the more I pulled on this string, the more stuff I found.
The attached updated patch fixes several additional significant
areas:

* call_cntr and max_calls in FuncCallContext are now uint64

* Result widths for PortalRunFetch and allied routines are now uint64

* count arguments to ExecutorRun and related routines are now uint64

* Maximum number of tuples in a SPITupleTable is now uint64

* Management of portal (cursor) positions fixed.  I promoted portalPos to
uint64, and got rid of posOverflow, which seems not especially necessary
if we're using 64-bit counters.

* A whole lot of places that were comparing integer-width variables
to SPI_processed, and would therefore go into infinite loops with
result sizes above 2G tuples, now use uint64 counters instead.

However, I drew the line at changing FetchStmt.howMany, which means that
the count-limit inputs to PortalRunFetch et al didn't change; they're
still signed long.  Changing that would require messing with the lexer
which I did not feel like doing.  So the maximum offset/position in a
FETCH or MOVE is (still) platform-dependent.  I'm not sure that this is
worth changing, as you will get a clean syntax error if you try to exceed
the limit.

BTW, if anyone thinks the above changes represent too much API churn,
speak up.  This is definitely going to cause some pain for extensions.

There's a fair amount of work yet to do:

1. I found two places (marked XXX in this patch) that are using strtoul()
to parse a tuple count back out of a command tag.  That won't do anymore.
pg_stat_statements has a messy hack for the same problem (look for
HAVE_STRTOULL), which is probably what we want to do, but not by
copy-and-pasting #ifdef HAVE_STRTOULL into multiple places.  I'd be
inclined to provide a utility function "pg_strtouint64" or some such
to encapsulate that.  (numutils.c might be a good place for it.)

2. As I was just complaining to -hackers, plpython plperl and pltcl
all now contain attempts to pass uint64 values (from SPI_processed)
into language-specific functions.  We need to figure out whether
that will overflow and whether it's worth doing something about.

3. This patch still needs a lot of review as I may have missed
something.

So I'm bouncing this back to Waiting on Author.

regards, tom lane

diff --git a/contrib/auto_explain/auto_explain.c b/contrib/auto_explain/auto_explain.c
index 76d1831..6708d81 100644
*** a/contrib/auto_explain/auto_explain.c
--- b/contrib/auto_explain/auto_explain.c
*** void		_PG_fini(void);
*** 61,67 
  static void explain_ExecutorStart(QueryDesc *queryDesc, int eflags);
  static void explain_ExecutorRun(QueryDesc *queryDesc,
  	ScanDirection direction,
! 	long count);
  static void explain_ExecutorFinish(QueryDesc *queryDesc);
  static void explain_ExecutorEnd(QueryDesc *queryDesc);
  
--- 61,67 
  static void explain_ExecutorStart(QueryDesc *queryDesc, int eflags);
  static void explain_ExecutorRun(QueryDesc *queryDesc,
  	ScanDirection direction,
! 	uint64 count);
  static void explain_ExecutorFinish(QueryDesc *queryDesc);
  static void explain_ExecutorEnd(QueryDesc *queryDesc);
  
*** explain_ExecutorStart(QueryDesc *queryDe
*** 257,263 
   * ExecutorRun hook: all we need do is track nesting depth
   */
  static void
! explain_ExecutorRun(QueryDesc *queryDesc, ScanDirection direction, long count)
  {
  	nesting_level++;
  	PG_TRY();
--- 257,263 
   * ExecutorRun hook: all we need do is track nesting depth
   */
  static void
! explain_ExecutorRun(QueryDesc *queryDesc, ScanDirection direction, uint64 count)
  {
  	nesting_level++;
  	PG_TRY();
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c b/contrib/pg_stat_statements/pg_stat_statements.c
index 9ce60e6..29e12cd 100644
*** a/contrib/pg_stat_statements/pg_stat_statements.c
--- b/contrib/pg_stat_statements/pg_stat_statements.c
*** static void pgss_post_parse_analyze(Pars
*** 289,295 
  static void pgss_ExecutorStart(QueryDesc *queryDesc, int eflags);
  static void pgss_ExecutorRun(QueryDesc *queryDesc,
   ScanDirection direction,
!  long count);
  static void pgss_ExecutorFinish(QueryDesc *queryDesc);
  static void pgss_ExecutorEnd(QueryDesc *queryDesc);
  static void pgss_ProcessUtility(Node *parsetree, const char *queryString,
--- 289,295 
  static void pgss_ExecutorStart(QueryDesc *queryDesc, int eflags);
  static void pgss_ExecutorRun(QueryDesc *queryDesc,
   ScanDirection direction,
!  uint64 count);
  static void pgss_ExecutorFinish(QueryDesc *queryDesc);
  static void pgss_ExecutorEnd(QueryDesc *queryDesc);
  static void pgss_ProcessUtility(Node *parsetree, const char *queryString,
*** pgss_ExecutorStart(QueryDesc *queryDesc,
*** 866,872 
   * ExecutorRun hook: all we need do is track nesting depth
   */
  static void
! pgss_ExecutorRun(QueryDesc

Re: [HACKERS] pl/pgSQL, get diagnostics and big data

2016-03-11 Thread Tom Lane

"Andreas 'ads' Scherbaum"  writes:
> On 09.02.2016 20:32, Christian Ullrich wrote:
>> To fix this, I think it will be enough to change the format strings to
>> use "%zu" instead of "%lu".

> Attached is a new version of the patch, with %lu replaced by %zu.

Nonono ... that just moves the portability problem to a different set of
platforms.  "%z" means size_t, and sizeof(size_t) is not any more fixed
than sizeof(long).  The right thing to use is UINT64_FORMAT.

+  /* Int64GetDatum() instead of 
UInt64GetDatum(),
+ because there is no UInt64GetDatum() */
+  Int64GetDatum(estate->eval_processed),

Although in practice you'd get the same conversion anyway, I'm thinking
it's time to fix that omission rather than just averting our eyes.  The
use of int64 data isn't decreasing as time goes on.

These are small enough changes that there's no need for a new patch
submission.  I'll take it from here, unless I find bigger issues.

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] pl/pgSQL, get diagnostics and big data

2016-02-14 Thread Christian Ullrich


* Andreas 'ads' Scherbaum wrote:


Attached is a new version of the patch, with %lu replaced by %zu.
I re-ran all the tests, especially the long test with 2^32+x rows, and
it produces the same result as before.


To paraphrase Twain: "Sire, the Board finds this patch perfect in all 
the requirements and qualifications for inclusion into core, and doth 
hold his case open for decision after due examination by his committer."


The Windows issue is corrected, and all regression tests pass on Windows 
and FreeBSD. I can find no further fault with this patch.


Sorry it took so long, my other PostgreSQL issue happened just when I 
was going to test the updated patch.


--
Christian



--
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] pl/pgSQL, get diagnostics and big data

2016-02-10 Thread Andreas 'ads' Scherbaum



Hello,

thanks for reviewing the patch!

On 09.02.2016 20:32, Christian Ullrich wrote:


- Are there portability issues/Will it work on Windows/BSD etc.:

   No, it will not work correctly on Windows when built with MSVC,
   although it may work with MinGW.

   +++ postgresql-9.5.0/src/backend/tcop/pquery.c
   @@ -195,7 +195,7 @@
{
  case CMD_SELECT:
  snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
   - "SELECT %u", queryDesc->estate->es_processed);
   + "SELECT %lu", queryDesc->estate->es_processed);


   %lu formats unsigned long. "long" is problematic in terms of
   portability, because sizeof(long) is different everywhere. It is 32
   bits on Windows and on 32-bit *nix, and 64 bits on 64-bit *nix.

   I added the following line to the INSERT formatting in pquery.c:

 queryDesc->estate->es_processed += 471147114711LL;

   This number is 0x6DB28E70D7; so inserting one row should return
   "INSERT 0 2995679448" (0xB28E70D8):

 postgres=# insert into t1 values (0);
 INSERT 0 2995679448

   To fix this, I think it will be enough to change the format strings to
   use "%zu" instead of "%lu". pg_snprintf() is selected by configure if
   the platform's snprintf() does not support the "z" conversion. I tried
   this, and it appears to work:

 postgres=# insert into t1 values (0);
 INSERT 0 471147114712

   I have looked for other uses of "%lu", and found none that may cause
   the same issue; apparently they are all used with values that clearly
   have 32-bit type; actually, most of them are used to format error
   codes in Windows-specific code.


Attached is a new version of the patch, with %lu replaced by %zu.
I re-ran all the tests, especially the long test with 2^32+x rows, and 
it produces the same result as before.



Regards,

--
Andreas 'ads' Scherbaum
German PostgreSQL User Group
European PostgreSQL User Group - Board of Directors
Volunteer Regional Contact, Germany - PostgreSQL Project


64bit_4.diff.gz
Description: application/gzip

-- 
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] pl/pgSQL, get diagnostics and big data

2016-02-09 Thread Christian Ullrich

Ah, so it turns out I should have used the commitfest tool. My 
apologies; I will send the whole thing through that again. Please 
disregard the earlier message.




--
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] pl/pgSQL, get diagnostics and big data

2016-02-09 Thread Christian Ullrich


* Andreas 'ads' Scherbaum wrote:


one of our customers approached us and complained, that GET DIAGNOSTICS
row_count returns invalid results if the number of rows is > 2^31. It's



Attached patch expands the row_count to 64 bit.

diagnostics=# select testfunc_pg((2^32 + 5)::bigint);
  testfunc_pg
-
   4295017296
(1 row)


This is my first patch review, but I have to get my feet wet at some 
point, and this is a nice, small patch to do that.


Following the checklist from the wiki:

- Is the patch in context format: Yes.

- Does it apply cleanly to master: Yes.

- Does it include reasonable tests, doc patches, etc.: No. While
  it would be nice if it had some, a test that inserts 2^32 rows
  will take a while and can hardly be called reasonable.


The patch is designed to expand the size of the "affected records" count 
in the command tag from 32 to 64 bits.


- Does it do that: Yes.

- Do we want that: Yes, because it is motivated by reports from users
  who have queries like that in real life.

- Do we already have it: No.

- Does it follow SQL spec or community-agreed behavior: This is not
  covered by the SQL standard and there has not, to my knowledge, been
  any discussion on this point on -hackers. It is, however, the
  obvious approach to solving the specific issue.

- Does it include pg_dump support: n/a

- Are there dangers: Existing applications and client libraries must
  support the increased maximum size (up to nine additional digits) and
  maximum value. libpq apparently does not parse the command tag, only
  stores it as a string for retrieval by PQcmdStatus(), so it is not
  affected in terms of parsing the value, and for storage, it uses a
  64-character buffer, which will overflow if the command name part of
  the tag exceeds 32 characters (63 - 19 [row count] - 10 [OID] - 2
  [spaces]). The longest command name I can think of is "REFRESH
  MATERIALIZED VIEW" which, at 25 characters, stays comfortably below
  this limit, and does not include a row count anyway.

- Have all the bases been covered: The patch changes all locations
  where the command tag is formatted, and where the record count is
  retrieved by PL/pgSQL.

- Does the patch follow the coding guidelines: I believe so.

- Are there portability issues/Will it work on Windows/BSD etc.:

  No, it will not work correctly on Windows when built with MSVC,
  although it may work with MinGW.

  +++ postgresql-9.5.0/src/backend/tcop/pquery.c
  @@ -195,7 +195,7 @@
   {
case CMD_SELECT:
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
  -  "SELECT %u", queryDesc->estate->es_processed);
  +  "SELECT %lu", queryDesc->estate->es_processed);


  %lu formats unsigned long. "long" is problematic in terms of
  portability, because sizeof(long) is different everywhere. It is 32
  bits on Windows and on 32-bit *nix, and 64 bits on 64-bit *nix.

  I added the following line to the INSERT formatting in pquery.c:

queryDesc->estate->es_processed += 471147114711LL;

  This number is 0x6DB28E70D7; so inserting one row should return
  "INSERT 0 2995679448" (0xB28E70D8):

postgres=# insert into t1 values (0);
INSERT 0 2995679448

  To fix this, I think it will be enough to change the format strings to
  use "%zu" instead of "%lu". pg_snprintf() is selected by configure if
  the platform's snprintf() does not support the "z" conversion. I tried
  this, and it appears to work:

postgres=# insert into t1 values (0);
INSERT 0 471147114712

  I have looked for other uses of "%lu", and found none that may cause
  the same issue; apparently they are all used with values that clearly
  have 32-bit type; actually, most of them are used to format error
  codes in Windows-specific code.

- Are the comments sufficient and accurate: Yes.

- Does it do what it says, correctly: Yes, except for the Windows thing.

- Does it produce compiler warnings: No. First, pg_snprintf() does not
  use the system implementation, and second, a warning (C4477) for
  this kind of format string mismatch was only added in VS2015, which
  is not officially supported (it works for me).

- Can you make it crash: No. The problematic argument always appears
  last in the sprintf() calls, so the format string issue should not
  be exploitable.

I did not run the regression tests or do the "performance" sections 
after I found the Windows issue. I do not think it will negatively 
affect performance, though.


In all, if replacing four "l"s with "z"s is indeed enough, I think this 
patch is an appropriate solution for solving the underlying issue.


--
Christian



--
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] pl/pgSQL, get diagnostics and big data

2016-02-09 Thread Christian Ullrich

The following review has been posted through the commitfest application:
make installcheck-world:  not tested
Implements feature:   tested, failed
Spec compliant:   not tested
Documentation:not tested

* Andreas 'ads' Scherbaum wrote:

> one of our customers approached us and complained, that GET DIAGNOSTICS
> row_count returns invalid results if the number of rows is > 2^31. It's

> Attached patch expands the row_count to 64 bit.
>
> diagnostics=# select testfunc_pg((2^32 + 5)::bigint);
>   testfunc_pg
> -
>4295017296
> (1 row)

This is my first patch review, but I have to get my feet wet at some point, and 
this is a nice, small patch to do that.

Following the checklist from the wiki:

- Is the patch in context format: Yes.

- Does it apply cleanly to master: Yes.

- Does it include reasonable tests, doc patches, etc.: No. While
  it would be nice if it had some, a test that inserts 2^32 rows
  will take a while and can hardly be called reasonable.


The patch is designed to expand the size of the "affected records" count in the 
command tag from 32 to 64 bits.

- Does it do that: Yes.

- Do we want that: Yes, because it is motivated by reports from users
  who have queries like that in real life.

- Do we already have it: No.

- Does it follow SQL spec or community-agreed behavior: This is not
  covered by the SQL standard and there has not, to my knowledge, been
  any discussion on this point on -hackers. It is, however, the
  obvious approach to solving the specific issue.

- Does it include pg_dump support: n/a

- Are there dangers: Existing applications and client libraries must
  support the increased maximum size (up to nine additional digits) and
  maximum value. libpq apparently does not parse the command tag, only
  stores it as a string for retrieval by PQcmdStatus(), so it is not
  affected in terms of parsing the value, and for storage, it uses a
  64-character buffer, which will overflow if the command name part of
  the tag exceeds 32 characters (63 - 19 [row count] - 10 [OID] - 2
  [spaces]). The longest command name I can think of is "REFRESH
  MATERIALIZED VIEW" which, at 25 characters, stays comfortably below
  this limit, and does not include a row count anyway.

- Have all the bases been covered: The patch changes all locations
  where the command tag is formatted, and where the record count is
  retrieved by PL/pgSQL.

- Does the patch follow the coding guidelines: I believe so.

- Are there portability issues/Will it work on Windows/BSD etc.:

  No, it will not work correctly on Windows when built with MSVC,
  although it may work with MinGW.

  +++ postgresql-9.5.0/src/backend/tcop/pquery.c
  @@ -195,7 +195,7 @@
   {
 case CMD_SELECT:
 snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
  - "SELECT %u", queryDesc->estate->es_processed);
  + "SELECT %lu", queryDesc->estate->es_processed);


  %lu formats unsigned long. "long" is problematic in terms of
  portability, because sizeof(long) is different everywhere. It is 32
  bits on Windows and on 32-bit *nix, and 64 bits on 64-bit *nix.

  I added the following line to the INSERT formatting in pquery.c:

queryDesc->estate->es_processed += 471147114711LL;

  This number is 0x6DB28E70D7; so inserting one row should return
  "INSERT 0 2995679448" (0xB28E70D8):

postgres=# insert into t1 values (0);
INSERT 0 2995679448

  To fix this, I think it will be enough to change the format strings to
  use "%zu" instead of "%lu". pg_snprintf() is selected by configure if
  the platform's snprintf() does not support the "z" conversion. I tried
  this, and it appears to work:

postgres=# insert into t1 values (0);
INSERT 0 471147114712

  I have looked for other uses of "%lu", and found none that may cause
  the same issue; apparently they are all used with values that clearly
  have 32-bit type; actually, most of them are used to format error
  codes in Windows-specific code.

- Are the comments sufficient and accurate: Yes.

- Does it do what it says, correctly: Yes, except for the Windows thing.

- Does it produce compiler warnings: No. First, pg_snprintf() does not
  use the system implementation, and second, a warning (C4477) for
  this kind of format string mismatch was only added in VS2015, which
  is not officially supported (it works for me).

- Can you make it crash: No. The problematic argument always appears
  last in the sprintf() calls, so the format string issue should not
  be exploitable.

I did not run the regression tests or do the "performance" sections after I 
found the Windows issue. I do not think it will negatively affect performance, 
though.

In all, if replacing four "l"s with "z"s is indeed enough, I think this patch 
is an appropriate solution for solving the underlying issue.

The new status of this patch is: Waiting on Author

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your

Re: [HACKERS] pl/pgSQL, get diagnostics and big data

2016-02-01 Thread Andreas 'ads' Scherbaum


On 01.02.2016 21:24, Andreas 'ads' Scherbaum wrote:


Attached patch expands the row_count to 64 bit.


Remembering an issue we had recently with clear text patches, attached 
is a gzipped version as well.



Regards,

--
Andreas 'ads' Scherbaum
German PostgreSQL User Group
European PostgreSQL User Group - Board of Directors
Volunteer Regional Contact, Germany - PostgreSQL Project


64bit_3.diff.gz
Description: application/gzip

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] pl/pgSQL, get diagnostics and big data

2016-02-01 Thread Andreas 'ads' Scherbaum



Hello,

one of our customers approached us and complained, that GET DIAGNOSTICS 
row_count returns invalid results if the number of rows is > 2^31. It's 
a bit complicated to test for this case, so I set up a separate instance 
with this patch, and inserted 2^32+x rows into a table. Internally, 
row_count it's a signed integer, the resulting number is negative:


diagnostics=# select testfunc_pg((2^31 + 5)::bigint);
 testfunc_pg
-
 -2147433648
(1 row)


Going over 2^32 wraps around:

diagnostics=# select testfunc_pg((2^32 + 5)::bigint);
 testfunc_pg
-
   5
(1 row)



Attached patch expands the row_count to 64 bit.

diagnostics=# select testfunc_pg((2^32 + 5)::bigint);
 testfunc_pg
-
  4295017296
(1 row)


I hope, I covered all the places which count the result set.


Regards,

--
Andreas 'ads' Scherbaum
German PostgreSQL User Group
European PostgreSQL User Group - Board of Directors
Volunteer Regional Contact, Germany - PostgreSQL Project
diff -ru postgresql-9.5.0.orig/src/backend/executor/spi.c postgresql-9.5.0/src/backend/executor/spi.c
--- postgresql-9.5.0.orig/src/backend/executor/spi.c	2016-01-04 22:29:34.0 +0100
+++ postgresql-9.5.0/src/backend/executor/spi.c	2016-01-27 01:30:06.099132294 +0100
@@ -36,7 +36,7 @@
 #include "utils/typcache.h"
 
 
-uint32		SPI_processed = 0;
+uint64		SPI_processed = 0;
 Oid			SPI_lastoid = InvalidOid;
 SPITupleTable *SPI_tuptable = NULL;
 int			SPI_result;
@@ -1994,7 +1994,7 @@
   bool read_only, bool fire_triggers, long tcount)
 {
 	int			my_res = 0;
-	uint32		my_processed = 0;
+	uint64		my_processed = 0;
 	Oid			my_lastoid = InvalidOid;
 	SPITupleTable *my_tuptable = NULL;
 	int			res = 0;
@@ -2562,7 +2562,7 @@
 static bool
 _SPI_checktuples(void)
 {
-	uint32		processed = _SPI_current->processed;
+	uint64		processed = _SPI_current->processed;
 	SPITupleTable *tuptable = _SPI_current->tuptable;
 	bool		failed = false;
 
diff -ru postgresql-9.5.0.orig/src/backend/tcop/pquery.c postgresql-9.5.0/src/backend/tcop/pquery.c
--- postgresql-9.5.0.orig/src/backend/tcop/pquery.c	2016-01-04 22:29:34.0 +0100
+++ postgresql-9.5.0/src/backend/tcop/pquery.c	2016-01-30 12:11:56.573841810 +0100
@@ -195,7 +195,7 @@
 		{
 			case CMD_SELECT:
 snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
-		 "SELECT %u", queryDesc->estate->es_processed);
+		 "SELECT %lu", queryDesc->estate->es_processed);
 break;
 			case CMD_INSERT:
 if (queryDesc->estate->es_processed == 1)
@@ -203,15 +203,15 @@
 else
 	lastOid = InvalidOid;
 snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
-   "INSERT %u %u", lastOid, queryDesc->estate->es_processed);
+   "INSERT %u %lu", lastOid, queryDesc->estate->es_processed);
 break;
 			case CMD_UPDATE:
 snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
-		 "UPDATE %u", queryDesc->estate->es_processed);
+		 "UPDATE %lu", queryDesc->estate->es_processed);
 break;
 			case CMD_DELETE:
 snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
-		 "DELETE %u", queryDesc->estate->es_processed);
+		 "DELETE %lu", queryDesc->estate->es_processed);
 break;
 			default:
 strcpy(completionTag, "???");
@@ -892,7 +892,7 @@
 {
 	QueryDesc  *queryDesc;
 	ScanDirection direction;
-	uint32		nprocessed;
+	uint64		nprocessed;
 
 	/*
 	 * NB: queryDesc will be NULL if we are fetching from a held cursor or a
diff -ru postgresql-9.5.0.orig/src/include/executor/spi.h postgresql-9.5.0/src/include/executor/spi.h
--- postgresql-9.5.0.orig/src/include/executor/spi.h	2016-01-04 22:29:34.0 +0100
+++ postgresql-9.5.0/src/include/executor/spi.h	2016-01-27 01:34:46.388245129 +0100
@@ -59,7 +59,7 @@
 #define SPI_OK_UPDATE_RETURNING 13
 #define SPI_OK_REWRITTEN		14
 
-extern PGDLLIMPORT uint32 SPI_processed;
+extern PGDLLIMPORT uint64 SPI_processed;
 extern PGDLLIMPORT Oid SPI_lastoid;
 extern PGDLLIMPORT SPITupleTable *SPI_tuptable;
 extern PGDLLIMPORT int SPI_result;
diff -ru postgresql-9.5.0.orig/src/include/executor/spi_priv.h postgresql-9.5.0/src/include/executor/spi_priv.h
--- postgresql-9.5.0.orig/src/include/executor/spi_priv.h	2016-01-04 22:29:34.0 +0100
+++ postgresql-9.5.0/src/include/executor/spi_priv.h	2016-01-27 01:34:55.220056918 +0100
@@ -21,7 +21,7 @@
 typedef struct
 {
 	/* current results */
-	uint32		processed;		/* by Executor */
+	uint64		processed;		/* by Executor */
 	Oid			lastoid;
 	SPITupleTable *tuptable;	/* tuptable currently being built */
 
diff -ru postgresql-9.5.0.orig/src/include/nodes/execnodes.h postgresql-9.5.0/src/include/nodes/execnodes.h
--- postgresql-9.5.0.orig/src/include/nodes/execnodes.h	2016-01-04 22:29:34.0 +0100
+++ postgresql-9.5.0/src/include/nodes/execnodes.h	2016-01-27 01:32:04.711625720 +0100
@@ -387,7 +387,7 @@
 
 	List	   *es_rowMarks;	/* List of ExecRowMarks */
 
-	uint32		es_processed;	/* # of tuples processed */
+	uint64		es_processed;	/* # of tuples

Re: [HACKERS] pl/pgSQL, get diagnostics and big data

Re: [HACKERS] pl/pgSQL, get diagnostics and big data

Re: [HACKERS] pl/pgSQL, get diagnostics and big data

Re: [HACKERS] pl/pgSQL, get diagnostics and big data

Re: [HACKERS] pl/pgSQL, get diagnostics and big data

Re: [HACKERS] pl/pgSQL, get diagnostics and big data

Re: [HACKERS] pl/pgSQL, get diagnostics and big data

Re: [HACKERS] pl/pgSQL, get diagnostics and big data

Re: [HACKERS] pl/pgSQL, get diagnostics and big data

Re: [HACKERS] pl/pgSQL, get diagnostics and big data

Re: [HACKERS] pl/pgSQL, get diagnostics and big data

[HACKERS] pl/pgSQL, get diagnostics and big data

12 matches

Site Navigation

Mail list logo

Footer information