[PATCH] Use correct types and limits for PL/Perl SPI query results

Started by Dagfinn Ilmari Mannsåkerabout 10 years ago4 messageshackers
Jump to latest

Hi hackers,

Commit 23a27b039d94ba359286694831eafe03cd970eef changed the type of
numbers-of-tuples-processed counters to uint64 and adjusted various PLs
to cope with this. I noticed the PL/Perl changes did not take full
advantage of what Perl is capable of handling, so here's a patch that
improves that.

1) Perl's integers are at least pointer-sized and either signed or
unsigned, so can potentially hold up to 2⁶⁴-1. Floating point numbers
can also be larger than double (up to 128bit), allowing for exact
representation of integers beyond 2⁵³. Hence, adjust the setting of
the "processed" hash item to use UV_MAX for the limit and (NV) or
(UV) for the casts.

2) Perl 5.20 and later use SSize_t for array indices, so can cope with
up to SSize_t_max items in an array (if you have the memory).

3) To be able to actually return such result sets from sp_exec_query(),
I had to change the repalloc() in spi_printtup() to repalloc_huge().

--
"A disappointingly low fraction of the human race is,
at any given time, on fire." - Stig Sandbeck Mathisen

Attachments:

0001-Use-correct-types-and-limits-for-PL-Perl-SPI-query-r.patchtext/x-diffDownload+12-11
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Dagfinn Ilmari Mannsåker (#1)
Re: [PATCH] Use correct types and limits for PL/Perl SPI query results

ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:

1) Perl's integers are at least pointer-sized and either signed or
unsigned, so can potentially hold up to 2⁶⁴-1. Floating point numbers
can also be larger than double (up to 128bit), allowing for exact
representation of integers beyond 2⁵³. Hence, adjust the setting of
the "processed" hash item to use UV_MAX for the limit and (NV) or
(UV) for the casts.

I thought about using UV where feasible, but it was not clear to me
whether unsigned numbers behave semantically differently from signed ones
in Perl. If they do, the change you suggest would create a small semantic
change in the behavior of code using spi_exec_query. I'm not sure it's
worth taking any risk of that, considering we already discourage people
from using spi_exec_query for large results.

2) Perl 5.20 and later use SSize_t for array indices, so can cope with
up to SSize_t_max items in an array (if you have the memory).

I don't much like having such hard-wired knowledge about Perl versions
in our code. Is there a way to identify this change other than
#if PERL_BCDVERSION >= 0x5019004 ?

3) To be able to actually return such result sets from sp_exec_query(),
I had to change the repalloc() in spi_printtup() to repalloc_huge().

Hmm, good point. I wonder if there are any hazards to doing that.

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

In reply to: Tom Lane (#2)
Re: [PATCH] Use correct types and limits for PL/Perl SPI query results

Tom Lane <tgl@sss.pgh.pa.us> writes:

ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:

1) Perl's integers are at least pointer-sized and either signed or
unsigned, so can potentially hold up to 2⁶⁴-1. Floating point numbers
can also be larger than double (up to 128bit), allowing for exact
representation of integers beyond 2⁵³. Hence, adjust the setting of
the "processed" hash item to use UV_MAX for the limit and (NV) or
(UV) for the casts.

I thought about using UV where feasible, but it was not clear to me
whether unsigned numbers behave semantically differently from signed ones
in Perl. If they do, the change you suggest would create a small semantic
change in the behavior of code using spi_exec_query. I'm not sure it's
worth taking any risk of that, considering we already discourage people
from using spi_exec_query for large results.

IVs and UVs are semantically equivalent, and Perl will automatically
convert between them (and NVs) as necessary, i.e. when crossing
IV_MAX/UV_MAX/IV_MIN.

2) Perl 5.20 and later use SSize_t for array indices, so can cope with
up to SSize_t_max items in an array (if you have the memory).

I don't much like having such hard-wired knowledge about Perl versions
in our code. Is there a way to identify this change other than
#if PERL_BCDVERSION >= 0x5019004 ?

There isn't, unfortunately. I could add e.g. AVidx_t and _MAX defines,
but that wouldn't be available until 5.26 (May 2017) at the earliest,
since full code freeze for May's 5.24 is next week.

3) To be able to actually return such result sets from sp_exec_query(),
I had to change the repalloc() in spi_printtup() to repalloc_huge().

Hmm, good point. I wonder if there are any hazards to doing that.

One hazard would be that people not heeding the warning in
spi_exec_query will get a visit from the OOM killer (or death by swap)
instead of a friendly "invalid memory alloc request size".

regards, tom lane

ilmari

--
- Twitter seems more influential [than blogs] in the 'gets reported in
the mainstream press' sense at least. - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
to a mainstream media article. - Calle Dybedahl

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

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Dagfinn Ilmari Mannsåker (#3)
Re: [PATCH] Use correct types and limits for PL/Perl SPI query results

ilmari@ilmari.org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=) writes:

Tom Lane <tgl@sss.pgh.pa.us> writes:

I thought about using UV where feasible, but it was not clear to me
whether unsigned numbers behave semantically differently from signed ones
in Perl. If they do, the change you suggest would create a small semantic
change in the behavior of code using spi_exec_query. I'm not sure it's
worth taking any risk of that, considering we already discourage people
from using spi_exec_query for large results.

IVs and UVs are semantically equivalent, and Perl will automatically
convert between them (and NVs) as necessary, i.e. when crossing
IV_MAX/UV_MAX/IV_MIN.

OK, thanks for the authoritative statement.

I don't much like having such hard-wired knowledge about Perl versions
in our code. Is there a way to identify this change other than
#if PERL_BCDVERSION >= 0x5019004 ?

There isn't, unfortunately.

Sigh ... it was worth asking anyway.

3) To be able to actually return such result sets from sp_exec_query(),
I had to change the repalloc() in spi_printtup() to repalloc_huge().

Hmm, good point. I wonder if there are any hazards to doing that.

One hazard would be that people not heeding the warning in
spi_exec_query will get a visit from the OOM killer (or death by swap)
instead of a friendly "invalid memory alloc request size".

Yeah. But there are plenty of other ways to drive a backend into swap
hell, and the whole point of this change is to eliminate arbitrary
boundaries on query result size. So let's do it.

Thanks for the patch and discussion!

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