Обсуждение: BUG #16703: pg-dump fails to process recursive view definition

The following bug has been logged on the website:

Bug reference:      16703
Logged by:          Alexander Lakhin
Email address:      exclusion@gmail.com
PostgreSQL version: 13.0
Operating system:   Ubuntu 20.04
Description:

The following recursive views (borrowed from regress/sql/lock.sql, see
"detecting infinite recursions in view definitions"):
CREATE VIEW lock_view2(a) AS SELECT NULL::integer;
CREATE VIEW lock_view3 AS SELECT * from lock_view2;
CREATE OR REPLACE VIEW lock_view2 AS SELECT * from lock_view3;
can be created successfully, but make a subsequent pg_dump fail:
pg_dump: error: query failed: ERROR:  infinite recursion detected in rules
for relation "lock_view2"
pg_dump: error: query was: LOCK TABLE public.lock_view2 IN ACCESS SHARE
MODE

The offending commit is 64fc3e03.

Thanks to Andrew Bille for discovering this.

PG Bug reporting form <noreply@postgresql.org> writes:
> The following recursive views (borrowed from regress/sql/lock.sql, see
> "detecting infinite recursions in view definitions"):
> CREATE VIEW lock_view2(a) AS SELECT NULL::integer;
> CREATE VIEW lock_view3 AS SELECT * from lock_view2;
> CREATE OR REPLACE VIEW lock_view2 AS SELECT * from lock_view3;
> can be created successfully, but make a subsequent pg_dump fail:
> pg_dump: error: query failed: ERROR:  infinite recursion detected in rules
> for relation "lock_view2"
> pg_dump: error: query was: LOCK TABLE public.lock_view2 IN ACCESS SHARE
> MODE

> The offending commit is 64fc3e03.

Yeah, not surprising.

It seems like the least painful solution might be to teach
LockTableRecurse to detect recursion and just not recurse into
a view it's already locked.  (BTW, I wonder if we shouldn't
have a stack depth check there, too.)

            regards, tom lane

I wrote:
> It seems like the least painful solution might be to teach
> LockTableRecurse to detect recursion and just not recurse into
> a view it's already locked.  (BTW, I wonder if we shouldn't
> have a stack depth check there, too.)

Oh, I see it already has a recursion test ... it's just that throwing
an error is an unnecessarily harsh reaction.  We can just stop
recursing.  Will push a fix shortly.

(Also, we need no check_stack_depth call here, because
expression_tree_walker has one.)

            regards, tom lane

Further news: I thought to myself "let's leave a self-referential view
behind in the final regression test state, so we can actually exercise
pg_dump/pg_upgrade with one".  It turns out that that's not gonna work,
at least not right away.  pg_dump dumps the view all right, but it dumps

CREATE VIEW "public"."self_referential_view" AS
 SELECT "self_referential_view"."key",
    "self_referential_view"."data"
   FROM "public"."self_referential_view";

which of course fails to load, complaining "relation
"public.self_referential_view" does not exist".

I'm not particularly desperate to do anything about that.  It's important
that pg_dump not fail on such a view, so you don't have a risk that your
backups didn't work at all.  But if you have to do some finagling to
restore it, that's less critical.  Also, this has been the situation all
along and there have been no complaints.

One could imagine getting pg_dump to handle this by treating the
self-reference as a circular reference and then doing what it does
to break reference loops with views.  I experimented briefly with
that, but it's a bigger can of worms than it seems; pg_dump_sort.c
does not seem to have quite enough info to tell whether references
are explicit self-references or not.

            regards, tom lane

Andrew Bille <andrewbille@gmail.com> writes:
> pg_dump is also fails to process the view created by the following script
> (excerpt from privileges.sql):

> CREATE USER user1;
> CREATE TABLE test (col1 varchar(10), col2 boolean);
> SET SESSION AUTHORIZATION user1;
> CREATE VIEW testv AS SELECT * FROM test;

Hm, yeah, so more to do here.  (Sure glad we found these issues before
next week's releases, not after.)

I propose that what we'd better do is

(1) Make pg_dump use LOCK TABLE ONLY, not LOCK TABLE.

(2) Make LOCK TABLE ONLY on a view not recurse to the view's dependencies.
It's quite unclear to me why it didn't work that way all along.

            regards, tom lane

I wrote:
> Andrew Bille <andrewbille@gmail.com> writes:
>> pg_dump is also fails to process the view created by the following script
>> (excerpt from privileges.sql):

>> CREATE USER user1;
>> CREATE TABLE test (col1 varchar(10), col2 boolean);
>> SET SESSION AUTHORIZATION user1;
>> CREATE VIEW testv AS SELECT * FROM test;

> Hm, yeah, so more to do here.  (Sure glad we found these issues before
> next week's releases, not after.)

After some off-list discussion with Alvaro, we're thinking the best way
forward is to revert the pg_dump and LOCK TABLE changes for now, and
try again after next week's releases.  My proposal about

> (2) Make LOCK TABLE ONLY on a view not recurse to the view's dependencies.

seems like it needs wider discussion, and there's not really time to
get that done before the wrap.  Having found these two bugs in the patch
set doesn't inspire confidence that there aren't others, too.

            regards, tom lane