Hi Michael,
Thanks for the review.
On 22/04/2026 01:41, Michael Paquier wrote:
> As a whole, we were not really convinced that this is something that
> needs any kind of specific fix, especially not something that should
> be backpatched.
This surprises me a little bit. Until now I was under the impression
that fixing it was the consensus here. Just the "how" was being discussed.
> After saying all that, there is some value in what you are doing here:
> it is true that we lack test coverage in terms of interactions of
> temporary objects across multiple sessions, and that we should have
> some. TAP is adapted for this purpose, isolation tests could be an
> extra one but the schema names make that unpredictible in output. The
> patch unfortunately does a poor job in showing what it wants to
> change. One thing that I would suggest is to *reverse* the order of
> the patches:
> - First have a patch that introduces new tests, that shows the
> original behavior. This needs to be more complete in terms of command
> patterns. The DROP TABLE is one case that we want to keep. This
> should be kept as-is, and it is critical to document the reason why we
> want to keep things this way (aka autovacuum and orphaned tables,
> AFAIK).
> - Then implement the second patch that updates the tests introduced in
> the first patch, so as one can track *what* has changed, and so as one
> does not have to test manually what the original behavior was.
Fair point on tests comments and coverage. But before we start with the
refactoring, I'd like to make sure I understood your suggestion
correctly. You're suggesting:
0001 - TAP tests with improved coverage and comments that pass on
current master, documenting the existing behaviour, which means broken
commands silently succeed (e.g. SELECT returns 0 rows, no error)
0002 - read_stream.c and bufmgr.c fix + updated test expectations (the
same commands now raise errors)
Is it what you had in mind?
Best, Jim
