Re: BUG #17812: LOCK TABLE IN ACCESS EXCLUSIVE MODE with a view returns an empty tuple set

On Wed, Mar 1, 2023 at 8:22 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Julien Rouhaud <rjuju123(at)gmail(dot)com> writes:
> > On Wed, Mar 01, 2023 at 01:53:22AM -0500, Tom Lane wrote:
> >> Maybe it isn't. The code flow for CREATE TABLE AS is a bit weird
> >> IIRC, and maybe it's missing a step where we should update the
> >> active snapshot.
>
> > I think it comes from this chunk in ExecCreateTableAs():
>
> Hm. There are a lot of places that do this:
>
> > PushCopiedSnapshot(GetActiveSnapshot());
> > UpdateActiveSnapshotCommandId();
>
> rather than
>
> PushActiveSnapshot(GetTransactionSnapshot());
>
> which is what would have the effect of noticing changes from other
> sessions. Digging into the history of that, I found this commit:
>
> Author: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
> Branch: master Release: REL9_1_BR [c0b007603] 2011-02-28 23:28:06 -0500
>
> Rearrange snapshot handling to make rule expansion more consistent.
>
> With this patch, portals, SQL functions, and SPI all agree that there
> should be only a CommandCounterIncrement between the queries that are
> generated from a single SQL command by rule expansion.
>
[...]

>
> So I guess it's intended behavior that we don't notice other-session
> changes between steps of a single command. Whether that rule should
> apply to CREATE TABLE AS is perhaps debatable --- but I see several
> other places that are doing it exactly like this, so it's not like
> CREATE TABLE AS is alone in its folly.
>
> I'm pretty hesitant to change this without substantial thought.
>
>
So both Sessions 4 (view) and 5 (table) make it into tcop/postgres.c and
into parse analysis.

The view session gets past there because it is a utility command whose body
is parsable. postgres.c tcop invokes the executor (with a new snapshot but
still of the empty table) for the utility command CTAS passing the parsable
query along. CTAS performs rewrite and blocks there while holding the
empty table snapshot, and when it continues on to execution CTAS doesn't
pull forward the snapshot.

The table session fails to get past parse analysis while the lock is held
and once it does postgres.c tcop assigns a new snapshot which does contain
the data in table1, then invokes the CTAS utility.

To get parity in behavior either CTAS would need to assign a new snapshot
OR postgres.c tcop would need to perform rewriting on the query under the
CTAS prior to beginning execution of CTAS, since that rewriting would then
block the process before the new snapshot is assigned. (That is all in a
mostly questioning, not authoritative, tone

The non-CTAS view query gets blocked on rewrite while the non-CTAS table
query gets blocked in parse analysis. When unblocked, postgres.c tcop
assigns both a new snapshot just prior to execution.

I experimented with putting the create temp table into a function body then
calling the function - same results.

I agree this does seem like a poor risk/reward on the fixing side,
especially absent a concrete live use case problem. I am curious what led
to this discovery.

In terms of documentation; "query began" is the terminology we use but as
demonstrated here, from an end-user's perspective, and in the presence of
locking, the point at which the query began seems undefined. If it was
when I send the statement to the server then none of the four sessions
should be producing results, instead I get three that seem to violate the
read-committed rule. Which moves expectations to "begins retrieving rows"
in which case the CTAS+View combination, getting stuck in between, when the
CTAS+table one doesn't, indeed seems odd and maybe should at least be
addressed, even in general terms.

David J.