Обсуждение: pg_dump: CREATE TABLE + CREATE RULE vs. relreplident

Hi,

My colleague Per Lejontand brought to my attention that when dumping
views with circular dependencies from a postgres version older than 9.4
using a recent pg_dump, the SQL looks something like the following:

   create table qwr();
   create rule "_RETURN" as on select to qwr do instead select;

In this case the relreplident column in pg_class for the view ends up
being 'd', instead of the 'n' normally used for views.  Patch to update
relreplident when turning a table into a view is attached; this makes
sure that the identity is NOTHING regardless of how the view was created.

I consider this a bug fix, and suggest back patching to 9.4.


.m

>>>>> "Marko" == Marko Tiikkaja <marko@joh.to> writes:
Marko> Hi,
Marko> My colleague Per Lejontand brought to my attention that whenMarko> dumping views with circular dependencies from
apostgres versionMarko> older than 9.4 using a recent pg_dump, the SQL looks somethingMarko> like the following:
 
Marko>   create table qwr();Marko>   create rule "_RETURN" as on select to qwr do instead select;

I've wondered for a while whether this wouldn't have been better handled
as:

create view qwr(colnames...) as select null::type1, null::type2, ...;
/* ... */
create or replace view qwr as ...;

-- 
Andrew (irc:RhodiumToad)

Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> "Marko" == Marko Tiikkaja <marko@joh.to> writes:
>  Marko>   create table qwr();
>  Marko>   create rule "_RETURN" as on select to qwr do instead select;

> I've wondered for a while whether this wouldn't have been better handled
> as:

> create view qwr(colnames...) as select null::type1, null::type2, ...;
> /* ... */
> create or replace view qwr as ...;

Yeah, possibly.  The existing pg_dump coding dates from before we had
CREATE OR REPLACE VIEW.

But we'll have to live with pg_dump files that do this for the indefinite
future, so I agree some fix is needed on the backend side.
        regards, tom lane

Hi,

On 2015-03-12 14:25:24 +0100, Marko Tiikkaja wrote:
> My colleague Per Lejontand brought to my attention that when dumping views
> with circular dependencies from a postgres version older than 9.4 using a
> recent pg_dump, the SQL looks something like the following:
> 
>   create table qwr();
>   create rule "_RETURN" as on select to qwr do instead select;
> 
> In this case the relreplident column in pg_class for the view ends up being
> 'd', instead of the 'n' normally used for views.  Patch to update
> relreplident when turning a table into a view is attached; this makes sure
> that the identity is NOTHING regardless of how the view was created.

I think that's a good idea.

> I consider this a bug fix, and suggest back patching to 9.4.

I agree on backpatching it. Arguably we could additionally avoid
emitting the ALTER TABLE ... REPLICA IDENTITY for views that have
already been created with identity set like this. But I doubt it's worth
it.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services

>>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
>> I've wondered for a while whether this wouldn't have been better>> handled as:
>> create view qwr(colnames...) as select null::type1, null::type2, ...;>> /* ... */>> create or replace view qwr as
...;
Tom> Yeah, possibly.  The existing pg_dump coding dates from before weTom> had CREATE OR REPLACE VIEW.

As it happens it does not; the issue came up originally because of a
hack I came up with, and I've never used any pg version so old it didn't
have CREATE OR REPLACE VIEW. Nor does it look like the change was ever
backpatched (or at least not that far).

http://www.postgresql.org/message-id/20986.1102296367@sss.pgh.pa.us

Of course, at the time I myself didn't think of using a view rather than
a table for the initial creation; I was focused on rules because that
was the only way to get updateable views then. So arguably it is at
least partly my fault.
Tom> But we'll have to live with pg_dump files that do this for theTom> indefinite future, so I agree some fix is
neededon the backendTom> side.
 

Certainly.

-- 
Andrew (irc:RhodiumToad)

Andrew Gierth wrote:
> >>>>> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:

>  Tom> Yeah, possibly.  The existing pg_dump coding dates from before we
>  Tom> had CREATE OR REPLACE VIEW.
> 
> As it happens it does not; the issue came up originally because of a
> hack I came up with, and I've never used any pg version so old it didn't
> have CREATE OR REPLACE VIEW. Nor does it look like the change was ever
> backpatched (or at least not that far).
> 
> http://www.postgresql.org/message-id/20986.1102296367@sss.pgh.pa.us

Wow --- We've had CREATE OR REPLACE VIEW since 2002:

commit 248c67d7ed505d98d3a94cd3954835255317ff16
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date:   Mon Sep 2 02:13:02 2002 +0000
   CREATE OR REPLACE VIEW, CREATE OR REPLACE RULE.   Gavin Sherry, Neil Conway, and Tom Lane all got their hands dirty
on this one ...
 


What is newer is the ability to add columns:

commit ff1ea2173a92dea975d399a4ca25723f83762e55
Author: Bruce Momjian <bruce@momjian.us>
Date:   Sat Dec 6 23:22:46 2008 +0000
   Allow CREATE OR REPLACE VIEW to add columns to the _end_ of the view.      Robert Haas

