From: | Peter Geoghegan <pg(at)heroku(dot)com> |
---|---|
To: | Andrew Gierth <andrew(at)tao11(dot)riddles(dot)org(dot)uk> |
Cc: | Robert Haas <robertmhaas(at)gmail(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-03-25 13:31:57 |
Message-ID: | CAM3SWZT99HR6QxbVEa2Amnk5QPj7-Ln91rM-fVWa037MujLvpg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Mar 24, 2015 at 12:03 AM, Andrew Gierth
<andrew(at)tao11(dot)riddles(dot)org(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Thom Brown | 2015-03-25 13:39:55 | Re: Parallel Seq Scan |
Previous Message | David Steele | 2015-03-25 13:24:44 | Re: Auditing extension for PostgreSQL (Take 2) |