Thread: BUG #9555: pg_dump for tables with inheritance recreates the table with the wrong order of columns

The following bug has been logged on the website:

Bug reference:      9555
Logged by:          Fazal Majid
Email address:      majid@apsalar.com
PostgreSQL version: 9.2.6
Operating system:   OpenIndiana b151 Solaris x64
Description:

Reproduction case:

create table A(a int, b int, c int);
create table B(a int, c int);
alter table A inherit B;

pg_dump (whether in default or -Fc mode) generates:

CREATE TABLE a (
    a integer,
    b integer,
    c integer
)
INHERITS (b);

and after pg_restore, the order of columns is inverted, as if the table
were:

CREATE TABLE a (
    a integer,
    c integer,
    b intege,
)

This causes things like unqualified INSERT INTO statements to start failing
(bad practice, I know).

The solution would be to generate CREATE TABLE followed by ALTER TABLE
INHERIT rather than CREATE TABLE ... INHERITS.

majid@apsalar.com writes:
> create table A(a int, b int, c int);
> create table B(a int, c int);
> alter table A inherit B;

> pg_dump (whether in default or -Fc mode) generates [ A with a,c,b ]

This is not a bug, but expected and desired behavior.  The parent's column
order is considered canonical, and child tables are made to append their
columns to that.  Perhaps in your use-case that's not how you'd like to
think about it, but if we were to change this we'd doubtless break things
for other people.

Possibly you could use pg_upgrade, which I believe does preserve physical
column order in such cases.

            regards, tom lane

majid@apsalar.com wrote:

> Reproduction case:
>
> create table A(a int, b int, c int);
> create table B(a int, c int);
> alter table A inherit B;

I wonder if the real fix here is to have ALTER / INHERIT error out of
the columns in B are not a prefix of those in A.

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

Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> majid@apsalar.com wrote:
>> Reproduction case:
>>
>> create table A(a int, b int, c int);
>> create table B(a int, c int);
>> alter table A inherit B;

> I wonder if the real fix here is to have ALTER / INHERIT error out of
> the columns in B are not a prefix of those in A.

Years ago, we sweated quite a lot of blood to make these cases work.
I'm not thrilled about throwing away all that effort because one person
doesn't like the behavior.

            regards, tom lane

Tom Lane-2 wrote
> Alvaro Herrera <

> alvherre@

> > writes:
>>

> majid@

>  wrote:
>>> Reproduction case:
>>>
>>> create table A(a int, b int, c int);
>>> create table B(a int, c int);
>>> alter table A inherit B;
>
>> I wonder if the real fix here is to have ALTER / INHERIT error out of
>> the columns in B are not a prefix of those in A.
>
> Years ago, we sweated quite a lot of blood to make these cases work.
> I'm not thrilled about throwing away all that effort because one person
> doesn't like the behavior.

At least a warning is in order so that the user at least has chance to know
they are doing something inconsistent and can affect a manual refactoring of
their create/alter table commands to make them consistent.

Adding something like the following after the first paragraph in the inherit
clause of alter table would seem wise.

"The columns need not be in the same order, or in the leading position, in
the child as in the parent but if they are not then during a dump-restore
the resultant CREATE TABLE will physically re-order the columns.  This will
affect any code that references the columns of this table using * (implied -
via insert - or otherwise)."

I imagine the rules are a little more complicated when more than one parent
is involved.  The wording in CREATE TABLE is goes into great detail as to
how column names must be presented but do not comment of column ordering at
all.  Passing around and using table references is not that uncommon that
the impact of inherent on column ordering should be ignored.

In the case of create table are there cases, without an intervening "alter
table ...inherits" that the initial create and the one run after
dump/restore could result in a different physical order?  The main risk area
is adding/removing columns from the parent(s).

Without actually predicting the order would it at least be possible to run a
check of any inheritance trees affected by any kind of change to see if the
current column order would match the order resulting from a dump/restore and
emit a warning if a discrepancy is found?

Pg_upgrade may work but at minimum recovery is still broken as is creating
copies for development purposes.

David J.





--
View this message in context:
http://postgresql.1045698.n5.nabble.com/BUG-9555-pg-dump-for-tables-with-inheritance-recreates-the-table-with-the-wrong-order-of-columns-tp5795882p5795953.html
Sent from the PostgreSQL - bugs mailing list archive at Nabble.com.

On Mar 13, 2014, at 7:19, Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
>> majid@apsalar.com wrote:
>>> Reproduction case:
>>>=20
>>> create table A(a int, b int, c int);
>>> create table B(a int, c int);
>>> alter table A inherit B;
>=20
>> I wonder if the real fix here is to have ALTER / INHERIT error out of
>> the columns in B are not a prefix of those in A.
>=20
> Years ago, we sweated quite a lot of blood to make these cases work.
> I'm not thrilled about throwing away all that effort because one =
person
> doesn't like the behavior.

That makes sense, but then it would make sense to document this behavior =
under ALTER TABLE ... INHERIT, and possibly change its behavior so it =
reorders the columns on the source database=92s data dictionary (I am =
not sure whether the logical column order has to match the physical =
order already embedded in the data files).

Thanks,

--
Fazal Majid
CTO, Apsalar Inc.

Tom Lane wrote:
> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> > majid@apsalar.com wrote:
> >> Reproduction case:
> >>
> >> create table A(a int, b int, c int);
> >> create table B(a int, c int);
> >> alter table A inherit B;
>
> > I wonder if the real fix here is to have ALTER / INHERIT error out of
> > the columns in B are not a prefix of those in A.
>
> Years ago, we sweated quite a lot of blood to make these cases work.
> I'm not thrilled about throwing away all that effort because one person
> doesn't like the behavior.

Hm, well in that case it makes sense to consider the original
suggestion: if the columns in the parent are not a prefix of those of
the child, use ALTER INHERIT after creating both tables rather than
CREATE TABLE INHERITS.

It'd be a lot of new code in pg_dump though.  I am not volunteering ...

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

On Fri, Mar 14, 2014 at 12:33:04PM -0300, Alvaro Herrera wrote:
> Tom Lane wrote:
> > Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> > > majid@apsalar.com wrote:
> > >> Reproduction case:
> > >>
> > >> create table A(a int, b int, c int);
> > >> create table B(a int, c int);
> > >> alter table A inherit B;
> >
> > > I wonder if the real fix here is to have ALTER / INHERIT error out of
> > > the columns in B are not a prefix of those in A.
> >
> > Years ago, we sweated quite a lot of blood to make these cases work.
> > I'm not thrilled about throwing away all that effort because one person
> > doesn't like the behavior.

Agreed.  That also makes the current pg_dump behavior a bug.  Column order
matters; pg_dump is failing to recreate a semantically-equivalent database.

> Hm, well in that case it makes sense to consider the original
> suggestion: if the columns in the parent are not a prefix of those of
> the child, use ALTER INHERIT after creating both tables rather than
> CREATE TABLE INHERITS.
>
> It'd be a lot of new code in pg_dump though.  I am not volunteering ...

"pg_dump --binary-upgrade" already gets this right.  Perhaps it won't take too
much code to make dumpTableSchema() reuse that one part of its binary-upgrade
approach whenever the columns of B are not a prefix of those in A.

nm

--
Noah Misch
EnterpriseDB                                 http://www.enterprisedb.com