Re: 9.5: Better memory accounting, towards memory-bounded HashAgg

From: Jeff Davis <pgsql(at)j-davis(dot)com>
To: Tomas Vondra <tv(at)fuzzy(dot)cz>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: 9.5: Better memory accounting, towards memory-bounded HashAgg
Date: 2014-12-23 09:16:01
Message-ID: 1419326161.24895.13.camel@jeff-desktop
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

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?

Regards
Jeff Davis

Attachment Content-Type Size
hashagg-disk-20141222.patch text/x-patch 45.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ravi Kiran 2014-12-23 09:36:34 mysql with postgres
Previous Message Oskari Saarenmaa 2014-12-23 08:54:39 Re: REINDEX CONCURRENTLY 2.0