Alvaro Herrera wrote:
> Tom Lane wrote:
>
> > We could probably fix the specific issue being seen here by passing the
> > expression tree through a suitable attno remapping,
>
> Here's a first attempt at fixing this. It makes the test pass, but I
> have the feeling that more complex ones might need more work.
Here's another one with three main differences:
1. Make the whole-row check an ereport() not elog(). You can use a
whole-row expression in USING, which makes it fire, so better make it
translatable. An artificial example is in the new regression tests,
ALTER TABLE test_type_diff2 ALTER COLUMN int_four TYPE int4 USING
(pg_column_size(test_type_diff2));
but I suppose somebody with more imagination could come up with
something actually interesting.
2. The foreign table case was broken, as evidenced by the foreign_table
regression test.
3. If there is no USING expression, there is no need to do the whole
map_variable_attnos() dance.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index e217f57..77b4bf5 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7999,12 +7999,98 @@ ATPrepAlterColumnType(List **wqueue,
ReleaseSysCache(tuple);
/*
- * The recursion case is handled by ATSimpleRecursion. However, if we
are
- * told not to recurse, there had better not be any child tables; else
the
- * alter would put them out of step.
+ * Recurse manually by queueing a new command for each child, if
+ * necessary. We cannot apply ATSimpleRecursion here because we need to
+ * remap attribute numbers in the USING expression, if any.
+ *
+ * If we are told not to recurse, there had better not be any child
+ * tables; else the alter would put them out of step.
*/
if (recurse)
- ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
+ {
+ Oid relid = RelationGetRelid(rel);
+ ListCell *child;
+ List *children;
+
+ children = find_all_inheritors(relid, lockmode, NULL);
+
+ /*
+ * find_all_inheritors does the recursive search of the
inheritance
+ * hierarchy, so all we have to do is process all of the relids
in the
+ * list that it returns.
+ */
+ foreach(child, children)
+ {
+ Oid childrelid = lfirst_oid(child);
+ Relation childrel;
+
+ if (childrelid == relid)
+ continue;
+
+ /* find_all_inheritors already got lock */
+ childrel = relation_open(childrelid, NoLock);
+ CheckTableNotInUse(childrel, "ALTER TABLE");
+
+ /*
+ * Remap the attribute numbers. If no USING expression
was
+ * specified, there is no need for this step.
+ */
+ if (def->cooked_default)
+ {
+ TupleDesc ptdesc;
+ TupleDesc ctdesc;
+ AttrNumber *attmap;
+ AttrNumber pnum;
+ bool found_whole_row;
+
+ /*
+ * Build an attribute map for
map_variable_attnos. This is
+ * O(N^2) on the number of attributes ...
+ */
+ ptdesc = RelationGetDescr(rel);
+ ctdesc = RelationGetDescr(childrel);
+ attmap = (AttrNumber *)
palloc0(sizeof(AttrNumber) *
+
ptdesc->natts);
+ for (pnum = 1; pnum <= ptdesc->natts; pnum++)
+ {
+ bool found = false;
+ AttrNumber cnum;
+
+ for (cnum = 1; cnum <= ctdesc->natts;
cnum++)
+ {
+ if
(strcmp(NameStr(ptdesc->attrs[pnum - 1]->attname),
+
NameStr(ctdesc->attrs[cnum - 1]->attname)) == 0)
+ {
+ attmap[pnum - 1] = cnum;
+ found = true;
+ break;
+ }
+ }
+
+ /* should not happen */
+ if (!found)
+ elog(ERROR, "column \"%s\" not
found in child table \"%s\"",
+
NameStr(ptdesc->attrs[pnum - 1]->attname),
+
RelationGetRelationName(childrel));
+ }
+
+ cmd = copyObject(cmd);
+ ((ColumnDef *) cmd->def)->cooked_default =
+ map_variable_attnos(def->cooked_default,
+
1, 0,
+
attmap, ptdesc->natts,
+
&found_whole_row);
+ if (found_whole_row)
+ ereport(ERROR,
+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot convert
whole-row table reference"),
+ errdetail("USING
expression contains a whole-row table reference.")));
+ pfree(attmap);
+ }
+ ATPrepCmd(wqueue, childrel, cmd, false, true, lockmode);
+ relation_close(childrel, NoLock);
+ }
+ }
else if (!recursing &&
find_inheritance_children(RelationGetRelid(rel),
NoLock) != NIL)
ereport(ERROR,
diff --git a/src/test/regress/expected/alter_table.out
b/src/test/regress/expected/alter_table.out
index de3c69a..c693f77 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2018,6 +2018,28 @@ select relname, conname, coninhcount, conislocal,
connoinherit
test_inh_check_child | test_inh_check_a_check | 1 | f | f
(6 rows)
+-- ALTER COLUMN TYPE with different schema in children
+-- Bug at https://postgr.es/message-id/[email protected]
+CREATE TABLE test_type_diff (f1 int);
+CREATE TABLE test_type_diff_c (extra smallint) INHERITS (test_type_diff);
+ALTER TABLE test_type_diff ADD COLUMN f2 int;
+INSERT INTO test_type_diff_c VALUES (1, 2, 3);
+ALTER TABLE test_type_diff ALTER COLUMN f2 TYPE bigint USING f2::bigint;
+CREATE TABLE test_type_diff2 (int_two int2, int_four int4, int_eight int8);
+CREATE TABLE test_type_diff2_c1 (int_four int4, int_eight int8, int_two int2);
+CREATE TABLE test_type_diff2_c2 (int_eight int8, int_two int2, int_four int4);
+CREATE TABLE test_type_diff2_c3 (int_two int2, int_four int4, int_eight int8);
+ALTER TABLE test_type_diff2_c1 INHERIT test_type_diff2;
+ALTER TABLE test_type_diff2_c2 INHERIT test_type_diff2;
+ALTER TABLE test_type_diff2_c3 INHERIT test_type_diff2;
+INSERT INTO test_type_diff2_c1 VALUES (1, 2, 3);
+INSERT INTO test_type_diff2_c2 VALUES (4, 5, 6);
+INSERT INTO test_type_diff2_c3 VALUES (7, 8, 9);
+ALTER TABLE test_type_diff2 ALTER COLUMN int_four TYPE int8 USING
int_four::int8;
+-- whole-row references are disallowed
+ALTER TABLE test_type_diff2 ALTER COLUMN int_four TYPE int4 USING
(pg_column_size(test_type_diff2));
+ERROR: cannot convert whole-row table reference
+DETAIL: USING expression contains a whole-row table reference.
-- check for rollback of ANALYZE corrupting table property flags (bug #11638)
CREATE TABLE check_fk_presence_1 (id int PRIMARY KEY, t text);
CREATE TABLE check_fk_presence_2 (id int REFERENCES check_fk_presence_1, t
text);
diff --git a/src/test/regress/sql/alter_table.sql
b/src/test/regress/sql/alter_table.sql
index 1459311..f3a7d8e 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1321,6 +1321,28 @@ select relname, conname, coninhcount, conislocal,
connoinherit
where relname like 'test_inh_check%' and c.conrelid = r.oid
order by 1, 2;
+-- ALTER COLUMN TYPE with different schema in children
+-- Bug at https://postgr.es/message-id/[email protected]
+CREATE TABLE test_type_diff (f1 int);
+CREATE TABLE test_type_diff_c (extra smallint) INHERITS (test_type_diff);
+ALTER TABLE test_type_diff ADD COLUMN f2 int;
+INSERT INTO test_type_diff_c VALUES (1, 2, 3);
+ALTER TABLE test_type_diff ALTER COLUMN f2 TYPE bigint USING f2::bigint;
+
+CREATE TABLE test_type_diff2 (int_two int2, int_four int4, int_eight int8);
+CREATE TABLE test_type_diff2_c1 (int_four int4, int_eight int8, int_two int2);
+CREATE TABLE test_type_diff2_c2 (int_eight int8, int_two int2, int_four int4);
+CREATE TABLE test_type_diff2_c3 (int_two int2, int_four int4, int_eight int8);
+ALTER TABLE test_type_diff2_c1 INHERIT test_type_diff2;
+ALTER TABLE test_type_diff2_c2 INHERIT test_type_diff2;
+ALTER TABLE test_type_diff2_c3 INHERIT test_type_diff2;
+INSERT INTO test_type_diff2_c1 VALUES (1, 2, 3);
+INSERT INTO test_type_diff2_c2 VALUES (4, 5, 6);
+INSERT INTO test_type_diff2_c3 VALUES (7, 8, 9);
+ALTER TABLE test_type_diff2 ALTER COLUMN int_four TYPE int8 USING
int_four::int8;
+-- whole-row references are disallowed
+ALTER TABLE test_type_diff2 ALTER COLUMN int_four TYPE int4 USING
(pg_column_size(test_type_diff2));
+
-- check for rollback of ANALYZE corrupting table property flags (bug #11638)
CREATE TABLE check_fk_presence_1 (id int PRIMARY KEY, t text);
CREATE TABLE check_fk_presence_2 (id int REFERENCES check_fk_presence_1, t
text);
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
