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

Commit 4d4ab6c

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 ed8e8b8 commit 4d4ab6c

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
< 1E79 tr class="diff-line-row">
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"
@@ -7999,12 +8000,69 @@ ATPrepAlterColumnType(List **wqueue,
79998000
ReleaseSysCache(tuple);
80008001

80018002
/*
8002-
* The recursion case is handled by ATSimpleRecursion. However, if we are
8003-
* told not to recurse, there had better not be any child tables; else the
8004-
* alter would put them out of step.
8003+
* Recurse manually by queueing a new command for each child, if
8004+
* necessary. We cannot apply ATSimpleRecursion here because we need to
8005+
* remap attribute numbers in the USING expression, if any.
8006+
*
8007+
* If we are told not to recurse, there had better not be any child
8008+
* tables; else the alter would put them out of step.
80058009
*/
80068010
if (recurse)
8007-
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
8011+
{
8012+
Oid relid = RelationGetRelid(rel);
8013+
ListCell *child;
8014+
List *children;
8015+
8016+
children = find_all_inheritors(relid, lockmode, NULL);
8017+
8018+
/*
8019+
* find_all_inheritors does the recursive search of the inheritance
8020+
* hierarchy, so all we have to do is process all of the relids in the
8021+
* list that it returns.
8022+
*/
8023+
foreach(child, children)
8024+
{
8025+
Oid childrelid = lfirst_oid(child);
8026+
Relation childrel;
8027+
8028+
if (childrelid == relid)
8029+
continue;
8030+
8031+
/* find_all_inheritors already got lock */
8032+
childrel = relation_open(childrelid, NoLock);
8033+
CheckTableNotInUse(childrel, "ALTER TABLE");
8034+
8035+
/*
8036+
* Remap the attribute numbers. If no USING expression was
8037+
* specified, there is no need for this step.
8038+
*/
8039+
if (def->cooked_default)
8040+
{
8041+
AttrNumber *attmap;
8042+
bool found_whole_row;
8043+
8044+
/* create a copy to scribble on */
8045+
cmd = copyObject(cmd);
8046+
8047+
attmap = convert_tuples_by_name_map(RelationGetDescr(childrel),
8048+
RelationGetDescr(rel),
8049+
gettext_noop("could not convert row type"));
8050+
((ColumnDef *) cmd->def)->cooked_default =
8051+
map_variable_attnos(def->cooked_default,
8052+
1, 0,
8053+
attmap, RelationGetDescr(rel)->natts,
8054+
&found_whole_row);
8055+
if (found_whole_row)
8056+
ereport(ERROR,
8057+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
8058+
errmsg("cannot convert whole-row table reference"),
8059+
errdetail("USING expression contains a whole-row table reference.")));
8060+
pfree(attmap);
8061+
}
8062+
ATPrepCmd(wqueue, childrel, cmd, false, true, lockmode);
8063+
relation_close(childrel, NoLock);
8064+
}
8065+
}
80088066
else if (!recursing &&
80098067
find_inheritance_children(RelationGetRelid(rel), NoLock) != NIL)
80108068
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
@@ -2018,6 +2018,28 @@ select relname, conname, coninhcount, conislocal, connoinherit
20182018
test_inh_check_child | test_inh_check_a_check | 1 | f | f
20192019
(6 rows)
20202020

