Re: Abbreviated keys for Numeric

On Thu, Apr 2, 2015 at 10:41 PM, Andrew Gierth
<andrew(at)tao11(dot)riddles(dot)org(dot)uk> wrote:
> Robert> - Changed some definitions to depend on SIZEOF_DATUM rather
> Robert> than USE_FLOAT8_BYVAL. Hopefully I didn't muff this; please
> Robert> check it.
>
> No, that's wrong; it must use USE_FLOAT8_BYVAL since that's what the
> definitions of Int64GetDatum / DatumGetInt64 are conditional on. As you
> have it now, it'll break if you build with --disable-float8-byval on a
> 64bit platform.

I'd prefer to avoid that by avoiding the use of those macros in this
code path rather than by leaving the top 32 bits of the Datum unused
in that configuration. I tried to get there by using a cast for
DatumGetNumericAbbrev(), but I forgot to update the NAN case, so at
least this much is probably needed:

--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -299,10 +299,10 @@ typedef struct
#define NUMERIC_ABBREV_BITS (SIZEOF_DATUM * BITS_PER_BYTE)
#if SIZEOF_DATUM == 8
#define DatumGetNumericAbbrev(d) ((int64) d)
-#define NUMERIC_ABBREV_NAN Int64GetDatum(PG_INT64_MIN)
+#define NUMERIC_ABBREV_NAN ((Datum) PG_INT64_MIN)
#else
#define DatumGetNumericAbbrev(d) ((int32) d)
-#define NUMERIC_ABBREV_NAN Int32GetDatum(PG_INT32_MIN)
+#define NUMERIC_ABBREV_NAN ((Datum) PG_INT32_MIN)
#endif

What other risks do you see?

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