Horton

Re: Converting SetOp to read its two inputs separately

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: David Rowley <dgrowleyml@gmail.com>
Cc: Richard Guo <guofenglinux@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2024-11-29T23:36:35Z
Lists: pgsql-hackers

Attachments

I wrote:
> David Rowley <dgrowleyml@gmail.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

Commits

  1. Get rid of old version of BuildTupleHashTable().

  2. Use ExecGetCommonSlotOps infrastructure in more places.

  3. Improve planner's handling of SetOp plans.

  4. Convert SetOp to read its inputs as outerPlan and innerPlan.

  5. Fix typo in header comment for set_operation_ordered_results_useful

  6. Allow planner to use Merge Append to efficiently implement UNION