2021+
-- ALTER COLUMN TYPE with different schema in children
2022+
-- Bug at https://postgr.es/m/20170102225618.GA10071@telsasoft.com
2023+
CREATE TABLE test_type_diff (f1 int);
2024+
CREATE TABLE test_type_diff_c (extra smallint) INHERITS (test_type_diff);
2025+
ALTER TABLE test_type_diff ADD COLUMN f2 int;
2026+
INSERT INTO test_type_diff_c VALUES (1, 2, 3);
2027+
ALTER TABLE test_type_diff ALTER COLUMN f2 TYPE bigint USING f2::bigint;
2028+
CREATE TABLE test_type_diff2 (int_two int2, int_four int4, int_eight int8);
2029+
CREATE TABLE test_type_diff2_c1 (int_four int4, int_eight int8, int_two int2);
2030+
CREATE TABLE test_type_diff2_c2 (int_eight int8, int_two int2, int_four int4);
2031+
CREATE TABLE test_type_diff2_c3 (int_two int2, int_four int4, int_eight int8);
2032+
ALTER TABLE test_type_diff2_c1 INHERIT test_type_diff2;
2033+
ALTER TABLE test_type_diff2_c2 INHERIT test_type_diff2;
2034+
ALTER TABLE test_type_diff2_c3 INHERIT test_type_diff2;
2035+
INSERT INTO test_type_diff2_c1 VALUES (1, 2, 3);
2036+
INSERT INTO test_type_diff2_c2 VALUES (4, 5, 6);
2037+
INSERT INTO test_type_diff2_c3 VALUES (7, 8, 9);
2038+
ALTER TABLE test_type_diff2 ALTER COLUMN int_four TYPE int8 USING int_four::int8;
2039+
-- whole-row references are disallowed
2040+
ALTER TABLE test_type_diff2 ALTER COLUMN int_four TYPE int4 USING (pg_column_size(test_type_diff2));
2041+
ERROR: cannot convert whole-row table reference
2042+
DETAIL: USING expression contains a whole-row table reference.
20212043
-- check for rollback of ANALYZE corrupting table property flags (bug #11638)
20222044
CREATE TABLE check_fk_presence_1 (id int PRIMARY KEY, t text);
20232045
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
@@ -1321,6 +1321,28 @@ select relname, conname, coninhcount, conislocal, connoinherit
13211321
where relname like 'test_inh_check%' and c.conrelid = r.oid
13221322
order by 1, 2;
13231323

1324+
-- ALTER COLUMN TYPE with different schema in children
1325+
-- Bug at https://postgr.es/m/20170102225618.GA10071@telsasoft.com
1326+
CREATE TABLE test_type_diff (f1 int);
1327+
CREATE TABLE test_type_diff_c (extra smallint) INHERITS (test_type_diff);
1328+
ALTER TABLE test_type_diff ADD COLUMN f2 int;
1329+
INSERT INTO test_type_diff_c VALUES (1, 2, 3);
1330+
ALTER TABLE test_type_diff ALTER COLUMN f2 TYPE bigint USING f2::bigint;
1331+
1332+
CREATE TABLE test_type_diff2 (int_two int2, int_four int4, int_eight int8);
1333+
CREATE TABLE test_type_diff2_c1 (int_four int4, int_eight int8, int_two int2);
1334+
CREATE TABLE test_type_diff2_c2 (int_eight int8, int_two int2, int_four int4);
1335+
CREATE TABLE test_type_diff2_c3 (int_two int2, int_four int4, int_eight int8);
1336+
ALTER TABLE test_type_diff2_c1 INHERIT test_type_diff2;
1337+
ALTER TABLE test_type_diff2_c2 INHERIT test_type_diff2;
1338+
ALTER TABLE test_type_diff2_c3 INHERIT test_type_diff2;
1339+
INSERT INTO test_type_diff2_c1 VALUES (1, 2, 3);
1340+
INSERT INTO test_type_diff2_c2 VALUES (4, 5, 6);
1341+
INSERT INTO test_type_diff2_c3 VALUES (7, 8, 9);
1342+
ALTER TABLE test_type_diff2 ALTER COLUMN int_four TYPE int8 USING int_four::int8;
1343+
-- whole-row references are disallowed
1344+
ALTER TABLE test_type_diff2 ALTER COLUMN int_four TYPE int4 USING (pg_column_size(test_type_diff2));
1345+
13241346
-- check for rollback of ANALYZE corrupting table property flags (bug #11638)
13251347
CREATE TABLE check_fk_presence_1 (id int PRIMARY KEY, t text);
13261348
CREATE TABLE check_fk_presence_2 (id int REFERENCES check_fk_presence_1, t text);

0 commit comments

Comments
 (0)
0