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 |
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 |