I find that pg_upgrade fails in HEAD when asked to do a 9.1-to-9.1
upgrade of the regression database. It gets to this bit of the
restore script:

CREATE TABLE test_tbl2 OF public.test_type2;

-- For binary upgrade, recreate dropped column.
UPDATE pg_catalog.pg_attribute
SET attlen = -1, attalign = 'i', attbyval = false
WHERE attname = '........pg.dropped.2........'
AND attrelid = 'test_tbl2'::pg_catalog.regclass;
ALTER TABLE ONLY test_tbl2 DROP COLUMN "........pg.dropped.2........";

and fails with

ERROR: cannot drop column from typed table

which probably is because test_type2 has a dropped column.

Somebody has failed to think through something, because if this state of
affairs was allowed to be created during the regression tests, why
should we not be able to restore it?

(pg_upgrade's ENUM support is broken too, but at least that one is a
one-line fix.)

regards, tom lane

Search Discussions

  • Bruce Momjian at Feb 10, 2011 at 4:16 am

    Tom Lane wrote:
    I find that pg_upgrade fails in HEAD when asked to do a 9.1-to-9.1
    upgrade of the regression database. It gets to this bit of the
    restore script:

    CREATE TABLE test_tbl2 OF public.test_type2;

    -- For binary upgrade, recreate dropped column.
    UPDATE pg_catalog.pg_attribute
    SET attlen = -1, attalign = 'i', attbyval = false
    WHERE attname = '........pg.dropped.2........'
    AND attrelid = 'test_tbl2'::pg_catalog.regclass;
    ALTER TABLE ONLY test_tbl2 DROP COLUMN "........pg.dropped.2........";

    and fails with

    ERROR: cannot drop column from typed table

    which probably is because test_type2 has a dropped column.

    Somebody has failed to think through something, because if this state of
    affairs was allowed to be created during the regression tests, why
    should we not be able to restore it?
    I am not aware of this code changing in 9.1. Was this test in 9.0?
    Does this problem happen for 9.0?

    --
    Bruce Momjian <bruce@momjian.us> http://momjian.us
    EnterpriseDB http://enterprisedb.com

    + It's impossible for everything to be true. +
  • Peter Eisentraut at Feb 10, 2011 at 4:32 am

    On ons, 2011-02-09 at 23:16 -0500, Bruce Momjian wrote:
    I am not aware of this code changing in 9.1. Was this test in 9.0?
    Does this problem happen for 9.0?
    No, because you can't drop anything from a typed table in 9.0.
  • Peter Eisentraut at Feb 10, 2011 at 4:31 am

    On ons, 2011-02-09 at 18:43 -0500, Tom Lane wrote:
    I find that pg_upgrade fails in HEAD when asked to do a 9.1-to-9.1
    upgrade of the regression database. It gets to this bit of the
    restore script:

    CREATE TABLE test_tbl2 OF public.test_type2;

    -- For binary upgrade, recreate dropped column.
    UPDATE pg_catalog.pg_attribute
    SET attlen = -1, attalign = 'i', attbyval = false
    WHERE attname = '........pg.dropped.2........'
    AND attrelid = 'test_tbl2'::pg_catalog.regclass;
    ALTER TABLE ONLY test_tbl2 DROP COLUMN "........pg.dropped.2........";

    and fails with

    ERROR: cannot drop column from typed table

    which probably is because test_type2 has a dropped column.
    It should call

    ALTER TYPE test_type2 DROP ATTRIBUTE xyz CASCADE;

    instead. That will propagate to the table.

    I'm not sure though, whether a composite type preserves the dropped
    attribute for re-dropping in this case.
  • Peter Eisentraut at Mar 30, 2011 at 4:50 pm

    On tor, 2011-02-10 at 06:31 +0200, Peter Eisentraut wrote:
    ERROR: cannot drop column from typed table

    which probably is because test_type2 has a dropped column.
    It should call

    ALTER TYPE test_type2 DROP ATTRIBUTE xyz CASCADE;

    instead. That will propagate to the table.
    Here is a patch that addresses this problem.

    It looks like Noah Misch might have found another problem in this area.
    We'll have to investigate that.
  • Noah Misch at Mar 31, 2011 at 1:32 am

    On Wed, Mar 30, 2011 at 07:50:12PM +0300, Peter Eisentraut wrote:
    On tor, 2011-02-10 at 06:31 +0200, Peter Eisentraut wrote:
    ERROR: cannot drop column from typed table

    which probably is because test_type2 has a dropped column.
    It should call

    ALTER TYPE test_type2 DROP ATTRIBUTE xyz CASCADE;

    instead. That will propagate to the table.
    Here is a patch that addresses this problem.
    This only works when exactly one typed table uses each composite type having
    dropped columns. With zero users, the placeholder column never gets dropped.
    Actually, it happens to work for >1 user, but only because ALTER TYPE mistakenly
    only touches the first table-of-type:

    create type t as (x int, y int);
    create table is_a of t;
    create table is_a2 of t;
    alter type t drop attribute y cascade, add attribute z int cascade;
    \d is_a
    Table "public.is_a"
    Column | Type | Modifiers
    --------+---------+-----------
    x | integer |
    z | integer |
    Typed table of type: t
    \d is_a2
    Table "public.is_a2"
    Column | Type | Modifiers
    --------+---------+-----------
    x | integer |
    y | integer |
    Typed table of type: t

    Might be a simple fix; looks like find_typed_table_dependencies() only grabs the
    first match. Incidentally, this led me to notice that you can hang a typed
    table off a table row type. ALTER TABLE never propagates to such typed tables,
    allowing them to get out of sync:

    create table t (x int, y int);
    create table is_a of t;
    create table is_a2 of t;
    alter table t drop y, add z int;
    \d is_a
    Table "public.is_a"
    Column | Type | Modifiers
    --------+---------+-----------
    x | integer |
    y | integer |
    Typed table of type: t

    Perhaps we should disallow the use of table row types in CREATE TABLE ... OF?
    It looks like Noah Misch might have found another problem in this area.
    We'll have to investigate that.
    Your bits in dumpCompositeType() are most of what's needed to fix that, I think.

    Thanks,
    nm
  • Robert Haas at Apr 5, 2011 at 1:45 pm

    On Wed, Mar 30, 2011 at 9:32 PM, Noah Misch wrote:
    On Wed, Mar 30, 2011 at 07:50:12PM +0300, Peter Eisentraut wrote:
    On tor, 2011-02-10 at 06:31 +0200, Peter Eisentraut wrote:
    ERROR:  cannot drop column from typed table

    which probably is because test_type2 has a dropped column.
    It should call

    ALTER TYPE test_type2 DROP ATTRIBUTE xyz CASCADE;

    instead.  That will propagate to the table.
    Here is a patch that addresses this problem.
    This only works when exactly one typed table uses each composite type having
    dropped columns.  With zero users, the placeholder column never gets dropped.
    Actually, it happens to work for >1 user, but only because ALTER TYPE mistakenly
    only touches the first table-of-type:

    create type t as (x int, y int);
    create table is_a of t;
    create table is_a2 of t;
    alter type t drop attribute y cascade, add attribute z int cascade;
    \d is_a
    Table "public.is_a"
    Column |  Type   | Modifiers
    --------+---------+-----------
    x      | integer |
    z      | integer |
    Typed table of type: t
    \d is_a2
    Table "public.is_a2"
    Column |  Type   | Modifiers
    --------+---------+-----------
    x      | integer |
    y      | integer |
    Typed table of type: t

    Might be a simple fix; looks like find_typed_table_dependencies() only grabs the
    first match.  Incidentally, this led me to notice that you can hang a typed
    table off a table row type.  ALTER TABLE never propagates to such typed tables,
    allowing them to get out of sync:

    create table t (x int, y int);
    create table is_a of t;
    create table is_a2 of t;
    alter table t drop y, add z int;
    \d is_a
    Table "public.is_a"
    Column |  Type   | Modifiers
    --------+---------+-----------
    x      | integer |
    y      | integer |
    Typed table of type: t

    Perhaps we should disallow the use of table row types in CREATE TABLE ... OF?
    It looks like Noah Misch might have found another problem in this area.
    We'll have to investigate that.
    Your bits in dumpCompositeType() are most of what's needed to fix that, I think.
    Where are we on this?

    --
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
  • Noah Misch at Apr 6, 2011 at 3:57 pm

    On Tue, Apr 05, 2011 at 09:44:44AM -0400, Robert Haas wrote:
    On Wed, Mar 30, 2011 at 9:32 PM, Noah Misch wrote:
    On Wed, Mar 30, 2011 at 07:50:12PM +0300, Peter Eisentraut wrote:
    Here is a patch that addresses this problem.
    This only works when exactly one typed table uses each composite type having
    dropped columns. ?With zero users, the placeholder column never gets dropped.
    Actually, it happens to work for >1 user, but only because ALTER TYPE mistakenly
    only touches the first table-of-type:

    create type t as (x int, y int);
    create table is_a of t;
    create table is_a2 of t;
    alter type t drop attribute y cascade, add attribute z int cascade;
    \d is_a
    ? ? Table "public.is_a"
    ?Column | ?Type ? | Modifiers
    --------+---------+-----------
    ?x ? ? ?| integer |
    ?z ? ? ?| integer |
    Typed table of type: t
    \d is_a2
    ? ? Table "public.is_a2"
    ?Column | ?Type ? | Modifiers
    --------+---------+-----------
    ?x ? ? ?| integer |
    ?y ? ? ?| integer |
    Typed table of type: t

    Might be a simple fix; looks like find_typed_table_dependencies() only grabs the
    first match. ?Incidentally, this led me to notice that you can hang a typed
    table off a table row type. ?ALTER TABLE never propagates to such typed tables,
    allowing them to get out of sync:

    create table t (x int, y int);
    create table is_a of t;
    create table is_a2 of t;
    alter table t drop y, add z int;
    \d is_a
    ? ? Table "public.is_a"
    ?Column | ?Type ? | Modifiers
    --------+---------+-----------
    ?x ? ? ?| integer |
    ?y ? ? ?| integer |
    Typed table of type: t

    Perhaps we should disallow the use of table row types in CREATE TABLE ... OF?
    It looks like Noah Misch might have found another problem in this area.
    We'll have to investigate that.
    Your bits in dumpCompositeType() are most of what's needed to fix that, I think.
    Where are we on this?
    Peter, were you planning to complete this? I can take a swing at it, if it
    would be helpful.
  • Peter Eisentraut at Apr 7, 2011 at 9:25 pm

    On ons, 2011-04-06 at 11:49 -0400, Noah Misch wrote:
    Peter, were you planning to complete this? I can take a swing at it, if it
    would be helpful.
    Help is always welcome.
  • Peter Eisentraut at Apr 27, 2011 at 6:31 pm
    Here is the patch to fix that, as discussed.
  • Tom Lane at Apr 27, 2011 at 6:43 pm

    Peter Eisentraut writes:
    Here is the patch to fix that, as discussed.
    Looks sane --- I assume you tested it against the originally
    complained-of scenario?
    http://archives.postgresql.org/message-id/201103111328.p2BDSFd10499@momjian.us

    If so, please apply soon --- we need to wrap beta1 this evening.

    regards, tom lane
  • Noah Misch at Apr 27, 2011 at 7:52 pm

    On Wed, Apr 27, 2011 at 09:30:41PM +0300, Peter Eisentraut wrote:
    Here is the patch to fix that, as discussed.
    Looks correct. Thanks.
  • Noah Misch at Apr 8, 2011 at 11:53 am

    On Wed, Mar 30, 2011 at 09:32:08PM -0400, Noah Misch wrote:
    ... ALTER TYPE mistakenly
    only touches the first table-of-type:

    create type t as (x int, y int);
    create table is_a of t;
    create table is_a2 of t;
    alter type t drop attribute y cascade, add attribute z int cascade;
    \d is_a
    Table "public.is_a"
    Column | Type | Modifiers
    --------+---------+-----------
    x | integer |
    z | integer |
    Typed table of type: t
    \d is_a2
    Table "public.is_a2"
    Column | Type | Modifiers
    --------+---------+-----------
    x | integer |
    y | integer |
    Typed table of type: t

    Might be a simple fix; looks like find_typed_table_dependencies() only grabs the
    first match.
    This is a fairly independent one-liner, so here's that patch. I didn't
    incorporate the test case, because it seems distinctly unlikely to recur.
  • Robert Haas at Apr 8, 2011 at 7:43 pm

    On Wed, Mar 30, 2011 at 9:32 PM, Noah Misch wrote:
    Incidentally, this led me to notice that you can hang a typed
    table off a table row type.  ALTER TABLE never propagates to such typed tables,
    allowing them to get out of sync:

    create table t (x int, y int);
    create table is_a of t;
    create table is_a2 of t;
    alter table t drop y, add z int;
    \d is_a
    Table "public.is_a"
    Column |  Type   | Modifiers
    --------+---------+-----------
    x      | integer |
    y      | integer |
    Typed table of type: t

    Perhaps we should disallow the use of table row types in CREATE TABLE ... OF?
    Yes, I think we need to do that.
    It looks like Noah Misch might have found another problem in this area.
    We'll have to investigate that.
    Your bits in dumpCompositeType() are most of what's needed to fix that, I think.
    Most?

    --
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
  • Noah Misch at Apr 8, 2011 at 8:33 pm

    On Fri, Apr 08, 2011 at 03:43:39PM -0400, Robert Haas wrote:
    On Wed, Mar 30, 2011 at 9:32 PM, Noah Misch wrote:
    Incidentally, this led me to notice that you can hang a typed
    table off a table row type. ?ALTER TABLE never propagates to such typed tables,
    allowing them to get out of sync:

    create table t (x int, y int);
    create table is_a of t;
    create table is_a2 of t;
    alter table t drop y, add z int;
    \d is_a
    ? ? Table "public.is_a"
    ?Column | ?Type ? | Modifiers
    --------+---------+-----------
    ?x ? ? ?| integer |
    ?y ? ? ?| integer |
    Typed table of type: t

    Perhaps we should disallow the use of table row types in CREATE TABLE ... OF?
    Yes, I think we need to do that.
    Having thought about it some more, that would be unfortunate. We rarely
    distinguish between table row types and CREATE TYPE AS types. Actually, I'm not
    aware of any place we distinguish other than in ALTER TABLE/ALTER TYPE, to
    instruct you to use the other.

    But depending on how hard it is to fix, that might be a good stopgap.
    It looks like Noah Misch might have found another problem in this area.
    We'll have to investigate that.
    Your bits in dumpCompositeType() are most of what's needed to fix that, I think.
    Most?
    I think it will just fall out of the completed fix for the original reported
    problem. Will keep you posted.

    nm
  • Robert Haas at Apr 8, 2011 at 8:06 pm

    On Wed, Mar 30, 2011 at 12:50 PM, Peter Eisentraut wrote:
    On tor, 2011-02-10 at 06:31 +0200, Peter Eisentraut wrote:
    ERROR:  cannot drop column from typed table

    which probably is because test_type2 has a dropped column.
    It should call

    ALTER TYPE test_type2 DROP ATTRIBUTE xyz CASCADE;

    instead.  That will propagate to the table.
    Here is a patch that addresses this problem.

    It looks like Noah Misch might have found another problem in this area.
    We'll have to investigate that.
    There's something wrong with this patch - it never arranges to
    actually drop the phony column. Consider:

    create type foo as (a int, b int);
    alter table foo drop attribute b;
    create table x (a int, b int);
    alter table x drop column b;

    Then pg_dump --binary-upgrade emits, in relevant part, the following for x:

    CREATE TABLE x (
    a integer,
    "........pg.dropped.2........" INTEGER /* dummy */
    );

    -- For binary upgrade, recreate dropped column.
    UPDATE pg_catalog.pg_attribute
    SET attlen = 4, attalign = 'i', attbyval = false
    WHERE attname = '........pg.dropped.2........'
    AND attrelid IN ('x'::pg_catalog.regclass);
    ALTER TABLE ONLY x DROP COLUMN "........pg.dropped.2........";

    But for t we get only:

    CREATE TYPE foo AS (
    a integer,
    "........pg.dropped.2........" INTEGER /* dummy */
    );

    ...which is no good.

    --
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
  • Bruce Momjian at Mar 11, 2011 at 1:28 pm
    Is this still an open bug?

    ---------------------------------------------------------------------------

    Tom Lane wrote:
    I find that pg_upgrade fails in HEAD when asked to do a 9.1-to-9.1
    upgrade of the regression database. It gets to this bit of the
    restore script:

    CREATE TABLE test_tbl2 OF public.test_type2;

    -- For binary upgrade, recreate dropped column.
    UPDATE pg_catalog.pg_attribute
    SET attlen = -1, attalign = 'i', attbyval = false
    WHERE attname = '........pg.dropped.2........'
    AND attrelid = 'test_tbl2'::pg_catalog.regclass;
    ALTER TABLE ONLY test_tbl2 DROP COLUMN "........pg.dropped.2........";

    and fails with

    ERROR: cannot drop column from typed table

    which probably is because test_type2 has a dropped column.

    Somebody has failed to think through something, because if this state of
    affairs was allowed to be created during the regression tests, why
    should we not be able to restore it?

    (pg_upgrade's ENUM support is broken too, but at least that one is a
    one-line fix.)

    regards, tom lane
    --
    Bruce Momjian <bruce@momjian.us> http://momjian.us
    EnterpriseDB http://enterprisedb.com

    + It's impossible for everything to be true. +
  • Robert Haas at Mar 27, 2011 at 2:19 am

    On Fri, Mar 11, 2011 at 8:28 AM, Bruce Momjian wrote:
    Is this still an open bug?
    Is anyone working on fixing this?

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

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
grouppgsql-hackers @
categoriespostgresql
postedFeb 9, '11 at 11:43p
activeApr 27, '11 at 7:52p
posts18
users5
websitepostgresql.org...
irc#postgresql

People

Translate

site design / logo © 2022 Grokbase