Re: Abbreviated keys for Numeric

>>>>> "Peter" == Peter Geoghegan <pg(at)heroku(dot)com> writes:

Peter> Your V3 has obsolete comments here:

Peter> + nss = palloc(sizeof(NumericSortSupport));
Peter> +
Peter> + /*
Peter> + * palloc a buffer for handling unaligned packed values in addition to
Peter> + * the support struct
Peter> + */
Peter> + nss->buf = palloc(VARATT_SHORT_MAX + VARHDRSZ + 1);

I don't see why it's obsolete; it still describes what the code is
doing, even though the buffer is no longer contiguous with the support
struct. The code makes it clear that the buffer is separate.

Peter> I still don't think you should be referencing the text opclass
Peter> behavior here:

Consider it a fence against people trying to change the code for
"consistency".

Peter> This is dubious:

Peter> +#if DEC_DIGITS != 4
Peter> +#error "Numeric bases other than 10000 are no longer supported"
Peter> +#endif

Peter> Because there is a bunch of code within numeric.c that deals
Peter> with the DEC_DIGITS != 4 case. For example, this code has been
Peter> within numeric.c forever:

The earlier comment should make it clear that all the DEC_DIGITS != 4
support is "historical". I didn't consider it appropriate to actually
rip out all the #ifs; I simply tried to make it clear where the
landmines were if anyone wanted to try experimenting with other
DEC_DIGITS values.

Peter> I tend to think that when Tom wrote this code back in 2003, he
Peter> thought it might be useful to change DEC_DIGITS on certain
Peter> builds. And so, we ought to continue to support it to the extent
Peter> that we already do,

Right, but we really don't support it in any meaningful sense. The
base-10000 version was added for 7.4, which was also the first version
with protocol-3 support. Since the V3 protocol has usable support for
binary mode, people since then have been writing interfaces on the
assumption that the binary representation for numerics is base-10000.
Since there's no good way to query the server for the value of
DEC_DIGITS (and so clients don't try), this means that changing it
breaks compatibility.

--
Andrew (irc:RhodiumToad)