Re: table inheritance versus column compression and storage settings

On Tue, Dec 5, 2023 at 6:28 PM Peter Eisentraut <peter@eisentraut.org> wrote:
>
> On 05.12.23 05:26, Ashutosh Bapat wrote:
> >> - When inheriting from multiple parents with different settings, an
> >> explicit setting in the child is required.
> > When no explicit setting for child is specified, it will throw an
> > error as it does today. Right?
>
> Yes, it would throw an error, but a different error than today, saying
> something like "the settings in the parents conflict, so you need to
> specify one here to override the conflict".
>

PFA patch fixing inheritance and compression. It also fixes a crash
reported in [1].

The storage looks more involved. The way it has been coded, the child
always inherits the parent's storage properties.
#create table t1 (a text storage plain);
CREATE TABLE
#create table c1 (b text storage main) inherits(t1);
CREATE TABLE
#create table c1 (a text storage main) inherits(t1);
NOTICE:  merging column "a" with inherited definition
CREATE TABLE
#\d+ t1
                                          Table "public.t1"
 Column | Type | Collation | Nullable | Default | Storage |
Compression | Stats target | Description
--------+------+-----------+----------+---------+---------+-------------+--------------+-------------
 a      | text |           |          |         | plain   |
 |              |
Child tables: c1
Access method: heap
#\d+ c1
                                          Table "public.c1"
 Column | Type | Collation | Nullable | Default | Storage |
Compression | Stats target | Description
--------+------+-----------+----------+---------+---------+-------------+--------------+-------------
 a      | text |           |          |         | plain   |
 |              |
Inherits: t1
Access method: heap

Observe that c1.a did not have storage "main" but instead inherits
"plain" from t1.

According to the code at

https://github.com/postgres/postgres/blob/6a1ea02c491d16474a6214603dce40b5b122d4d1/src/backend/commands/tablecmds.c#L3253,
there is supposed to be a conflict error. But that does not happen
since child's storage specification is in ColumnDef::storage_name
which is never consulted. The ColumnDef::storage_name is converted to
ColumnDef::storage only in BuildDescForRelation(), after
MergeAttribute() has been finished. There are two ways to fix this
1. In MergeChildAttribute() resolve ColumnDef::storage_name to
ColumnDef::storage before comparing it against inherited property. I
don't like this approach since a. we duplicate the conversion logic in
MergeChildAttribute() and BuildDescForRelation(), b. the conversion
happens only for the attributes which are inherited.

2. Deal with it the same way as compression. Get rid of
ColumnDef::storage altogether. Instead set ColumnDef::storage_name at

https://github.com/postgres/postgres/blob/6a1ea02c491d16474a6214603dce40b5b122d4d1/src/backend/commands/tablecmds.c#L2723.

I am inclined to take the second approach. Let me know if you feel otherwise.

[1] https://www.postgresql.org/message-id/b22a6834-aacb-7b18-0424-a3f5fe889667@gmail.com

--
Best Wishes,
Ashutosh Bapat