| From: | Jim Jones <jim(dot)jones(at)uni-muenster(dot)de> |
|---|---|
| To: | Michael Paquier <michael(at)paquier(dot)xyz>, Alexander Korotkov <aekorotkov(at)gmail(dot)com> |
| Cc: | Soumya S Murali <soumyamurali(dot)work(at)gmail(dot)com>, Daniil Davydov <3danissimo(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Stepan Neretin <slpmcf(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Mohamed Ali <moali(dot)pg(at)gmail(dot)com>, Nazneen Jafri <jafrinazneen(at)gmail(dot)com>, Shawn McCoy <shawn(dot)the(dot)mccoy(at)gmail(dot)com> |
| Subject: | Re: Fix bug with accessing to temporary tables of other sessions |
| Date: | 2026-04-24 22:09:39 |
| Message-ID: | 98c1c0d9-7962-4669-90f2-deb28a665a55@uni-muenster.de |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | David G. Johnston | 2026-04-24 22:30:51 | Re: Fix bug with accessing to temporary tables of other sessions |
| Previous Message | Bharath Rupireddy | 2026-04-24 20:20:56 | Re: Fix race condition in pg_get_publication_tables with concurrent DROP TABLE |