On 23.12.2014 10:16, Jeff Davis wrote: > It seems that these two patches are being reviewed together. Should > I just combine them into one? My understanding was that some wanted > to review the memory accounting patch separately.
I think we should keep the patches separate. Applying two patches is trivial, splitting them not so much. > On Sun, 2014-12-21 at 20:19 +0100, Tomas Vondra wrote: >> That's the only conflict, and after fixing it it compiles OK. >> However, I got a segfault on the very first query I tried :-( > > If lookup_hash_entry doesn't find the group, and there's not enough > memory to create it, then it returns NULL; but the caller wasn't > checking for NULL. My apologies for such a trivial mistake, I was > doing most of my testing using DISTINCT. My fix here was done > quickly, so I'll take a closer look later to make sure I didn't miss > something else. > > New patch attached (rebased, as well). > > I also see your other message about adding regression testing. I'm > hesitant to slow down the tests for everyone to run through this > code path though. Should I add regression tests, and then remove them > later after we're more comfortable that it works? I think when done right, the additional time will be negligible. By setting the work_mem low, we don't need that much data. regards Tomas -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers