Re: Abbreviated keys for Numeric

From: Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
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 17:39:45
Message-ID: 871tk1w5hy.fsf@news-spur.riddles.org.uk
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>>>>> "Robert" == Robert Haas <robertmhaas(at)gmail(dot)com> writes:

>> I don't consider it appropriate to override ./configure in this way.

Robert> I don't see how that's overriding configure. Commit
Robert> 8472bf7a73487b0535c95e299773b882f7523463, which introduced
Robert> --disable-float8-byval in 2008, states that the motivation for
Robert> this is that it might break functions using the version 0
Robert> calling convention. It should not propagate, like kudzu, into
Robert> things that having nothing to do with how values are passed to
Robert> V0 functions.

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.

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).

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.

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.

--
Andrew (irc:RhodiumToad)

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2015-04-03 18:44:09 Re: Abbreviated keys for Numeric
Previous Message Alvaro Herrera 2015-04-03 16:32:32 Re: initdb -S and tablespaces