Thank you for having another look at this.
On Wed, 3 Jul 2024 at 00:20, Matthias van de Meent
<boekewurm+postgres@gmail.com> wrote:
> I noticed this change to buffile.c shows up in both v1 and v2 of the
> patchset, but I can't trace the reasoning why it changed with this
> patch (rather than a separate bugfix):
I didn't explain that very well here, did I. I took up the discussion
about that in [1]. (Which you've now seen)
> > +show_material_info(MaterialState *mstate, ExplainState *es)
> > + spaceUsedKB = (tuplestore_space_used(tupstore) + 1023) / 1024;
>
> I think this should use the BYTES_TO_KILOBYTES macro for consistency.
Yes, that needs to be done. Thank you.
> Lastly, I think this would benefit from a test in
> regress/sql/explain.sql, as the test changes that were included
> removed the only occurrance of a Materialize node from the regression
> tests' EXPLAIN outputs.
I've modified the tests where the previous patch disabled
enable_material to enable it again and mask out the possibly unstable
part. Do you think that's an ok level of testing?
David
[1] https://postgr.es/m/CAApHDvofgZT0VzydhyGH5MMb-XZzNDqqAbzf1eBZV5HDm3%2BosQ%40mail.gmail.com
