Thank you for having another look at this. On Wed, 3 Jul 2024 at 00:20, Matthias van de Meent <boekewurm+postg...@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
v3-0001-Remove-incorrect-Asserts-in-buffile.c.patch
Description: Binary data
v3-0002-Add-memory-disk-usage-for-Material-in-EXPLAIN-ANA.patch
Description: Binary data
v3-0003-Optimize-memory-management-and-increase-performan.patch
Description: Binary data
