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

From: ilmari(at)ilmari(dot)org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=)
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] Use correct types and limits for PL/Perl SPI query results
Date: 2016-03-14 17:31:27
Message-ID: d8jio0p9dao.fsf@dalvik.ping.uio.no
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> ilmari(at)ilmari(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2016-03-14 17:37:32 Re: WIP: Upper planner pathification
Previous Message Robert Haas 2016-03-14 17:27:56 Re: WIP: Upper planner pathification