David Rowley writes:
> I'm fine with the patch, assuming it now includes the new
> BuildTupleHashTable() comment and adjusted
> ExecEndRecursiveUnion/ExecEndSetOp comments.
OK, pushed after a tiny bit more comment-smithing. Thanks
for reviewing!
regards, tom lane
On Thu, 30 Oct 2025 at 13:22, Tom Lane wrote:
>
> David Rowley writes:
> > Is it worth mentioning there that we leave it up to the destruction of
> > the hash table itself to when the ExecutorState context is reset?
> > (Apparently execGrouping.c does not expose any way to do
> > tuplehash_destro
David Rowley writes:
> On Thu, 30 Oct 2025 at 08:51, Tom Lane wrote:
>> Here's a v2 that tries to clean up some of the mess.
> Thanks for doing that cleanup. A good improvement.
Thanks for reviewing!
> Just a couple of
> notes from the review for your consideration:
> 1) The comment in ExecEn
On Sun, 2025-10-26 at 18:11 -0400, Tom Lane wrote:
> Related to this, while I was chasing Jeff's complaint I realized that
> the none-too-small simplehash table for this is getting made in the
> query's ExecutorState. That's pretty awful from the standpoint of
> being able to blame memory consumpt
On Thu, 30 Oct 2025 at 08:51, Tom Lane wrote:
>
> I wrote:
> > David Rowley writes:
> >> I can't help wonder if we can improve the memory context parameter
> >> names in BuildTupleHashTable(). Every time I see "tablecxt" I have to
> >> remind myself that it's not for the bucket array, just the st
I wrote:
> David Rowley writes:
>> I can't help wonder if we can improve the memory context parameter
>> names in BuildTupleHashTable(). Every time I see "tablecxt" I have to
>> remind myself that it's not for the bucket array, just the stuff we
>> have the buckets point to. Would "hashedtuplecxt"
On Mon, 27 Oct 2025 at 16:55, Tom Lane wrote:
> Hmm, I wasn't really expecting any direct time saving; the point
> was about cutting memory consumption. So Chao Li's nearby results
> are in line with mine.
It's for the same reason that Hash Join starts to run more slowly once
the hash table is l
David Rowley writes:
>> On Mon, 27 Oct 2025 at 09:55, Tom Lane wrote:
>>> I looked at the callers of BuildTupleHashTable, and realized that
>>> (a) every one of them can use a BumpContext for the "tablecxt",
>>> and (b) Jeff Davis already noticed that for nodeAgg.c, in commit
>>> cc721c459. So w
. So we have precedent for the idea working. Here's
> a fleshed-out patch to fix the remaining callers.
>
> regards, tom lane
>
> From 15ef2e50085ac83728cb9189e482964ff02e5aae Mon Sep 17 00:00:00 2001
> From: Tom Lane
> Date: Sun, 26 Oct 2025 16:46:57 -0400
> Subj
On Mon, 27 Oct 2025 at 10:46, David Rowley wrote:
>
> On Mon, 27 Oct 2025 at 09:55, Tom Lane wrote:
> >
> > [ starting a new thread to keep this separate from the estimation
> > issue ]
> >
> > I looked at the callers of BuildTupleHashTable, and realized that
> > (a) every one of them can use a B
David Rowley writes:
> On Mon, 27 Oct 2025 at 11:11, Tom Lane wrote:
>> ... We could keep it in that same "tablectx"
> I don't think you could do that and have your patch as SH_GROW() needs
> to pfree the old bucket array after rehashing, which bump won't like.
Ah, good point. Nevermind that i
On Mon, 27 Oct 2025 at 11:11, Tom Lane wrote:
> Related to this, while I was chasing Jeff's complaint I realized that
> the none-too-small simplehash table for this is getting made in the
> query's ExecutorState. That's pretty awful from the standpoint of
> being able to blame memory consumption
David Rowley writes:
> I can't help wonder if we can improve the memory context parameter
> names in BuildTupleHashTable(). Every time I see "tablecxt" I have to
> remind myself that it's not for the bucket array, just the stuff we
> have the buckets point to. Would "hashedtuplecxt" be better?
I
On Mon, 27 Oct 2025 at 09:55, Tom Lane wrote:
>
> [ starting a new thread to keep this separate from the estimation
> issue ]
>
> I looked at the callers of BuildTupleHashTable, and realized that
> (a) every one of them can use a BumpContext for the "tablecxt",
> and (b) Jeff Davis already noticed
So we have precedent for the idea working. Here's
a fleshed-out patch to fix the remaining callers.
regards, tom lane
From 15ef2e50085ac83728cb9189e482964ff02e5aae Mon Sep 17 00:00:00 2001
From: Tom Lane
Date: Sun, 26 Oct 2025 16:46:57 -0400
Subject: [PATCH v1] Use BumpC
15 matches
Mail list logo
