On Thu, 19 Sept 2024 at 22:17, Tatsuo Ishii <ishii@postgresql.org> wrote:
> Attached patch fixes 1 & 2.
I looked at this and thought that one thing you might want to consider
is adjusting show_storage_info() to accept the size and type
parameters so you don't have to duplicate the formatting code in
show_recursive_union_info().
The first of the new tests also isn't testing what you want it to
test. Maybe you could add a "materialized" in there to stop the CTE
being inlined:
explain (analyze,costs off) with w(n) as materialized (select n from
generate_series(1,10) a(n)) select sum(n) from w
Also, I'm on the fence about if the new tests are worthwhile. I won't
object to them, however. I just wanted to note that most of the
complexity is in tuplestore.c of which there's already coverage for.
The test's value is reduced by the fact that most of the interesting
details have to be masked out due to possible platform variations in
the number of bytes. Really the new tests are only testing that we
display the storage details and maybe that the storage type came out
as expected. It seems unlikely these would get broken. I'd say it's
committers preference, however. I just wanted to add my thoughts. You
have to offset the value against the fact that the expected output is
likely to change over the years which adds to the burden of making
changes to the EXPLAIN output.
David
