Re: Abbreviated keys for Numeric

On Fri, Apr 3, 2015 at 1:39 PM, Andrew Gierth
<andrew@tao11.riddles.org.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