(Andrew's original post is from 2004)

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> "Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
>  Tom> Yeah, possibly.  The existing pg_dump coding dates from before we
>  Tom> had CREATE OR REPLACE VIEW.

> As it happens it does not; the issue came up originally because of a
> hack I came up with, and I've never used any pg version so old it didn't
> have CREATE OR REPLACE VIEW. Nor does it look like the change was ever
> backpatched (or at least not that far).

Sorry, that's mere historical revisionism.  The oldest PG version I still
have in captivity is 7.0, and in it pg_dump does this:

$ createdb db1    
CREATE DATABASE
$ psql db1
Welcome to psql, the PostgreSQL interactive terminal.

Type:  \copyright for distribution terms      \h for help with SQL commands      \? for help on internal slash commands
    \g or terminate with semicolon to execute query      \q to quit
 

db1=# select version();                            version                              
------------------------------------------------------------------PostgreSQL 7.0.3 on hppa2.0-hp-hpux10.20, compiled by
gcc2.95.3
 
(1 row)

db1=# create table t1 (f1 int, f2 text);
CREATE
db1=# create view v1 as select * from t1;
CREATE 148340 1
db1=# \q
$ pg_dump db1
\connect - postgres
CREATE TABLE "t1" (       "f1" int4,       "f2" text
);
CREATE TABLE "v1" (       "f1" int4,       "f2" text
);
COPY "t1" FROM stdin;
\.
CREATE RULE "_RETv1" AS ON SELECT TO v1 DO INSTEAD SELECT t1.f1, t1.f2 FROM t1;
$ 


Later (in 7.1, looks like) we improved the pg_dump code to dump views as
views, but the underlying ability to dump the ON SELECT rule separately
was still there.  I think what you are remembering is commit
86a069bbed9264daaa85270ece0a2d5959017336, but that just re-enabled the
aboriginal behavior when we discover a circularity involving a view rule.
If I'd had to write actual new dumping code, I probably would not have
done it like that, and might have hit on the CREATE OR REPLACE VIEW
solution instead.

OTOH, some experimenting shows that 7.3 is the oldest version that accepts
the syntax CREATE OR REPLACE VIEW, so at the time we might not have wanted
to use that solution in pg_dump anyway.
        regards, tom lane

On Thu, Mar 12, 2015 at 12:46 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> As it happens it does not; the issue came up originally because of a
>> hack I came up with, and I've never used any pg version so old it didn't
>> have CREATE OR REPLACE VIEW. Nor does it look like the change was ever
>> backpatched (or at least not that far).
>
> Sorry, that's mere historical revisionism.

Can we please keep this a little more civil?  Saying it that way
implies bad faith.  You could instead write "I think you are
mistaken".

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Mar 12, 2015 at 12:46 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Sorry, that's mere historical revisionism.

> Can we please keep this a little more civil?  Saying it that way
> implies bad faith.  You could instead write "I think you are
> mistaken".

Hmm, I take that phrase as being a bit of a joke.  I didn't mean it
to be uncivil, and if Andrew perceives it as such, I apologize.
        regards, tom lane

On Thu, Mar 12, 2015 at 03:14:48PM +0100, Andres Freund wrote:
> Hi,
> 
> On 2015-03-12 14:25:24 +0100, Marko Tiikkaja wrote:
> > My colleague Per Lejontand brought to my attention that when dumping views
> > with circular dependencies from a postgres version older than 9.4 using a
> > recent pg_dump, the SQL looks something like the following:
> > 
> >   create table qwr();
> >   create rule "_RETURN" as on select to qwr do instead select;
> > 
> > In this case the relreplident column in pg_class for the view ends up being
> > 'd', instead of the 'n' normally used for views.  Patch to update
> > relreplident when turning a table into a view is attached; this makes sure
> > that the identity is NOTHING regardless of how the view was created.
> 
> I think that's a good idea.
> 
> > I consider this a bug fix, and suggest back patching to 9.4.
> 
> I agree on backpatching it. Arguably we could additionally avoid
> emitting the ALTER TABLE ... REPLICA IDENTITY for views that have
> already been created with identity set like this. But I doubt it's worth
> it.

Applied and backpatched to 9.4.


--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + Everyone has their own god. +

On 2015-05-01 13:03:39 -0400, Bruce Momjian wrote:
> Applied and backpatched to 9.4.

Ah. Forgot about that. Thanks!

Andres


.oO(bugtracker?)

On Fri, May 1, 2015 at 1:06 PM, Andres Freund <andres@anarazel.de> wrote:
> On 2015-05-01 13:03:39 -0400, Bruce Momjian wrote:
>> Applied and backpatched to 9.4.
>
> Ah. Forgot about that. Thanks!
>
> Andres
>
> .oO(bugtracker?)

I don't really see how that would help.  People can forget about a
ticket in a bugtracker at least as easily as they can forget about a
mailing list thread or a CF entry.

There's no substitute for human nagging, and I'm glad Bruce has taken that on.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company