On Fri, Feb 20, 2015 at 4:01 PM, Peter Geoghegan <p...@heroku.com> wrote: > On Fri, Feb 20, 2015 at 11:58 AM, Tomas Vondra > <tomas.von...@2ndquadrant.com> wrote: >> This seems to happen because ordered_set_startup() calls >> tuplesort_begin_datum() when (use_tuples == true), which only sets >> 'onlyKey' and leaves (sortKeys == NULL). So 'mergeruns' fails because it >> does not expect that. > > Oops. You're right. Attached patch fixes the issue, by making > tuplesort_begin_datum() consistent with other comparable routines for > other tuple cases. I think it makes sense to have the 'sortKeys' field > always set, and the 'onlyKey' field also set when that additional > optimization makes sense. That was what I understood the API to be, so > arguably this is a pre-existing issue with tuplesort_begin_datum().
This doesn't completely fix the issue. Even with this patch applied: CREATE TABLE stuff AS SELECT random()::text AS randtext FROM generate_series(1,50000000); set maintenance_work_mem='1024'; create index on stuff using hash (randtext); ...wait for a bit, server crash... I find your statement that this is a pre-existing issue in tuplesort_begin_datum() to be pretty misleading, unless what you mean by it is "pre-existing since November, when an earlier patch by Peter Geoghegan changed the comments without changing the code to match". Commit 5ea86e6e65dd2da3e9a3464484985d48328e7fe3, changed the comments for the sortKey field to say that it would be set in all cases except for the hash index case, but it did not make that statement true. Subsequently, another of your patches, which I committed as 5cefbf5a6c4466ac6b1cc2a4316b4eba9108c802, added a dependency on state->sortKeys always being set. Now you've got this patch here, which you claim makes sure that sortKeys is always set, but it actually doesn't, which is why the above still crashes. There are not so many tuplesort_begin_xxx routines that you couldn't audit them all before submitting your patches. Anyway, I propose the attached instead, which makes both Tomas's test case and the one above pass. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
tuplesort-fixes.patch
Description: binary/octet-stream
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
