MIME-Version: 1.0
References: <CA+HiwqFpZ80UJKr4tZus4Omgg7YESzFXKSwSHRW2Ap2=XSVyUA@mail.gmail.com>
 <202406191709.jbvpf7d7hl6g@alvherre.pgsql> <CA+HiwqF+3tv=tuB9EVfOj9YcXhSq477X+1RKOpJ5JqCCj3qgww@mail.gmail.com>
 <CA+TgmobHL_vTjOdy6KVMVeW-CQQmXXz_yU6Q9d2YjnVfFxuy6A@mail.gmail.com>
 <CA+HiwqHL=aGU9Y4RYXQ5VCp4L5NVdiaQLLoXN3NCQQQMKo0ByQ@mail.gmail.com>
 <CA+TgmoabYD=pnccFLzbbREFsqkFgE4EZ+FdHoTOhgCqn4jP2Cw@mail.gmail.com>
 <CA+HiwqE_rQ9pZnkXeoHdds2kgAiT7XNNHZW8gTGicBdXv0rwnw@mail.gmail.com>
 <CA+TgmoY2drv9PmrRAC7AR77mkx09sOh-+5qJkHB_hLKeHRNqzQ@mail.gmail.com> <1386845.1724086454@sss.pgh.pa.us>
In-Reply-To: <1386845.1724086454@sss.pgh.pa.us>
From: Robert Haas <robertmhaas@gmail.com>
Date: Mon, 19 Aug 2024 13:38:28 -0400
Message-ID: <CA+TgmoaLXb+JpPGYrw08BNQM74VEMPH_MOy5SK4p_+TDXH2z9w@mail.gmail.com>
Subject: Re: generic plans and "initial" pruning
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Amit Langote <amitlangote09@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, 
	Andres Freund <andres@anarazel.de>, Daniel Gustafsson <daniel@yesql.se>, 
	David Rowley <dgrowleyml@gmail.com>, PostgreSQL Hackers <pgsql-hackers@postgresql.org>, 
	Thom Brown <thom@linux.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CA%2BTgmoaLXb%2BJpPGYrw08BNQM74VEMPH_MOy5SK4p_%2BTDXH2z9w%40mail.gmail.com>
Precedence: bulk

On Mon, Aug 19, 2024 at 12:54=E2=80=AFPM Tom Lane <tgl@sss.pgh.pa.us> wrote=
:
> What the examples here are showing is that AcquireExecutorLocks
> is incomplete because it only provides defenses against DDL
> initiated by other sessions, not by our own session.  We have
> CheckTableNotInUse but I'm not sure if it could be applied here.
> We certainly aren't calling that in anywhere near as systematic
> a way as we have for acquiring locks.
>
> Maybe we should rethink the principle that a session's locks
> never conflict against itself, although I fear that might be
> a nasty can of worms.

It might not be that bad. It could replace the CheckTableNotInUse()
protections that we have today but maybe cover more cases, and it
could do so without needing any changes to the shared lock manager.
Say every time you start a query you give that query an ID number, and
all locks taken by that query are tagged with that ID number in the
local lock table, and maybe some flags indicating why the lock was
taken. When a new lock acquisition comes along you can say "oh, this
lock was previously taken so that we could do thus-and-so" and then
use that to fail with the appropriate error message. That seems like
it might be more powerful than the refcnt check within
CheckTableNotInUse().

But that seems somewhat incidental to what this thread is about. IIUC,
Amit's original design involved having the plan cache call some new
executor function to do partition pruning before lock acquisition, and
then passing that data structure around, including back to the
executor, so that we didn't repeat the pruning we already did, which
would be a bad thing to do not only because it would incur CPU cost
but also because really bad things would happen if we got a different
answer the second time. IIUC, you didn't think that was going to work
out nicely, and suggested instead moving the pruning+locking to
ExecutorStart() time. But now Amit is finding problems with that
approach, because by the time we reach PortalRun() for the
PORTAL_MULTI_QUERY case, it's too late to replan, because we can't ask
the plancache to replan just one query from the list; and if we try to
fix that by moving ExecutorStart() to PortalStart(), then there are
other problems. Do you have a view on what the way forward might be?

This thread has gotten a tad depressing, honestly. All of the opinions
about what we ought to do seem to be based on the firm conviction that
X or Y or Z will not work, rather than on the confidence that A or B
or C will work. Yet I'm inclined to believe this problem is solvable.

--=20
Robert Haas
EDB: http://www.enterprisedb.com