Re: Converting SetOp to read its two inputs separately

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: Richard Guo <guofenglinux(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Converting SetOp to read its two inputs separately
Date: 2024-11-29 23:36:35
Message-ID: 793904.1732923395@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> David Rowley <dgrowleyml(at)gmail(dot)com> writes:
>> nodeAgg.c seems to do this by using prepare_hash_slot() which deforms
>> the heap tuple and sets the Datums verbatim rather than making copies
>> of any byref ones.

> I'll take a look at that, thanks for the pointer.

So after digging into it, I realized that I'd been completely misled
by setop_fill_hash_table's not failing while reading the first input.
That's not because the code was correct, it was because none of our
test cases manage to reach TupleHashTableMatch() while reading the
first input. Apparently there are no regression test cases where
tuples of the first input have identical hash codes. But as soon
as I tried a case with duplicate rows in the first input, kaboom!

The short answer therefore is that LookupTupleHashEntry() absolutely
requires a MinimalTuple slot as input, and there is not some magic
code path whereby it doesn't. So I put in some code similar to
prepare_hash_slot() to do that as efficiently as can be managed.

I also switched to using slot_getallattrs() per your suggestion.
Other than that and rebasing, this is the same as v2.

regards, tom lane

Attachment Content-Type Size
v3-0001-Convert-SetOp-to-read-its-inputs-as-outerPlan-and.patch text/x-diff 70.0 KB
v3-0002-Remove-some-dead-code-in-prepunion.c.patch text/x-diff 12.4 KB
v3-0003-Get-rid-of-choose_hashed_setop.patch text/x-diff 16.9 KB
v3-0004-Fix-bogus-decisions-about-whether-we-want-pre-sor.patch text/x-diff 12.3 KB
v3-0005-Teach-generate_nonunion_paths-to-consider-pre-sor.patch text/x-diff 11.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2024-11-29 23:42:53 Re: Cannot find a working 64-bit integer type on Illumos
Previous Message Bruce Momjian 2024-11-29 22:21:08 Re: [PATCH] SVE popcount support