8000 Fix ALTER TABLE / SET TYPE for irregular inheritance · postgrespro/postgres@3957b58 · GitHub
[go: up one dir, main page]
Skip to content

    • Commit 3957b58

    Browse files
    alvherrealvherre
    committed
    Fix ALTER TABLE / SET TYPE for irregular inheritance
    If inherited tables don't have exactly the same schema, the USING clause in an ALTER TABLE / SET DATA TYPE misbehaves when applied to the children tables since commit 9550e83. Starting with that commit, the attribute numbers in the USING expression are fixed during parse analysis. This can lead to bogus errors being reported during execution, such as: ERROR: attribute 2 has wrong type DETAIL: Table has type smallint, but query expects integer. Since it wouldn't do to revert to the original coding, we now apply a transformation to map the attribute numbers to the correct ones for each child. Reported by Justin Pryzby Analysis by Tom Lane; patch by me. Discussion: https://postgr.es/m/20170102225618.GA10071@telsasoft.com
    1 parent 7403561 commit 3957b58

    File tree

    5 files changed

    +175
    -49
    lines changed

    5 files changed

    +175
    -49
    lines changed

    ‎src/backend/access/common/tupconvert.c

    Lines changed: 65 additions & 45 deletions
    Original file line numberDiff line numberDiff line change
    @@ -206,55 +206,12 @@ convert_tuples_by_name(TupleDesc indesc,
    206206
    {
    207207
    TupleConversionMap *map;
    208208
    AttrNumber *attrMap;
    209-
    int n;
    209+
    int n = outdesc->natts;
    210210
    int i;
    211211
    bool same;
    212212

    213213
    /* Verify compatibility and prepare attribute-number map */
    214-
    n = outdesc->natts;
    215-
    attrMap = (AttrNumber *) palloc0(n * sizeof(AttrNumber));
    216-
    for (i = 0; i < n; i++)
    217-
    {
    218-
    Form_pg_attribute att = outdesc->attrs[i];
    219-
    char *attname;
    220-
    Oid atttypid;
    221-
    int32 atttypmod;
    222-
    int j;
    223-
    224-
    if (att->attisdropped)
    225-
    continue; /* attrMap[i] is already 0 */
    226-
    attname = NameStr(att->attname);
    227-
    atttypid = att->atttypid;
    228-
    atttypmod = att->atttypmod;
    229-
    for (j = 0; j < indesc->natts; j++)
    230-
    {
    231-
    att = indesc->attrs[j];
    232-
    if (att->attisdropped)
    233-
    continue;
    234-
    if (strcmp(attname, NameStr(att->attname)) == 0)
    235-
    {
    236-
    /* Found it, check type */
    237-
    if (atttypid != att->atttypid || atttypmod != att->atttypmod)
    238-
    ereport(ERROR,
    239-
    (errcode(ERRCODE_DATATYPE_MISMATCH),
    240-
    errmsg_internal("%s", _(msg)),
    241-
    errdetail("Attribute \"%s\" of type %s does not match corresponding attribute of type %s.",
    242-
    attname,
    243-
    format_type_be(outdesc->tdtypeid),
    244-
    format_type_be(indesc->tdtypeid))));
    245-
    attrMap[i] = (AttrNumber) (j + 1);
    246-
    break;
    247-
    }
    248-
    }
    249-
    if (attrMap[i] == 0)
    250-
    ereport(ERROR,
    251-
    (errcode(ERRCODE_DATATYPE_MISMATCH),
    252-
    errmsg_internal("%s", _(msg)),
    253-
    errdetail("Attribute \"%s\" of type %s does not exist in type %s.",
    254-
    attname,
    255-
    format_type_be(outdesc->tdtypeid),
    256-
    format_type_be(indesc->tdtypeid))));
    257-
    }
    214+
    attrMap = convert_tuples_by_name_map(indesc, outdesc, msg);
    258215

    259216
    /*
    260217
    * Check to see if the map is one-to-one and the tuple types are the same.
    @@ -312,6 +269,69 @@ convert_tuples_by_name(TupleDesc indesc,
    312269
    return map;
    313270
    }
    314271

    272+
    /*
    273+
    * Return a palloc'd bare attribute map for tuple conversion, matching input
    274+
    * and output columns by name. (Dropped columns are ignored in both input and
    275+
    * output.) This is normally a subroutine for convert_tuples_by_name, but can
    276+
    * be used standalone.
    277+
    */
    278+
    AttrNumber *
    279+
    convert_tuples_by_name_map(TupleDesc indesc,
    280+
    TupleDesc outdesc,
    281+
    const char *msg)
    282+
    {
    283+
    AttrNumber *attrMap;
    284+
    int n;
    285+
    int i;
    286+
    287+
    n = outdesc->natts;
    288+
    attrMap = (AttrNumber *) palloc0(n * sizeof(AttrNumber));
    289+
    for (i = 0; i < n; i++)
    290+
    {
    291+
    Form_pg_attribute att = outdesc->attrs[i];
    292+
    char *attname;
    293+
    Oid atttypid;
    294+
    int32 atttypmod;
    295+
    int j;
    296+
    297+
    if (att->attisdropped)
    298+
    continue; /* attrMap[i] is already 0 */
    299+
    attname = NameStr(att->attname);
    300+
    atttypid = att->atttypid;
    301+
    atttypmod = att->atttypmod;
    302+
    for (j = 0; j < indesc->natts; j++)
    303+
    {
    304+
    att = indesc->attrs[j];
    305+
    if (att->attisdropped)
    306+
    continue;
    307+
    if (strcmp(attname, NameStr(att->attname)) == 0)
    308+
    {
    309+
    /* Found it, check type */
    310+
    if (atttypid != att->atttypid || atttypmod != att->atttypmod)
    311+
    ereport(ERROR,
    312+
    (errcode(ERRCODE_DATATYPE_MISMATCH),
    313+
    errmsg_internal("%s", _(msg)),
    314+
    errdetail("Attribute \"%s\" of type %s does not match corresponding attribute of type %s.",
    315+
    attname,
    316+
    format_type_be(outdesc->tdtypeid),
    317+
    format_type_be(indesc->tdtypeid))));
    318+
    attrMap[i] = (AttrNumber) (j + 1);
    319+
    break;
    320+
    }
    321+
    }
    322+
    if (attrMap[i] == 0)
    323+
    ereport(ERROR,
    324+
    (errcode(ERRCODE_DATATYPE_MISMATCH),
    325+
    errmsg_internal("%s", _(msg)),
    326+
    errdetail("Attribute \"%s\" of type %s does not exist in type %s.",
    327+
    attname,
    328+
    format_type_be(outdesc->tdtypeid),
    329+
    format_type_be(indesc->tdtypeid))));
    330+
    }
    331+
    332+
    return attrMap;
    333+
    }
    334+
    315335
    /*
    316336
    * Perform conversion of a tuple according to the map.
    317337
    */

    ‎src/backend/commands/tablecmds.c

    Lines changed: 62 additions & 4 deletions
    Original file line numberDiff line numberDiff line change
    @@ -20,6 +20,7 @@
    2020
    #include "access/reloptions.h"
    2121
    #include "access/relscan.h"
    2222
    #include "access/sysattr.h"
    23+
    #include "access/tupconvert.h"
    2324
    #include "access/xact.h"
    2425
    #include "access/xlog.h"
    2526
    #include "catalog/catalog.h"
    @@ -8540,12 +8541,69 @@ ATPrepAlterColumnType(List **wqueue,
    85408541
    ReleaseSysCache(tuple);
    85418542

    85428543
    /*
    8543-
    * The recursion case is handled by ATSimpleRecursion. However, if we are
    8544-
    * told not to recurse, there had better not be any child tables; else the
    8545-
    * alter would put them out of step.
    8544+
    * Recurse manually by queueing a new command for each child, if
    8545+
    * necessary. We cannot apply ATSimpleRecursion here because we need to
    8546+
    * remap attribute numbers in the USING expression, if any.
    8547+
    *
    8548+
    * If we are told not to recurse, there had better not be any child
    8549+
    * tables; else the alter would put them out of step.
    85468550
    */
    85478551
    if (recurse)
    8548-
    ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
    8552+
    {
    8553+
    Oid relid = RelationGetRelid(rel);
    8554+
    ListCell *child;
    8555+
    List *children;
    8556+
    8557+
    children = find_all_inheritors(relid, lockmode, NULL);
    8558+
    8559+
    /*
    8560+
    * find_all_inheritors does the recursive search of the inheritance
    8561+
    * hierarchy, so all we have to do is process all of the relids in the
    8562+
    * list that it returns.
    8563+
    */
    8564+
    foreach(child, children)
    8565+
    {
    8566+
    Oid childrelid = lfirst_oid(child);
    8567+
    Relation childrel;
    8568+
    8569+
    if (childrelid == relid)
    8570+
    continue;
    8571+
    8572+
    /* find_all_inheritors already got lock */
    8573+
    childrel = relation_open(childrelid, NoLock);
    8574+
    CheckTableNotInUse(childrel, "ALTER TABLE");
    8575+
    8576+
    /*
    8577+
    * Remap the attribute numbers. If no USING expression was
    8578+
    * specified, there is no need for this step.
    8579+
    */
    8580+
    if (def->cooked_default)
    8581+
    {
    8582+
    AttrNumber *attmap;
    8583+
    bool found_whole_row;
    8584+
    8585+
    /* create a copy to scribble on */
    8586+
    cmd = copyObject(cmd);
    8587+
    8588+
    attmap = convert_tuples_by_name_map(RelationGetDescr(childrel),
    8589+
    RelationGetDescr(rel),
    8590+
    gettext_noop("could not convert row type"));
    8591+
    ((ColumnDef *) cmd->def)->cooked_default =
    8592+
    map_variable_attnos(def->cooked_default,
    8593+
    1, 0,
    8594+
    attmap, RelationGetDescr(rel)->natts,
    8595+
    &found_whole_row);
    8596+
    if (found_whole_row)
    8597+
    ereport(ERROR,
    8598+
    (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
    8599+
    errmsg("cannot convert whole-row table reference"),
    8600+
    errdetail("USING expression contains a whole-row table reference.")));
    8601+
    pfree(attmap);
    8602+
    }
    8603+
    ATPrepCmd(wqueue, childrel, cmd, false, true, lockmode);
    8604+
    relation_close(childrel, NoLock);
    8605+
    }
    8606+
    }
    85498607
    else if (!recursing &&
    85508608
    find_inheritance_children(RelationGetRelid(rel), NoLock) != NIL)
    85518609
    ereport(ERROR,

    ‎src/include/access/tupconvert.h

    Lines changed: 4 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -38,6 +38,10 @@ extern TupleConversionMap *convert_tuples_by_name(TupleDesc indesc,
    3838
    TupleDesc outdesc,
    3939
    const char *msg);
    4040

    41+
    extern AttrNumber *convert_tuples_by_name_map(TupleDesc indesc,
    42+
    TupleDesc outdesc,
    43+
    const char *msg);
    44+
    4145
    extern HeapTuple do_convert_tuple(HeapTuple tuple, TupleConversionMap *map);
    4246

    4347
    extern void free_conversion_map(TupleConversionMap *map);

    ‎src/test/regress/expected/alter_table.out

    Lines changed: 22 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -2048,6 +2048,28 @@ select relname, conname, coninhcount, conislocal, connoinherit
    20482048
    test_inh_check_child | test_inh_check_a_check | 1 | f | f
    20492049
    (6 rows)
    20502050

    2051+
    -- ALTER COLUMN TYPE with different schema in children
    2052+
    -- Bug at https://postgr.es/m/20170102225618.GA10071@telsasoft.com
    2053+
    CREATE TABLE test_type_diff (f1 int);
    2054+
    CREATE TABLE test_type_diff_c (extra smallint) INHERITS (test_type_diff);
    2055+
    ALTER TABLE test_type_diff ADD COLUMN f2 int;
    2056+
    INSERT INTO test_type_diff_c VALUES (1, 2, 3);
    2057+
    ALTER TABLE test_type_diff ALTER COLUMN f2 TYPE bigint USING f2::bigint;
    2058+
    CREATE TABLE test_type_diff2 (int_two int2, int_four int4, int_eight int8);
    2059+
    CREATE TABLE test_type_diff2_c1 (int_four int4, int_eight int8, int_two int2);
    2060+
    CREATE TABLE test_type_diff2_c2 (int_eight int8, int_two int2, int_four int4);
    2061+
    CREATE TABLE test_type_diff2_c3 (int_two int2, int_four int4, int_eight int8);
    2062+
    ALTER TABLE test_type_diff2_c1 INHERIT test_type_diff2;
    2063+
    ALTER TABLE test_type_diff2_c2 INHERIT test_type_diff2;
    2064+
    ALTER TABLE test_type_diff2_c3 INHERIT test_type_diff2;
    2065+
    INSERT INTO test_type_diff2_c1 VALUES (1, 2, 3);
    2066+
    INSERT INTO test_type_diff2_c2 VALUES (4, 5, 6);
    2067+
    INSERT INTO test_type_diff2_c3 VALUES (7, 8, 9);
    2068+
    ALTER TABLE test_type_diff2 ALTER COLUMN int_four TYPE int8 USING int_four::int8;
    2069+
    -- whole-row references are disallowed
    2070+
    ALTER TABLE test_type_diff2 ALTER COLUMN int_four TYPE int4 USING (pg_column_size(test_type_diff2));
    2071+
    ERROR: cannot convert whole-row table reference
    2072+
    DETAIL: USING expression contains a whole-row table reference.
    20512073
    -- check for rollback of ANALYZE corrupting table property flags (bug #11638)
    20522074
    CREATE TABLE check_fk_presence_1 (id int PRIMARY KEY, t text);
    20532075
    CREATE TABLE check_fk_presence_2 (id int REFERENCES check_fk_presence_1, t text);

    ‎src/test/regress/sql/alter_table.sql

    Lines changed: 22 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -1335,6 +1335,28 @@ select relname, conname, coninhcount, conislocal, connoinherit
    13351335
    where relname like 'test_inh_check%' and c.conrelid = r.oid
    13361336
    order by 1, 2;
    13371337

    1338+
    -- ALTER COLUMN TYPE with different schema in children
    1339+
    -- Bug at https://postgr.es/m/20170102225618.GA10071@telsasoft.com
    1340+
    CREATE TABLE test_type_diff (f1 int);
    1341+
    CREATE TABLE test_type_diff_c (extra smallint) INHERITS (test_type_diff);
    1342+
    ALTER TABLE test_type_diff ADD COLUMN f2 int;
    1343+
    INSERT INTO test_type_diff_c VALUES (1, 2, 3);
    1344+
    ALTER TABLE test_type_diff ALTER COLUMN f2 TYPE bigint USING f2::bigint;
    1345+
    1346+
    CREATE TABLE test_type_diff2 (int_two int2, int_four int4, int_eight int8);
    1347+
    CREATE TABLE test_type_diff2_c1 (int_four int4, int_eight int8, int_two int2);
    1348+
    CREATE TABLE test_type_diff2_c2 (int_eight int8, int_two int2, int_four int4);
    1349+
    CREATE TABLE test_type_diff2_c3 (int_two int2, int_four int4, int_eight int8);
    1350+
    ALTER TABLE test_type_diff2_c1 INHERIT test_type_diff2;
    1351+
    ALTER TABLE test_type_diff2_c2 INHERIT test_type_diff2;
    1352+
    ALTER TABLE test_type_diff2_c3 INHERIT test_type_diff2;
    1353+
    INSERT INTO test_type_diff2_c1 VALUES (1, 2, 3);
    1354+
    INSERT INTO test_type_diff2_c2 VALUES (4, 5, 6);
    1355+
    INSERT INTO test_type_diff2_c3 VALUES (7, 8, 9);
    1356+
    ALTER TABLE test_type_diff2 ALTER COLUMN int_four TYPE int8 USING int_four::int8;
    1357+
    -- whole-row references are disallowed
    1358+
    ALTER TABLE test_type_diff2 ALTER COLUMN int_four TYPE int4 USING (pg_column_size(test_type_diff2));
    1359+
    13381360
    -- check for rollback of ANALYZE corrupting table property flags (bug #11638)
    13391361
    CREATE TABLE check_fk_presence_1 (id int PRIMARY KEY, t text);
    13401362
    CREATE TABLE check_fk_presence_2 (id int REFERENCES check_fk_presence_1, t text);

    0 commit comments

    Comments
     (0)
    0