Re: table inheritance versus column compression and storage settings - Mailing list pgsql-hackers

On Fri, Feb 16, 2024 at 11:54 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> I wrote:
> > I find it surprising that the committed patch does not touch
> > pg_dump.  Is it really true that pg_dump dumps situations with
> > differing compression/storage settings accurately already?
>
> It's worse than I thought.  Run "make installcheck" with
> today's HEAD, then:
>
> $ pg_dump -Fc regression >r.dump
> $ createdb r2
> $ pg_restore -d r2 r.dump
> pg_restore: error: could not execute query: ERROR:  column "a" inherits conflicting storage methods
> HINT:  To resolve the conflict, specify a storage method explicitly.
> Command was: CREATE TABLE public.stchild4 (
>     a text
> )
> INHERITS (public.stparent1, public.stparent2);
> ALTER TABLE ONLY public.stchild4 ALTER COLUMN a SET STORAGE MAIN;
>
>
> pg_restore: error: could not execute query: ERROR:  relation "public.stchild4" does not exist
> Command was: ALTER TABLE public.stchild4 OWNER TO postgres;
>
> pg_restore: error: could not execute query: ERROR:  relation "public.stchild4" does not exist
> Command was: COPY public.stchild4 (a) FROM stdin;
> pg_restore: warning: errors ignored on restore: 3

Thanks for the test. Let's call this Problem1. I expected
src/bin/pg_upgrade/t/002_pg_upgrade.pl to fail in this case since it
will execute similar steps as you did. And it actually does, except
that it uses binary-upgrade mode. In that mode, INHERITed tables are
dumped in a different manner
-- For binary upgrade, set up inheritance this way.
ALTER TABLE ONLY "public"."stchild4" INHERIT "public"."stparent1";
ALTER TABLE ONLY "public"."stchild4" INHERIT "public"."stparent2";
... snip ...
ALTER TABLE ONLY "public"."stchild4" ALTER COLUMN "a" SET STORAGE MAIN;

that does not lead to the conflict and pg_upgrade does not fail.

>
>
> What I'd intended to compare was the results of the query added to the
> regression tests:
>
> regression=# SELECT attrelid::regclass, attname, attstorage FROM pg_attribute
> WHERE (attrelid::regclass::name like 'stparent%'
> OR attrelid::regclass::name like 'stchild%')
> and attname = 'a'
> ORDER BY 1, 2;
>  attrelid  | attname | attstorage
> -----------+---------+------------
>  stparent1 | a       | p
>  stparent2 | a       | x
>  stchild1  | a       | p
>  stchild3  | a       | m
>  stchild4  | a       | m
>  stchild5  | a       | x
>  stchild6  | a       | m
> (7 rows)
>
> r2=# SELECT attrelid::regclass, attname, attstorage FROM pg_attribute
> WHERE (attrelid::regclass::name like 'stparent%'
> OR attrelid::regclass::name like 'stchild%')
> and attname = 'a'
> ORDER BY 1, 2;
>  attrelid  | attname | attstorage
> -----------+---------+------------
>  stparent1 | a       | p
>  stchild1  | a       | p
>  stchild3  | a       | m
>  stparent2 | a       | x
>  stchild5  | a       | p
>  stchild6  | a       | m
> (6 rows)
>
> So not only does stchild4 fail to restore altogether, but stchild5
> ends with the wrong attstorage.

With binary-upgrade dump and restore stchild5 gets the correct storage value.

Looks like we need a test which pg_dump s regression database and
restores it without going through pg_upgrade.

I think the fix is easy one. Dump the STORAGE and COMPRESSION clauses
with CREATE TABLE for local attributes. Those for inherited attributes
will be dumped separately.

But that will not fix an existing problem described below. Let's call
it Problem2. With HEAD  at commit
57f59396bb51953bb7b957780c7f1b7f67602125 (almost a month back)
$ createdb regression
$ psql -d regression
#create table par1 (a text storage plain);
#create table par2 (a text storage plain);
#create table chld (a text) inherits (par1, par2);
NOTICE:  merging multiple inherited definitions of column "a"
NOTICE:  merging column "a" with inherited definition
-- parent storages conflict after child creation
#alter table par1 alter column a set storage extended;
#SELECT attrelid::regclass, attname, attstorage FROM pg_attribute
 WHERE (attrelid::regclass::name like 'par%'
 OR attrelid::regclass::name like 'chld%')
 and attname = 'a'
 ORDER BY 1, 2;
 attrelid | attname | attstorage
----------+---------+------------
 par1     | a       | x
 par2     | a       | p
 chld     | a       | x
(3 rows)

$ createdb r2
$ pg_dump -Fc regression > /tmp/r.dump
$ pg_restore -d r2 /tmp/r.dump
pg_restore: error: could not execute query: ERROR:  inherited column
"a" has a storage parameter conflict
DETAIL:  EXTENDED versus PLAIN
Command was: CREATE TABLE public.chld (
    a text
)
INHERITS (public.par1, public.par2);

pg_restore: error: could not execute query: ERROR:  relation
"public.chld" does not exist
Command was: ALTER TABLE public.chld OWNER TO ashutosh;

pg_restore: error: could not execute query: ERROR:  relation
"public.chld" does not exist
Command was: COPY public.chld (a) FROM stdin;
pg_restore: warning: errors ignored on restore: 3

Fixing this requires that we dump ALTER TABLE ... ALTER COLUMN SET
STORAGE and COMPRESSION commands after all the tables (at least
children) have been created. That seems to break the way we dump the
whole table together right now. OR dump inherited tables like binary
upgrade mode.

--
Best Wishes,
Ashutosh Bapat