From: Tom Lane <tgl@sss.pgh.pa.us>
To: Chao Li <li.evan.chao@gmail.com>
Cc: pgsql-hackers@lists.postgresql.org
Subject: Re: Fixing some ancient errors in hash join costing
Date: Mon, 29 Dec 2025 13:08:39 -0500
Message-ID: <2985328.1767031719@sss.pgh.pa.us> (raw)
In-Reply-To: <F364A5CB-C518-4D28-8778-50F2F6BE9BDE@gmail.com>
References: <2380165.1766871097@sss.pgh.pa.us>
<2551253.1766952956@sss.pgh.pa.us>
<2712245.1766962600@sss.pgh.pa.us>
<705AC80B-8E3A-4DA2-9FC2-A05C4D6C70FD@gmail.com>
<2742257.1766978956@sss.pgh.pa.us>
<F364A5CB-C518-4D28-8778-50F2F6BE9BDE@gmail.com>
Chao Li <li.evan.chao@gmail.com> writes:
> On Dec 29, 2025, at 11:29, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Chao Li <li.evan.chao@gmail.com> writes:
>>> As flag 0 is passed to get_attstatsslot(), free_attstatsslot() is not needed.
>> True. I wrote it like that so people wouldn't wonder if I'd forgotten
>> free_attstatsslot(), but if other call sites passing flags == 0 don't
>> use it then it'd be better to be consistent. (I didn't check that.)
> I searched over the source tree, looks like not a direct reference. The only usages of flag 0 is in eqjoinsel(), the code snippet is:
Yeah, that's not a direct precedent since there is a
free_attstatsslot() further down; nonetheless it is relying on the
call with flags==0 to not do anything that'd need freeing, since
there's later calls that may overwrite the sslot structs. But
get_attstatsslot's API spec is clear enough that you don't need
free_attstatsslot() in this usage, so I removed it.
> My thought was just that at the point of initialization, the nearby comment reads as if we’re about to fetch a value, whereas the assignment is really initializing with “unknown so far”. A short tweak like “Initialize MCV frequency to unknown” might help make that intent obvious locally, but I’m fine either way.
Fair enough, I modified it along those lines.
Pushed, thanks for reviewing.
regards, tom lane
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: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, li.evan.chao@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Fixing some ancient errors in hash join costing
In-Reply-To: <2985328.1767031719@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox