Обсуждение: pg_dump compatibility level / use create view instead of create table/rule

Hi,

Can someone let me know when you're doing a pg_dump, can you specify not to use the view rules so that the statement in the pg_dump file uses create view instead of create table/create rule? We are not using anything specific to 9.5 like jsonb columns, so the DDL should be compatible between versions when creating / defining objects, it just the way it's creating them that has changed which is causing us an issue.

We dump from 9.5.5 and restore to one 9.5.18 server and two 9.2 servers....we've been doing this for awhile and had no issues until recently with certain views that are trying to be restored with rule views (some views in the pg_dump file are created with create view and some by create table / create rule) I've read this: https://www.postgresql.org/docs/9.5/rules-views.html but haven't fully understood it yet as to when it applies the create view  vs create table/rule syntax, as the pg_dump has a combination of both.

On the 9.5.18 server where it has the create table syntax for a view, it creates a table instead of a view.

For the 9.2.9 servers, it generates errors:

pg_restore: [archiver (db)] Error from TOC entry 11240; 1259 42703182 TABLE v_my_view postgres

LINE 19: ...E ONLY v_my_view REPLICA ID...

    Command was: CREATE TABLE v_my_view(

pg_restore: [archiver (db)] Error from TOC entry 87613; 2618 42703185 RULE _RETURN postgres

    Command was: CREATE RULE "_RETURN" AS

Aside from an upgrade to all the servers, is there anyway in pg_dump to set a compatibility level when dumping the database? I checked here, and I don't think there is: https://www.postgresql.org/docs/9.5/app-pgdump.html

Many thanks in advance.

Alex

Our setup is the following:

1. Source Postgresql 9.5 server (pg_dump source)

PostgreSQL 9.5.5 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-17), 64-bit

2. Two 9.2.9 servers (we restore to)

PostgreSQL 9.2.9 on x86_64-redhat-linux-gnu, compiled by gcc (GCC) 4.8.2 20140120 (Red Hat 4.8.2-16), 64-bit

3. One 9.5  (we restore to)

PostgreSQL 9.5.18 on x86_64-pc-linux-gnu, compiled by gcc (GCC) 4.4.7 20120313

(Red Hat 4.4.7-23), 64-bit

Sent with ProtonMail Secure Email.

Alex Williams <valenceshell@protonmail.com> writes:
> Can someone let me know when you're doing a pg_dump, can you specify not to use the view rules so that the statement
inthe pg_dump file uses create view instead of create table/create rule? 

No.

> We dump from 9.5.5 and restore to one 9.5.18 server and two 9.2 servers....we've been doing this for awhile and had
noissues until recently with certain views that are trying to be restored with rule views (some views in the pg_dump
fileare created with create view and some by create table / create rule) 

In general, we don't promise that pg_dump output from major version N
can be loaded into previous major versions.  Having said that, 9.2
should not have a problem with either the CREATE VIEW or CREATE TABLE-
plus-CREATE RULE approaches per se, so there's some critical detail
that you haven't told us about.  You didn't show the actual error
messages, either.

            regards, tom lane

Hi Tom,

Thanks for your reply, we appreciate it. This is a long reply, sorry about that, but if there's any specific I can
provideyou that helps, please let me know. 

OK, for the log, we do this when restoring:

pg_restore -d my_database -U postgres my_database.dump >restore_result.txt 2>&1

but our log file only records the following (I've added more detail below using the cmds below.) The view name/column
nameshave changed for the mailing list: 

"CREATE TABLE" -  cat restore_result.txt | grep -A 10 -B 10 -i "CREATE TABLE" | more

pg_restore: [archiver (db)] Error from TOC entry 11240; 1259 42703182 TABLE my_view postgres
pg_restore: [archiver (db)] could not execute query: ERROR:  syntax error at or near "REPLICA"
LINE 19: ...E ONLY my_view REPLICA ID..

pg_restore: [archiver (db)] could not execute query: ERROR:  relation "myschema.my_view " does not exist



"CREATE RULE" - cat restore_result.txt | grep -A 10 -B 10 -i "CREATE RULE" | more
pg_restore: [archiver (db)] Error from TOC entry 87618; 2618 42703185 RULE _RETURN postgres
pg_restore: [archiver (db)] could not execute query: ERROR:  relation "my_view" does not exist
    Command was: CREATE RULE "_RETURN" AS
    ON SELECT TO my_view DO INSTEAD  SELECT DISTINCT d.name AS p...



We assumed it was the create rule but also looked at "REPLICA ID" and couldn't find anything on the properties that it
hadsuch a property ... we used the query from here:
https://stackoverflow.com/questions/55249431/find-replica-identity-for-a-postgres-table

SELECT CASE relreplident
          WHEN 'd' THEN 'default'
          WHEN 'n' THEN 'nothing'
          WHEN 'f' THEN 'full'
          WHEN 'i' THEN 'index'
       END AS replica_identity
FROM pg_class
WHERE oid = 'my_view'::regclass;

and it returned nothing. But I'm wondering could it be any of the tables that the view uses that may have that id; I'm
notsure what REPLICA ID is used for, but our source DB for the dump has the the wal_level set to hot standby to sync
withanother server (same version) without using a dump (for failover/readonly report queries.) 

Reading this:
https://paquier.xyz/postgresql-2/postgres-9-4-feature-highlight-replica-identity-logical-replication/

and this

https://www.postgresql.org/docs/devel/sql-altertable.html

I'm not sure what config param would set that other than the wal_level, which in our case is hot standby not logical,
butit looks like 9.2 doesn't support that property and that could be causing the issue? Also, I see the replication
settingsin the conf file, but they are all defaulted to being commented out. 


So I'm still not sure what it could be. I'm in process of restoring the db from 9.5.5 to 9.5.18 at the moment to see if
itworks (currently "my_view" is still a table, I'm waiting for the restore to complete to see if when the rule is
applied,if it hasn't yet, that it shows as a view and returns records.") 

I'll see if I can extract the statements from another dump that doesn't use the Fc switches that we normally use, and
tryrunning them manually. 

Thanks again for your help!

Alex


Sent with ProtonMail Secure Email.

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Wednesday, October 9, 2019 1:01 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Alex Williams valenceshell@protonmail.com writes:
>
> > Can someone let me know when you're doing a pg_dump, can you specify not to use the view rules so that the
statementin the pg_dump file uses create view instead of create table/create rule? 
>
> No.
>
> > We dump from 9.5.5 and restore to one 9.5.18 server and two 9.2 servers....we've been doing this for awhile and had
noissues until recently with certain views that are trying to be restored with rule views (some views in the pg_dump
fileare created with create view and some by create table / create rule) 
>
> In general, we don't promise that pg_dump output from major version N
> can be loaded into previous major versions. Having said that, 9.2
> should not have a problem with either the CREATE VIEW or CREATE TABLE-
> plus-CREATE RULE approaches per se, so there's some critical detail
> that you haven't told us about. You didn't show the actual error
> messages, either.
>
> regards, tom lane

Sorry, there was a bit more that after reviewing again what I sent, I missed copying from the "CREATE TABLE" log:

pg_restore: [archiver (db)] Error from TOC entry 11240; 1259 42703182 TABLE my_view postgres
pg_restore: [archiver (db)] could not execute query: ERROR:  syntax error at or near "REPLICA"
LINE 19: ...E ONLY my_view REPLICA ID...
                                                              ^
    Command was: CREATE TABLE my_view (
    product character varying(255),
    product_id integer,
    payer...
pg_restore: [archiver (db)] could not execute query: ERROR:  relation "myschema.my_view" does not exist


Sent with ProtonMail Secure Email.

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Wednesday, October 9, 2019 5:32 PM, Alex Williams <valenceshell@protonmail.com> wrote:

> Hi Tom,
>
> Thanks for your reply, we appreciate it. This is a long reply, sorry about that, but if there's any specific I can
provideyou that helps, please let me know. 
>
> OK, for the log, we do this when restoring:
>
> pg_restore -d my_database -U postgres my_database.dump >restore_result.txt 2>&1
>
> but our log file only records the following (I've added more detail below using the cmds below.) The view name/column
nameshave changed for the mailing list: 
>
> "CREATE TABLE" - cat restore_result.txt | grep -A 10 -B 10 -i "CREATE TABLE" | more
>
> pg_restore: [archiver (db)] Error from TOC entry 11240; 1259 42703182 TABLE my_view postgres
> pg_restore: [archiver (db)] could not execute query: ERROR: syntax error at or near "REPLICA"
> LINE 19: ...E ONLY my_view REPLICA ID..
>
> pg_restore: [archiver (db)] could not execute query: ERROR: relation "myschema.my_view " does not exist
>
> "CREATE RULE" - cat restore_result.txt | grep -A 10 -B 10 -i "CREATE RULE" | more
> pg_restore: [archiver (db)] Error from TOC entry 87618; 2618 42703185 RULE _RETURN postgres
> pg_restore: [archiver (db)] could not execute query: ERROR: relation "my_view" does not exist
> Command was: CREATE RULE "_RETURN" AS
> ON SELECT TO my_view DO INSTEAD SELECT DISTINCT d.name AS p...
>
> We assumed it was the create rule but also looked at "REPLICA ID" and couldn't find anything on the properties that
ithad such a property ... we used the query from here:
https://stackoverflow.com/questions/55249431/find-replica-identity-for-a-postgres-table
>
> SELECT CASE relreplident
> WHEN 'd' THEN 'default'
> WHEN 'n' THEN 'nothing'
> WHEN 'f' THEN 'full'
> WHEN 'i' THEN 'index'
> END AS replica_identity
> FROM pg_class
> WHERE oid = 'my_view'::regclass;
>
> and it returned nothing. But I'm wondering could it be any of the tables that the view uses that may have that id;
I'mnot sure what REPLICA ID is used for, but our source DB for the dump has the the wal_level set to hot standby to
syncwith another server (same version) without using a dump (for failover/readonly report queries.) 
>
> Reading this:
> https://paquier.xyz/postgresql-2/postgres-9-4-feature-highlight-replica-identity-logical-replication/
>
> and this
>
> https://www.postgresql.org/docs/devel/sql-altertable.html
>
> I'm not sure what config param would set that other than the wal_level, which in our case is hot standby not logical,
butit looks like 9.2 doesn't support that property and that could be causing the issue? Also, I see the replication
settingsin the conf file, but they are all defaulted to being commented out. 
>
> So I'm still not sure what it could be. I'm in process of restoring the db from 9.5.5 to 9.5.18 at the moment to see
ifit works (currently "my_view" is still a table, I'm waiting for the restore to complete to see if when the rule is
applied,if it hasn't yet, that it shows as a view and returns records.") 
>
> I'll see if I can extract the statements from another dump that doesn't use the Fc switches that we normally use, and
tryrunning them manually. 
>
> Thanks again for your help!
>
> Alex
>
> Sent with ProtonMail Secure Email.
>
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Wednesday, October 9, 2019 1:01 AM, Tom Lane tgl@sss.pgh.pa.us wrote:
>
> > Alex Williams valenceshell@protonmail.com writes:
> >
> > > Can someone let me know when you're doing a pg_dump, can you specify not to use the view rules so that the
statementin the pg_dump file uses create view instead of create table/create rule? 
> >
> > No.
> >
> > > We dump from 9.5.5 and restore to one 9.5.18 server and two 9.2 servers....we've been doing this for awhile and
hadno issues until recently with certain views that are trying to be restored with rule views (some views in the
pg_dumpfile are created with create view and some by create table / create rule) 
> >
> > In general, we don't promise that pg_dump output from major version N
> > can be loaded into previous major versions. Having said that, 9.2
> > should not have a problem with either the CREATE VIEW or CREATE TABLE-
> > plus-CREATE RULE approaches per se, so there's some critical detail
> > that you haven't told us about. You didn't show the actual error
> > messages, either.
> > regards, tom lane

Ugh, sorry again, missed one more part, here is the full error for the create table in the log:

pg_restore: [archiver (db)] Error from TOC entry 11240; 1259 42703182 TABLE my_view postgres
pg_restore: [archiver (db)] could not execute query: ERROR:  syntax error at or near "REPLICA"
LINE 19: ...E ONLY my_view REPLICA ID...
                                                              ^
    Command was: CREATE TABLE my_view (
    product character varying(255),
    product_id integer,
    payer...
pg_restore: [archiver (db)] could not execute query: ERROR:  relation "myschema.my_view " does not exist
    Command was: ALTER TABLE myschema.my_view OWNER TO postgres;


But you can see, it doesn't show the whole statement, it uses an ellipses after a certain amount of lines/chars.

Sent with ProtonMail Secure Email.

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Wednesday, October 9, 2019 5:34 PM, Alex Williams <valenceshell@protonmail.com> wrote:

> Sorry, there was a bit more that after reviewing again what I sent, I missed copying from the "CREATE TABLE" log:
>
> pg_restore: [archiver (db)] Error from TOC entry 11240; 1259 42703182 TABLE my_view postgres
> pg_restore: [archiver (db)] could not execute query: ERROR: syntax error at or near "REPLICA"
> LINE 19: ...E ONLY my_view REPLICA ID...
> ^
> Command was: CREATE TABLE my_view (
> product character varying(255),
> product_id integer,
> payer...
> pg_restore: [archiver (db)] could not execute query: ERROR: relation "myschema.my_view" does not exist
>
> Sent with ProtonMail Secure Email.
>
> ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> On Wednesday, October 9, 2019 5:32 PM, Alex Williams valenceshell@protonmail.com wrote:
>
> > Hi Tom,
> > Thanks for your reply, we appreciate it. This is a long reply, sorry about that, but if there's any specific I can
provideyou that helps, please let me know. 
> > OK, for the log, we do this when restoring:
> > pg_restore -d my_database -U postgres my_database.dump >restore_result.txt 2>&1
> > but our log file only records the following (I've added more detail below using the cmds below.) The view
name/columnnames have changed for the mailing list: 
> > "CREATE TABLE" - cat restore_result.txt | grep -A 10 -B 10 -i "CREATE TABLE" | more
> > pg_restore: [archiver (db)] Error from TOC entry 11240; 1259 42703182 TABLE my_view postgres
> > pg_restore: [archiver (db)] could not execute query: ERROR: syntax error at or near "REPLICA"
> > LINE 19: ...E ONLY my_view REPLICA ID..
> > pg_restore: [archiver (db)] could not execute query: ERROR: relation "myschema.my_view " does not exist
> > "CREATE RULE" - cat restore_result.txt | grep -A 10 -B 10 -i "CREATE RULE" | more
> > pg_restore: [archiver (db)] Error from TOC entry 87618; 2618 42703185 RULE _RETURN postgres
> > pg_restore: [archiver (db)] could not execute query: ERROR: relation "my_view" does not exist
> > Command was: CREATE RULE "_RETURN" AS
> > ON SELECT TO my_view DO INSTEAD SELECT DISTINCT d.name AS p...
> > We assumed it was the create rule but also looked at "REPLICA ID" and couldn't find anything on the properties that
ithad such a property ... we used the query from here:
https://stackoverflow.com/questions/55249431/find-replica-identity-for-a-postgres-table
> > SELECT CASE relreplident
> > WHEN 'd' THEN 'default'
> > WHEN 'n' THEN 'nothing'
> > WHEN 'f' THEN 'full'
> > WHEN 'i' THEN 'index'
> > END AS replica_identity
> > FROM pg_class
> > WHERE oid = 'my_view'::regclass;
> > and it returned nothing. But I'm wondering could it be any of the tables that the view uses that may have that id;
I'mnot sure what REPLICA ID is used for, but our source DB for the dump has the the wal_level set to hot standby to
syncwith another server (same version) without using a dump (for failover/readonly report queries.) 
> > Reading this:
> > https://paquier.xyz/postgresql-2/postgres-9-4-feature-highlight-replica-identity-logical-replication/
> > and this
> > https://www.postgresql.org/docs/devel/sql-altertable.html
> > I'm not sure what config param would set that other than the wal_level, which in our case is hot standby not
logical,but it looks like 9.2 doesn't support that property and that could be causing the issue? Also, I see the
replicationsettings in the conf file, but they are all defaulted to being commented out. 
> > So I'm still not sure what it could be. I'm in process of restoring the db from 9.5.5 to 9.5.18 at the moment to
seeif it works (currently "my_view" is still a table, I'm waiting for the restore to complete to see if when the rule
isapplied, if it hasn't yet, that it shows as a view and returns records.") 
> > I'll see if I can extract the statements from another dump that doesn't use the Fc switches that we normally use,
andtry running them manually. 
> > Thanks again for your help!
> > Alex
> > Sent with ProtonMail Secure Email.
> > ‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
> > On Wednesday, October 9, 2019 1:01 AM, Tom Lane tgl@sss.pgh.pa.us wrote:
> >
> > > Alex Williams valenceshell@protonmail.com writes:
> > >
> > > > Can someone let me know when you're doing a pg_dump, can you specify not to use the view rules so that the
statementin the pg_dump file uses create view instead of create table/create rule? 
> > >
> > > No.
> > >
> > > > We dump from 9.5.5 and restore to one 9.5.18 server and two 9.2 servers....we've been doing this for awhile and
hadno issues until recently with certain views that are trying to be restored with rule views (some views in the
pg_dumpfile are created with create view and some by create table / create rule) 
> > >
> > > In general, we don't promise that pg_dump output from major version N
> > > can be loaded into previous major versions. Having said that, 9.2
> > > should not have a problem with either the CREATE VIEW or CREATE TABLE-
> > > plus-CREATE RULE approaches per se, so there's some critical detail
> > > that you haven't told us about. You didn't show the actual error
> > > messages, either.
> > > regards, tom lane

Alex Williams <valenceshell@protonmail.com> writes:
> Ugh, sorry again, missed one more part, here is the full error for the create table in the log:
> pg_restore: [archiver (db)] Error from TOC entry 11240; 1259 42703182 TABLE my_view postgres
> pg_restore: [archiver (db)] could not execute query: ERROR:  syntax error at or near "REPLICA"
> LINE 19: ...E ONLY my_view REPLICA ID...
>                                                               ^
>     Command was: CREATE TABLE my_view (
>     product character varying(255),
>     product_id integer,
>     payer...

This seems to be a chunk of a command like

ALTER TABLE ONLY my_view REPLICA IDENTITY FULL;

(or possibly REPLICA IDENTITY NOTHING), which pg_dump will emit if the
table has a non-default relreplident setting.  I do not, however,
understand your statement that this is a view.  AFAIK views should never
have non-default relreplident settings, and besides that, the TOC entry
description says it's a table not a view.  (If it's a materialized view,
it could have relreplident, but its TOC entry still shouldn't say TABLE.)

Anyway it's hardly surprising that 9.2 is choking on that syntax; it
doesn't have the REPLICA IDENTITY feature.

pg_dump actually is taking some pity on you here, in that it's emitting
this as a separate ALTER TABLE command, not as part of CREATE TABLE
directly.  This means you just need to get 9.2 to ignore the error
on the ALTER TABLE and keep plugging.  I think what you need to do
is something like pg_restore to stdout and then pipe stdout to psql,
rather than connecting directly to the target server.

Another fix, if this table was only accidentally labeled with
a replica identity (which I'm suspecting because you don't seem
to recognize the feature), is to get rid of the marking in the
source database:

ALTER TABLE ONLY my_view REPLICA IDENTITY DEFAULT;

            regards, tom lane

Hi Tom,

Thanks again for your quick reply! I've attached three images of the view:

1. The result of the alter table syntax you sent ( and it's definitely a view, actually, I created a few views in the
pastfew weeks, and they all get the same error when trying to restore.) 

2. The View definition

3. View info schema result

In text here, running this:
ALTER TABLE ONLY my_view REPLICA IDENTITY DEFAULT;

Returns:
ERROR:  "myschema.my_view" is not a table or materialized view
SQL state: 42809

Also, it's been about three hours so far into the restore on the server that is 9.5.18, but the restore of that view is
stilla table, hasn't changed to a view yet. I assumed it would run the DDL statements first, then the data copy and
possiblythe rules last, so I'm still waiting for the restore to complete to see if it changes. The 9.2.9 Server just
fails.

And running this: select * from INFORMATION_SCHEMA.views where table_name = 'my_view' returns the expected result.

I'll be away tomorrow, but will reply back on Friday with the result of your restore direction + the result of
zgreppingthe dump without the Fc switch that I have (sorry, didn't have a chance to do that yet.) 


Thanks again,

Alex




Sent with ProtonMail Secure Email.

‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
On Wednesday, October 9, 2019 6:46 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Alex Williams valenceshell@protonmail.com writes:
>
> > Ugh, sorry again, missed one more part, here is the full error for the create table in the log:
> > pg_restore: [archiver (db)] Error from TOC entry 11240; 1259 42703182 TABLE my_view postgres
> > pg_restore: [archiver (db)] could not execute query: ERROR: syntax error at or near "REPLICA"
> > LINE 19: ...E ONLY my_view REPLICA ID...
> > ^
> > Command was: CREATE TABLE my_view (
> > product character varying(255),
> > product_id integer,
> > payer...
>
> This seems to be a chunk of a command like
>
> ALTER TABLE ONLY my_view REPLICA IDENTITY FULL;
>
> (or possibly REPLICA IDENTITY NOTHING), which pg_dump will emit if the
> table has a non-default relreplident setting. I do not, however,
> understand your statement that this is a view. AFAIK views should never
> have non-default relreplident settings, and besides that, the TOC entry
> description says it's a table not a view. (If it's a materialized view,
> it could have relreplident, but its TOC entry still shouldn't say TABLE.)
>
> Anyway it's hardly surprising that 9.2 is choking on that syntax; it
> doesn't have the REPLICA IDENTITY feature.
>
> pg_dump actually is taking some pity on you here, in that it's emitting
> this as a separate ALTER TABLE command, not as part of CREATE TABLE
> directly. This means you just need to get 9.2 to ignore the error
> on the ALTER TABLE and keep plugging. I think what you need to do
> is something like pg_restore to stdout and then pipe stdout to psql,
> rather than connecting directly to the target server.
>
> Another fix, if this table was only accidentally labeled with
> a replica identity (which I'm suspecting because you don't seem
> to recognize the feature), is to get rid of the marking in the
> source database:
>
> ALTER TABLE ONLY my_view REPLICA IDENTITY DEFAULT;
>
> regards, tom lane

Alex Williams <valenceshell@protonmail.com> writes:
> [ gripes about pg_dump printing REPLICA IDENTITY NOTHING for a view ]

I spent a little bit of time trying to reproduce this, and indeed I can,
in versions before v10.

regression=# create table mytab (f1 int primary key, f2 text);
CREATE TABLE
regression=# create view myview as select * from mytab group by f1;
CREATE VIEW

This situation is problematic for pg_dump because validity of the
view depends on the existence of mytab's primary key constraint,
and we don't create primary keys till late in the restore process.
So it has to break myview into two parts, one to emit during normal
table/view creation and one to emit after index creation.

With 9.5's pg_dump, what comes out is:

--
-- Name: myview; Type: TABLE; Schema: public; Owner: postgres
--

CREATE TABLE public.myview (
    f1 integer,
    f2 text
);

ALTER TABLE ONLY public.myview REPLICA IDENTITY NOTHING;


ALTER TABLE public.myview OWNER TO postgres;

and then later:

--
-- Name: myview _RETURN; Type: RULE; Schema: public; Owner: postgres
--

CREATE RULE "_RETURN" AS
    ON SELECT TO public.myview DO INSTEAD  SELECT mytab.f1,
    mytab.f2
   FROM public.mytab
  GROUP BY mytab.f1;

The reason we get "REPLICA IDENTITY NOTHING" is that a view's relreplident
is set to 'n' not 'd', which might not have been a great choice.  But why
does pg_dump print anything --- it knows perfectly well that it should not
emit REPLICA IDENTITY for relkinds that don't have storage?  The answer
emerges from looking at the code that breaks the dependency loop:

    /* pretend view is a plain table and dump it that way */
    viewinfo->relkind = 'r';    /* RELKIND_RELATION */

After that, pg_dump *doesn't* know it's a view, which also explains
why the comment says TABLE not VIEW.

This is fixed in v10 and up thanks to d8c05aff5.  I was hesitant to
back-patch that at the time, but now that it's survived in the field
for a couple years, I think a good case could be made for doing so.
After a bit of looking around, the main argument I can find against
it is that emitting 'CREATE OR REPLACE VIEW' in a dropStmt will
break pg_restore versions preceding this commit:

Author: Tom Lane <tgl@sss.pgh.pa.us>
Branch: master Release: REL_10_BR [ac888986f] 2016-11-17 14:59:13 -0500
Branch: REL9_6_STABLE Release: REL9_6_2 [0eaa5118a] 2016-11-17 14:59:19 -0500
Branch: REL9_5_STABLE Release: REL9_5_6 [a7864037d] 2016-11-17 14:59:23 -0500
Branch: REL9_4_STABLE Release: REL9_4_11 [e69b532be] 2016-11-17 14:59:26 -0500

    Improve pg_dump/pg_restore --create --if-exists logic.
    
    Teach it not to complain if the dropStmt attached to an archive entry
    is actually spelled CREATE OR REPLACE VIEW, since that will happen due to
    an upcoming bug fix.  Also, if it doesn't recognize a dropStmt, have it
    print a WARNING and then emit the dropStmt unmodified.  That seems like a
    much saner behavior than Assert'ing or dumping core due to a null-pointer
    dereference, which is what would happen before :-(.
    
    Back-patch to 9.4 where this option was introduced.

AFAIR, we have not had complaints about back-rev pg_restore failing
on archives made by v10 and up; but perhaps it's more likely that
someone would try to use, say, 9.5.5 pg_restore with a dump made by
9.5.20 pg_dump.

An alternative that just responds to Alex's issue without fixing the
other problems d8c05aff5 fixed is to hack the dependency-loop code
like this:

    /* pretend view is a plain table and dump it that way */
    viewinfo->relkind = 'r';    /* RELKIND_RELATION */
    viewinfo->relkind = 'r';    /* RELKIND_RELATION */
+    viewinfo->relreplident = 'd';    /* REPLICA_IDENTITY_DEFAULT */

That's mighty ugly but it doesn't seem to carry any particular
risk.

Thoughts?

            regards, tom lane

Alex Williams <valenceshell@protonmail.com> writes:
> [ gripes about pg_dump printing REPLICA IDENTITY NOTHING for a view ]

I spent a little bit of time trying to reproduce this, and indeed I can,
in versions before v10.

regression=# create table mytab (f1 int primary key, f2 text);
CREATE TABLE
regression=# create view myview as select * from mytab group by f1;
CREATE VIEW

This situation is problematic for pg_dump because validity of the
view depends on the existence of mytab's primary key constraint,
and we don't create primary keys till late in the restore process.
So it has to break myview into two parts, one to emit during normal
table/view creation and one to emit after index creation.

With 9.5's pg_dump, what comes out is:

--
-- Name: myview; Type: TABLE; Schema: public; Owner: postgres
--

CREATE TABLE public.myview (
    f1 integer,
    f2 text
);

ALTER TABLE ONLY public.myview REPLICA IDENTITY NOTHING;


ALTER TABLE public.myview OWNER TO postgres;

and then later:

--
-- Name: myview _RETURN; Type: RULE; Schema: public; Owner: postgres
--

CREATE RULE "_RETURN" AS
    ON SELECT TO public.myview DO INSTEAD  SELECT mytab.f1,
    mytab.f2
   FROM public.mytab
  GROUP BY mytab.f1;

The reason we get "REPLICA IDENTITY NOTHING" is that a view's relreplident
is set to 'n' not 'd', which might not have been a great choice.  But why
does pg_dump print anything --- it knows perfectly well that it should not
emit REPLICA IDENTITY for relkinds that don't have storage?  The answer
emerges from looking at the code that breaks the dependency loop:

    /* pretend view is a plain table and dump it that way */
    viewinfo->relkind = 'r';    /* RELKIND_RELATION */

After that, pg_dump *doesn't* know it's a view, which also explains
why the comment says TABLE not VIEW.

This is fixed in v10 and up thanks to d8c05aff5.  I was hesitant to
back-patch that at the time, but now that it's survived in the field
for a couple years, I think a good case could be made for doing so.
After a bit of looking around, the main argument I can find against
it is that emitting 'CREATE OR REPLACE VIEW' in a dropStmt will
break pg_restore versions preceding this commit:

Author: Tom Lane <tgl@sss.pgh.pa.us>
Branch: master Release: REL_10_BR [ac888986f] 2016-11-17 14:59:13 -0500
Branch: REL9_6_STABLE Release: REL9_6_2 [0eaa5118a] 2016-11-17 14:59:19 -0500
Branch: REL9_5_STABLE Release: REL9_5_6 [a7864037d] 2016-11-17 14:59:23 -0500
Branch: REL9_4_STABLE Release: REL9_4_11 [e69b532be] 2016-11-17 14:59:26 -0500

    Improve pg_dump/pg_restore --create --if-exists logic.
    
    Teach it not to complain if the dropStmt attached to an archive entry
    is actually spelled CREATE OR REPLACE VIEW, since that will happen due to
    an upcoming bug fix.  Also, if it doesn't recognize a dropStmt, have it
    print a WARNING and then emit the dropStmt unmodified.  That seems like a
    much saner behavior than Assert'ing or dumping core due to a null-pointer
    dereference, which is what would happen before :-(.
    
    Back-patch to 9.4 where this option was introduced.

AFAIR, we have not had complaints about back-rev pg_restore failing
on archives made by v10 and up; but perhaps it's more likely that
someone would try to use, say, 9.5.5 pg_restore with a dump made by
9.5.20 pg_dump.

An alternative that just responds to Alex's issue without fixing the
other problems d8c05aff5 fixed is to hack the dependency-loop code
like this:

    /* pretend view is a plain table and dump it that way */
    viewinfo->relkind = 'r';    /* RELKIND_RELATION */
    viewinfo->relkind = 'r';    /* RELKIND_RELATION */
+    viewinfo->relreplident = 'd';    /* REPLICA_IDENTITY_DEFAULT */

That's mighty ugly but it doesn't seem to carry any particular
risk.

Thoughts?

            regards, tom lane

I wrote:
> Alex Williams <valenceshell@protonmail.com> writes:
>> [ gripes about pg_dump printing REPLICA IDENTITY NOTHING for a view ]

> This is fixed in v10 and up thanks to d8c05aff5.  I was hesitant to
> back-patch that at the time, but now that it's survived in the field
> for a couple years, I think a good case could be made for doing so.
> After a bit of looking around, the main argument I can find against
> it is that emitting 'CREATE OR REPLACE VIEW' in a dropStmt will
> break pg_restore versions preceding this commit:

> Author: Tom Lane <tgl@sss.pgh.pa.us>
> Branch: master Release: REL_10_BR [ac888986f] 2016-11-17 14:59:13 -0500
> Branch: REL9_6_STABLE Release: REL9_6_2 [0eaa5118a] 2016-11-17 14:59:19 -0500
> Branch: REL9_5_STABLE Release: REL9_5_6 [a7864037d] 2016-11-17 14:59:23 -0500
> Branch: REL9_4_STABLE Release: REL9_4_11 [e69b532be] 2016-11-17 14:59:26 -0500

>     Improve pg_dump/pg_restore --create --if-exists logic.

After further digging, I remembered that we bumped the archive file
version number in 3d2aed664 et al. to fix CVE-2018-1058.  So current
versions of pg_dump already emit archive files that will be rejected
by pg_restore versions preceding the above fix, and so there should be
no downside to emitting data that depends on it.

I'll go see about backpatching d8c05aff5.

            regards, tom lane

I wrote:
> Alex Williams <valenceshell@protonmail.com> writes:
>> [ gripes about pg_dump printing REPLICA IDENTITY NOTHING for a view ]

> This is fixed in v10 and up thanks to d8c05aff5.  I was hesitant to
> back-patch that at the time, but now that it's survived in the field
> for a couple years, I think a good case could be made for doing so.
> After a bit of looking around, the main argument I can find against
> it is that emitting 'CREATE OR REPLACE VIEW' in a dropStmt will
> break pg_restore versions preceding this commit:

> Author: Tom Lane <tgl@sss.pgh.pa.us>
> Branch: master Release: REL_10_BR [ac888986f] 2016-11-17 14:59:13 -0500
> Branch: REL9_6_STABLE Release: REL9_6_2 [0eaa5118a] 2016-11-17 14:59:19 -0500
> Branch: REL9_5_STABLE Release: REL9_5_6 [a7864037d] 2016-11-17 14:59:23 -0500
> Branch: REL9_4_STABLE Release: REL9_4_11 [e69b532be] 2016-11-17 14:59:26 -0500

>     Improve pg_dump/pg_restore --create --if-exists logic.

After further digging, I remembered that we bumped the archive file
version number in 3d2aed664 et al. to fix CVE-2018-1058.  So current
versions of pg_dump already emit archive files that will be rejected
by pg_restore versions preceding the above fix, and so there should be
no downside to emitting data that depends on it.

I'll go see about backpatching d8c05aff5.

            regards, tom lane

Hi,

On 2019-10-10 11:20:14 -0400, Tom Lane wrote:
> regression=# create table mytab (f1 int primary key, f2 text);
> CREATE TABLE
> regression=# create view myview as select * from mytab group by f1;
> CREATE VIEW
> 
> This situation is problematic for pg_dump because validity of the
> view depends on the existence of mytab's primary key constraint,
> and we don't create primary keys till late in the restore process.
> So it has to break myview into two parts, one to emit during normal
> table/view creation and one to emit after index creation.
> 
> With 9.5's pg_dump, what comes out is:
> 
> --
> -- Name: myview; Type: TABLE; Schema: public; Owner: postgres
> --
> 
> CREATE TABLE public.myview (
>     f1 integer,
>     f2 text
> );
> 
> ALTER TABLE ONLY public.myview REPLICA IDENTITY NOTHING;

Ick.


> The reason we get "REPLICA IDENTITY NOTHING" is that a view's relreplident
> is set to 'n' not 'd', which might not have been a great choice.

Hm, yea. I wonder if we should add a REPLICA_IDENTITY_INVALID or such,
for non relation relkinds?  I'm mildly inclined to think that setting it
to REPLICA_IDENTITY_DEFAULT is at least as confusing as
REPLICA_IDENTITY_DEFAULT...


> This is fixed in v10 and up thanks to d8c05aff5.  I was hesitant to
> back-patch that at the time, but now that it's survived in the field
> for a couple years, I think a good case could be made for doing so.

+1


>     /* pretend view is a plain table and dump it that way */
>     viewinfo->relkind = 'r';    /* RELKIND_RELATION */
>     viewinfo->relkind = 'r';    /* RELKIND_RELATION */
> +    viewinfo->relreplident = 'd';    /* REPLICA_IDENTITY_DEFAULT */
> 
> That's mighty ugly but it doesn't seem to carry any particular
> risk.

I also could live with this, given it'd only be in older back-branches.

Greetings,

Andres Freund

Hi,

On 2019-10-10 11:20:14 -0400, Tom Lane wrote:
> regression=# create table mytab (f1 int primary key, f2 text);
> CREATE TABLE
> regression=# create view myview as select * from mytab group by f1;
> CREATE VIEW
> 
> This situation is problematic for pg_dump because validity of the
> view depends on the existence of mytab's primary key constraint,
> and we don't create primary keys till late in the restore process.
> So it has to break myview into two parts, one to emit during normal
> table/view creation and one to emit after index creation.
> 
> With 9.5's pg_dump, what comes out is:
> 
> --
> -- Name: myview; Type: TABLE; Schema: public; Owner: postgres
> --
> 
> CREATE TABLE public.myview (
>     f1 integer,
>     f2 text
> );
> 
> ALTER TABLE ONLY public.myview REPLICA IDENTITY NOTHING;

Ick.


> The reason we get "REPLICA IDENTITY NOTHING" is that a view's relreplident
> is set to 'n' not 'd', which might not have been a great choice.

Hm, yea. I wonder if we should add a REPLICA_IDENTITY_INVALID or such,
for non relation relkinds?  I'm mildly inclined to think that setting it
to REPLICA_IDENTITY_DEFAULT is at least as confusing as
REPLICA_IDENTITY_DEFAULT...


> This is fixed in v10 and up thanks to d8c05aff5.  I was hesitant to
> back-patch that at the time, but now that it's survived in the field
> for a couple years, I think a good case could be made for doing so.

+1


>     /* pretend view is a plain table and dump it that way */
>     viewinfo->relkind = 'r';    /* RELKIND_RELATION */
>     viewinfo->relkind = 'r';    /* RELKIND_RELATION */
> +    viewinfo->relreplident = 'd';    /* REPLICA_IDENTITY_DEFAULT */
> 
> That's mighty ugly but it doesn't seem to carry any particular
> risk.

I also could live with this, given it'd only be in older back-branches.

Greetings,

Andres Freund

Andres Freund <andres@anarazel.de> writes:
> On 2019-10-10 11:20:14 -0400, Tom Lane wrote:
>> The reason we get "REPLICA IDENTITY NOTHING" is that a view's relreplident
>> is set to 'n' not 'd', which might not have been a great choice.

> Hm, yea. I wonder if we should add a REPLICA_IDENTITY_INVALID or such,
> for non relation relkinds?  I'm mildly inclined to think that setting it
> to REPLICA_IDENTITY_DEFAULT is at least as confusing as
> REPLICA_IDENTITY_DEFAULT...

Yeah, I'd be for that in HEAD probably.  But of course we can't change
the 9.x branches like that.

>> This is fixed in v10 and up thanks to d8c05aff5.  I was hesitant to
>> back-patch that at the time, but now that it's survived in the field
>> for a couple years, I think a good case could be made for doing so.

> +1

Just finishing up the back-patch now.

            regards, tom lane

Andres Freund <andres@anarazel.de> writes:
> On 2019-10-10 11:20:14 -0400, Tom Lane wrote:
>> The reason we get "REPLICA IDENTITY NOTHING" is that a view's relreplident
>> is set to 'n' not 'd', which might not have been a great choice.

> Hm, yea. I wonder if we should add a REPLICA_IDENTITY_INVALID or such,
> for non relation relkinds?  I'm mildly inclined to think that setting it
> to REPLICA_IDENTITY_DEFAULT is at least as confusing as
> REPLICA_IDENTITY_DEFAULT...

Yeah, I'd be for that in HEAD probably.  But of course we can't change
the 9.x branches like that.

>> This is fixed in v10 and up thanks to d8c05aff5.  I was hesitant to
>> back-patch that at the time, but now that it's survived in the field
>> for a couple years, I think a good case could be made for doing so.

> +1

Just finishing up the back-patch now.

            regards, tom lane