>>>>> "Peter" == Peter Geoghegan <pg@heroku.com> writes:
Peter> Your V3 has obsolete comments here:
Peter> + nss = palloc(sizeof(NumericSortSupport));Peter> +Peter> + /*Peter> + * palloc a buffer for handling unaligned
packedvalues in addition toPeter> + * the support structPeter> + */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 opclassPeter> behavior here:
Consider it a fence against people trying to change the code for
"consistency".
Peter> This is dubious:
Peter> +#if DEC_DIGITS != 4Peter> +#error "Numeric bases other than 10000 are no longer supported"Peter> +#endif
Peter> Because there is a bunch of code within numeric.c that dealsPeter> with the DEC_DIGITS != 4 case. For example,
thiscode has beenPeter> 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, hePeter> thought it might be useful to change
DEC_DIGITSon certainPeter> builds. And so, we ought to continue to support it to the extentPeter> 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)