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-12-19T21:19:05Z
Lists: pgsql-hackers 
David Rowley <dgrowleyml@gmail.com> writes:
> One minor detail... I think the only thing I'd like to see is the
> moving of the enable_hashagg checks to increment the disabled_nodes
> count in create_setop_path() instead of where it's being called.  I
> understand there's only 1 caller of that function that passes
> SETOP_HASHED, but it does seem nicer to put that logic where it
> belongs. With how you have it now, if we were ever to grow any more
> places that built SETOP_HASHED SetOpPaths, they'd also need to adjust
> disabled_nodes manually and that seems easy to forget.  Also, looking
> around for references to "disabled_nodes", it looks like all other
> places where we fiddle with the value of disabled_nodes are in
> costsize.c.

Looks like costsize.c and pathnode.c to me, but either way I take your
point.  I'd not realized that Robert set it up that way, but now I see
he did.  I agree that moving that bit of logic into
create_setop_path() seems better.  I'll make it so and push.

			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