Re: Abbreviated keys for Numeric

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>
Cc: Peter Geoghegan <pg(at)heroku(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>, Gavin Flower <GavinFlower(at)archidevsys(dot)co(dot)nz>
Subject: Re: Abbreviated keys for Numeric
Date: 2015-04-03 18:44:09
Message-ID: CA+TgmoYP3SZ6yW53Z2cD4S6FZzGbhnp0SoBkrn_2V9DdHkshpA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Apr 3, 2015 at 1:39 PM, Andrew Gierth
<andrew(at)tao11(dot)riddles(dot)org(dot)uk> wrote:
> It already does; it changes how int64 values are expected to be stored
> in Datum variables. _Everything_ that currently stores an int64 value in
> a Datum is affected.

But this isn't a value of the SQL type "int64". It's just a bit
pattern that has to fit inside however big a Datum happens to be.

> As I see it, you need a really good reason to override that in a
> specific case, and supporting 64-bit abbreviations on a
> --disable-float8-byval build really isn't a good reason (since 32-bit
> abbrevs work fine and such builds should be vanishingly rare anyway).

In my opinion, there is way too much crap around already just to cater
to --disable-float8-byval. I think not adding more is a perfectly
reasonable goal. If somebody wants to remove --disable-float8-byval
some day, I want to minimize the amount of stuff they have to change
in order to do that. I think that keeping this off the list of stuff
that will require modification is a worthy goal.

> The fact that making this one low-benefit change has introduced no less
> than three separate bugs - the compile error with that #ifdef, the use
> of Int64GetDatum for NANs, and the use of Int64GetDatum for the return
> value of the abbreviation function should possibly be taken as a hint to
> how bad an idea is.

But all of those are trivial, and the first would have been caught by
my compiler if I weren't using such a crappy old compiler. If
anything that might require as much as 10 lines of code churn to fix
is not worth doing, very little is worth doing.

> If you're determined to go this route - over my protest - then you need
> to do something like define a NumericAbbrevGetDatum(x) macro and use it
> in place of the Int64GetDatum / Int32GetDatum ones for both NAN and the
> return from numeric_abbrev_convert_var.

Patch for that attached.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment Content-Type Size
numeric-abbrev-fixes.patch application/x-patch 1.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2015-04-03 19:06:54 Re: Abbreviated keys for Numeric
Previous Message Andrew Gierth 2015-04-03 17:39:45 Re: Abbreviated keys for Numeric