Feedback-ID: i0fe9450f:Fastmail
Date: Wed, 22 Apr 2026 08:41:49 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Alexander Korotkov <aekorotkov@gmail.com>
Cc: Soumya S Murali <soumyamurali.work@gmail.com>,
	Jim Jones <jim.jones@uni-muenster.de>,
	Daniil Davydov <3danissimo@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
	Stepan Neretin <slpmcf@gmail.com>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>,
	Mohamed Ali <moali.pg@gmail.com>,
	Nazneen Jafri <jafrinazneen@gmail.com>,
	Shawn McCoy <shawn.the.mccoy@gmail.com>
Subject: Re: Fix bug with accessing to temporary tables of other sessions
Message-ID: <aegLPbRmc95UbOZG@paquier.xyz>
References: <e58a9580-7bbf-4d70-ad99-833fab77cf88@uni-muenster.de>
 <CAJDiXghBO_TqvHOSui8MOxiFmwLT20+SAnH5nW1rpWHk7Jwffg@mail.gmail.com>
 <402bbc8d-728b-4467-8024-31c2bc101ead@uni-muenster.de>
 <CAJDiXgi0JFk0f2KWWQkzLBC5P7erX9WP18qqnbi-rjZ-K-P=3w@mail.gmail.com>
 <CAMtXxw_hBNuAWQUdSRMpoeRVRYr+5+S7p0bSzuqtHxfpzJPd3w@mail.gmail.com>
 <b6614954-6c71-451a-a1d7-345d255b4afb@uni-muenster.de>
 <CAMtXxw_2VnP5U7U+2ObOCC2voEkAzUqAgH=rPTQBXrb_8K=pyA@mail.gmail.com>
 <CAPpHfdtsGSywUExfKMakAuRz8-+61d-4XHkVf=pNQeYTZTVYoA@mail.gmail.com>
 <CAMtXxw8J7L0r8aXt9NBFfqqSnaNyU=-_Uq4tW4aO3PHyVeXLAQ@mail.gmail.com>
 <CAPpHfdvLCgXgdhSvuPCkek+r=P4E-WiNceWfh7UQztiMzfFcvg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="jgWmfIn17drjFfvG"
Content-Disposition: inline
In-Reply-To: 
 <CAPpHfdvLCgXgdhSvuPCkek+r=P4E-WiNceWfh7UQztiMzfFcvg@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/aegLPbRmc95UbOZG%40paquier.xyz>
Precedence: bulk


--jgWmfIn17drjFfvG
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Tue, Apr 21, 2026 at 01:54:47PM +0300, Alexander Korotkov wrote:
> OK. I'm going to push and backpatch if no objections.

Timing is interesting here.  Last week I have been doing a on-site
patch review with a couple of colleagues and myself, and this thread
has been chosen as part of this exercise.  I am attaching them in CC
of this thread, and replying to this thread was an action item I had
to act on.

Here is some feedback, based on v18 posted here:
https://www.postgresql.org/message-id/b6614954-6c71-451a-a1d7-345d255b4afb@=
uni-muenster.de

We have found that the thread does not state clearly what it aims to
fix.  The subject states that it is a bug, perhaps it is but the
thread does not seem to do a good job in explaining why the current
superuser behavior is bad, while the behavior of the patch to block
some commands is better.  This should be clearer in explaining why
this new behavior is better.

The test script is under-documented.  There are zero comments that
actually explain why the patch does what it does.  The few comments
present could be removed: they are copy-pasted of the descriptions of
the test cases.  A much worse thing is this thing:

+# DROP TEMPORARY TABLE from other session
+$node->safe_psql('postgres', "DROP TABLE $tempschema.foo;");

DROP TABLE on a temporary relation for a superuser is a very useful
behavior to unstuck autovacuum if a temp relation has been orphaned.
It looks critical to me to explain that we want to keep this behavior
for this reason.  Using safe_psql() is not really adapted.  You should
use a psql() where we check that the command does not fail, so as the
test can generate a full report.

The test coverage actually has holes.  The three DML patterns INSERT,
UPDATE and DELETE are covered, and it is missing MERGE.  Also, what
about other patterns like ALTER TABLE, ALTER INDEX, ALTER FUNCTION,
DROP FUNCTION and the like?  There are many object patterns that can
be schema-qualified, and none of this is covered.  What about the new
REPACK and a bunch of other maintenance commands?  There is nothing
about VACUUM, TRUNCATE, CLUSTER, etc.  Just to name a few.

Another question is how do we make sure that new command patterns
follow the same rule as what is enforced here.  It looks like this
should be at least documented somewhere to be less surprising, but the
patch does nothing of the kind.

As a whole the patch lacks documentation, comments, and explanations,
making it difficult to act on.

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.

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.
=20
As a whole, this patch needs more work, based on what has been
currently posted on the lists.  That's not ready yet.  The backpatch
question is a separate matter.

Thanks,
--
Michael

--jgWmfIn17drjFfvG
Content-Type: application/pgp-signature; name=signature.asc

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmnoCz0ACgkQnvQgOdby
QH2tlw//Uq01Z0b1DAqg/dajaF9M4yPAL/X66LI5D3hSntDCvD91EJQXrx002ByN
TlDejUwUtLVZNrg+WNlHL6YDSsOmftutcrb1C5/vV1U5VXSjKp3hPMPOLOx2qL3R
hRUa9WcCAyv3oScqK+Qg3YpkRGfh4HXcto951Sn8e2G2+ik5tIP0oUFx3kLk5lJ2
Y4sQ2PKW9akubLPAFxR1wImecfK+Yrc6dAZdeL7cdwKvQJEq4XewYYIEjS/cC+Nz
KTnC27CI2Uaqy03Lf7RJPRGX6CFgtK5mICI0H1EOUud2bEjbgrxyEf383ZZ1NkVB
0G1ro7lmrTDHKT3Vt5FyBp1knEwTu2zBRDWCZqvEaivCm7eEBHTJRWx03QtPjUlu
MxplsUD7mpfkItlUCanNO3+mdNnQDQZDSF/Nzp6SyKmaHg1RPd8JCjmA+EPXUNgU
845ueVDU7nMo9PtZ4VK1q2Ln/IpDq8wyWN47+nJm6ZkOYbK+Xma+1pnsCo9dL7Lg
t4zUm2VmYzASKoeK3YZd/RladRYMYDh0pxfdQp30bwX5mqg/eDbnFPN0sDaKARKI
jVK47GUyXkvOcWwJ+SJTLA5efQU428MHVPIEOJD3qy6eBrLMyrziSwPL601Sf3R4
ohRSZJdUy8n6C5HG/2I8SYlHwsP+4LEpYJ2bad3bPBrqPxrTs4M=
=7cGA
-----END PGP SIGNATURE-----

--jgWmfIn17drjFfvG--