On Fri, 29 Mar 2024 at 08:53, Tom Lane <t...@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