public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: David Rowley <[email protected]>
Cc: PostgreSQL Developers <[email protected]>
Subject: Re: Incorrect CHUNKHDRSZ in nodeAgg.c
Date: Wed, 01 Jan 2025 19:33:23 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAApHDvozg3jDN4oVYvtpA91P1DA50gdRZ5071SnPg_24oL5PbA@mail.gmail.com>
References: <CAApHDvpMpRQvsTqZo3FinXkgytwxwF8sCyZm83xDj-1s_hLe+w@mail.gmail.com>
	<[email protected]>
	<CAApHDvozg3jDN4oVYvtpA91P1DA50gdRZ5071SnPg_24oL5PbA@mail.gmail.com>

David Rowley <[email protected]> writes:
> On Thu, 2 Jan 2025 at 12:18, Tom Lane <[email protected]> wrote:
>> I thought for a bit about whether we shouldn't try to account for
>> palloc power-of-2-block-size overhead here.  That omission would
>> typically be a far larger error than the one you are fixing.  However,
>> given that the inputs to hash_agg_entry_size are only estimates,
>> I'm not sure that we can hope to do better than the current behavior.

> Likely the most correct way would be to use GetMemoryChunkSpace(), but
> there might be some additional overhead to consider there.

Nah, you've got the wrong mental model.  hash_agg_entry_size is
trying to predict the average hash entry size in advance of seeing
any actual data, so that we can estimate how many entries will fit
in work_mem.  By the time we can use GetMemoryChunkSpace on an
actual entry, it's too late for that.

>> Could we use a generation or even bump context?

> Bump wouldn't work due to the SH_FREE() in SH_GROW() when resizing the
> table.

Meh.  I guess we'd have to keep that structure in a context separate
from the tuples.  Might not be worth the trouble.

> I think what would be more interesting is seeing if we can store the
> TupleHashEntryData.firstTuple in a bump context.

Are you saying the same as above, or something different?

			regards, tom lane

view thread (12+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Incorrect CHUNKHDRSZ in nodeAgg.c
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox