On Mon, 29 Dec 2025 at 22:00, Chao Li <li.evan.chao@gmail.com> wrote:
> I noticed this problem while reviewing Tom’s patch [1]. The function eqjoinsel() unnecessarily zero-out sslot1 and
sslot2before calling get_attstatsslot() because get_attstatsslot() will unconditionally zero-out sslot in the first
place.
>
> I searched over the source tree, none of other callers of get_attstatsslot() initialize sslot. So for consistency, we
shouldremove the unnecessary zero-out from eqjoinsel().
I looked at this and was confused at why you think they're not needed.
To check if I was misreading the code, I went on and applied your
patch and tried running the tests. I get an immediate crash. You may
not be aware that not all stats have MCVs lists. I believe there needs
to be at least one duplicate value found in the sampled rows for the
MCV list to be generated.
I know you didn't ask me for tips, but I feel the need to provide one
in the hope that at least something good comes out of this; if you're
looking for something to optimise in your patch work, you should
always pick quality over quantity. If too much poor-quality work
arrives here, you're more likely to be ignored than respected. IMO,
anyone who makes it to the ignore list of too many committers isn't
going to have a high success rate of having their patches committed.
Personally, I'd want to stay as far away from that as possible. We're
quite bottlenecked on committer bandwidth already, so the best thing
patch authors can do is make the committers job as easy as possible.
If your patches are a pleasure to commit, then you're more likely to
receive committer attention for your next patch.
Now, as for the patch, you *could* work a bit harder and still get rid
of the redundant zeroing, but I really doubt it's worth the effort.
The struct is 64 bytes. Tiny memsets like this will be inlined by the
compiler and zeroing 64 bytes amounts to a handful of mmx instructions
on x86 with the default -march flags and likely fewer if you use a
more modern -march version. If you believe these functions are hot
enough for this to matter, then please provide proof in the form of
benchmarks.
Otherwise, I recommend going and frying some bigger fish.
David
