Re: Converting SetOp to read its two inputs separately

David Rowley <dgrowleyml@gmail.com>

From: David Rowley <dgrowleyml@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Richard Guo <guofenglinux@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2024-12-19T22:53:26Z
Lists: pgsql-hackers
On Fri, 20 Dec 2024 at 11:23, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Pushed ... and now I have one more beef about the way things are
> in this area.  I don't think we should leave the compatibility
> function BuildTupleHashTable() in place in HEAD.  Making it a
> wrapper around a new function BuildTupleHashTableExt() was a fine
> solution for preserving ABI in released branches, but that doesn't
> mean we should clutter the code with unused ABI hacks forevermore.
> Attached is a patch to take it out and then rename
> BuildTupleHashTableExt() back to BuildTupleHashTable().

No complaints here. Thanks for cleaning that up.

I couldn't help but also notice the nbuckets parameter is using the
"long" datatype. The code in BuildTupleHashTable seems to think it's
fine to pass the long as the uint32 parameter to tuplehash_create().
I just thought if we're in the area of adjusting this API function's
signature then it might be worth fixing the "long" issue at the same
time, or at least in the same release.

I'm also quite keen to see less use of long as it's not a very
consistently sized datatype on all platforms which can lead to
platform dependent bugs.

David



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