On Tue, Mar 24, 2015 at 12:03 AM, Andrew Gierth
<andrew@tao11.riddles.org.uk> wrote:
> So here's the latest (and, hopefully, last) version:
>
> - adds diagnostic output from numeric_abbrev_abort using the trace_sort
> GUC
>
> - fixed Datum cs. uint32 issues in hash_uint32
>
> - added a short comment about excess-k representation
>
> - tweaked the indenting and comments a bit
You still pointlessly check memtupcount here:
+ if (memtupcount < 10000 || nss->input_count < 10000 || !nss->estimating)
+ return false;
I still don't like this comment:
+ nss = palloc(sizeof(NumericSortSupport));
+
+ /*
+ * palloc a buffer for handling unaligned packed values in addition to
+ * the support struct
+ */
+ nss->buf = palloc(VARATT_SHORT_MAX + VARHDRSZ + 1);
This cast to void is unnecessary:
+numeric_fast_cmp(Datum x, Datum y, SortSupport ssup)
+{
+ Numeric nx = DatumGetNumeric(x);
+ Numeric ny = DatumGetNumeric(y);
+ int result;
+
+ (void) ssup;
Please try and at least consider my feedback. I don't expect you to do
exactly what I ask, but I also don't expect you to blithely ignore it.
> I'm not particularly committed to any specific way of handling the
> DEC_DIGITS issue. (I moved away from the "transparently skip
> abbreviations" approach of the original because it seemed that reducing
> #ifdefism in the code was a desirable feature.)
Good.
> The INT64_MIN/MAX changes should be committed fairly soon. (I haven't
> posted a patch for TRACE_SORT)
I wouldn't assume that.
--
Peter Geoghegan