Re: refactoring - share str2*int64 functions

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: refactoring - share str2*int64 functions
Date: 2019-08-30 14:50:21
Message-ID: alpine.DEB.2.21.1908301640500.28828@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Michaël,

>> For symmetry and efficiency, ISTM that uint16 mul overflow could use uint32
>> and uint32 could use uint64, and the division-based method be dropped in
>> these cases.
>
> Yes, the division would be worse than the other. What do you think
> about using the previous module I sent and measure how long it takes
> to evaluate the overflows in some specific cases N times in loops?

Given the overheads of the SQL interpreter, I'm unsure about what you
would measure at the SQL level. You could just write a small standalone C
program to test perf and accuracy. Maybe this is what you have in mind.

>> I would tend to commit working solutions per type rather than by
>> installment, so that at least all added functions are actually used
>> somewhere, but it does not matter much, really.
>
> I prefer by section, with testing dedicated to each part properly
> done so as we can move to the next parts.

This suggests that you will test twice: once when adding the inlined
functions, once when calling from SQL.

>> The checking extension is funny, but ISTM that these checks should be (are
>> already?) included in some standard sql test, which will test the macros
>> from direct SQL operations:
>
> Sure. But not for the unsigned part as there are no equivalent
> in-core data types,

Yep, it bothered me sometimes, but not enough to propose to add them.

> still it is possible to trick things with signed integer arguments.

Is it?

> I found my toy useful to check test all implementations consistently.

Ok.

>> fabien=# SELECT INT2 '1234512434343';
>> ERROR: value "1234512434343" is out of range for type smallint
>>
>> Well, a quick look at "src/test/regress/sql/int2.sql" suggests that
>> there the existing tests should be extended… :-(
>
> We can tackle that separately. -32768 is perfectly legal for
> smallint, and the test is wrong here.

Do you mean:

sql> SELECT -32768::INT2;
ERROR: smallint out of range

This is not a negative constant but the reverse of a positive, which is
indeed out of range, although the error message could help more.

sql> SELECT (-32768)::INT2;
-32768 # ok

sql> SELECT INT2 '-32768';
-32768 # ok

--
Fabien.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2019-08-30 14:55:05 Re: pg_upgrade: Error out on too many command-line arguments
Previous Message Tom Lane 2019-08-30 14:47:48 Re: pg_upgrade: Error out on too many command-line arguments