On Fri, 29 Mar 2024 at 08:53, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> On third thought, I'm not at all convinced that we even want to
> invent this struct as compared to just adding another parameter
> to subquery_planner. The problem with a struct is what happens
> the next time we need to add a parameter? If we add yet another
> function parameter, we can count on the compiler to complain
> about call sites that didn't get the memo. Adding a field
> within an existing struct provokes no such warning, leading
> to situations with uninitialized fields that might accidentally
> work during testing, but fail the minute they get to the field.
I agree it's best to break callers that don't update their code to
consider passing or not passing a SetOperationStmt. I've just
committed a fix to do it that way. This also seems to be the path of
least resistance, which also appeals.
I opted to add a new test alongside the existing tests which validate
set operations with an empty SELECT list work. The new tests include
the variation that the set operation has both a materialized and
non-materialized CTE as a child. This was only a problem with a
materialized CTE, but I opted to include a non-materialized one as I
don't expect that we'll get this exact problem again. I was just keen
on getting more coverage with a couple of cheap tests.
Thanks for your input on this. I'll review your other comments shortly.
David