Обсуждение: BUG #17351: Altering a composite type created for a partitioned table can lead to a crash
BUG #17351: Altering a composite type created for a partitioned table can lead to a crash
От
PG Bug reporting form
Дата:
The following bug has been logged on the website: Bug reference: 17351 Logged by: Alexander Lakhin Email address: exclusion@gmail.com PostgreSQL version: 14.1 Operating system: Ubuntu 20.04 Description: When executing the following queries: create table pt (a int, b int) partition by list (b); create table t(a pt, check (a = '(1, 2)'::pt)); alter table pt alter column a type char(4); \d+ t The server crashes with the following stack: Core was generated by `postgres: law regression [local] SELECT '. Program terminated with signal SIGABRT, Aborted. #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 50 ../sysdeps/unix/sysv/linux/raise.c: No such file or directory. (gdb) bt #0 __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50 #1 0x00007f8af00f3859 in __GI_abort () at abort.c:79 #2 0x000055be552e0cc3 in ExceptionalCondition (conditionName=0x55be5536aaad "true", errorType=0x55be5536aa93 "unrecognized TOAST vartag", fileName=0x55be5536aa00 "heaptuple.c", lineNumber=1320) at assert.c:69 #3 0x000055be54bc28b9 in heap_deform_tuple (tuple=0x7fff462dbae0, tupleDesc=0x7f8ae2c518b0, values=0x55be55d89650, isnull=0x55be55d93fd8) at heaptuple.c:1320 #4 0x000055be552410bf in record_out (fcinfo=0x7fff462dbb70) at rowtypes.c:364 #5 0x000055be552ec16f in FunctionCall1Coll (flinfo=0x7fff462dbbf0, collation=0, arg1=94275972402416) at fmgr.c:1138 #6 0x000055be552ed72d in OutputFunctionCall (flinfo=0x7fff462dbbf0, val=94275972402416) at fmgr.c:1575 #7 0x000055be552ed9e4 in OidOutputFunctionCall (functionId=2291, val=94275972402416) at fmgr.c:1658 #8 0x000055be5525dece in get_const_expr (constval=0x55be55d88c98, context=0x7fff462dc300, showtype=0) at ruleutils.c:10261 #9 0x000055be55258add in get_rule_expr (node=0x55be55d88c98, context=0x7fff462dc300, showimplicit=true) at ruleutils.c:8348 #10 0x000055be552589dc in get_rule_expr_paren (node=0x55be55d88c98, context=0x7fff462dc300, showimplicit=true, parentNode=0x55be55d88b90) at ruleutils.c:8301 #11 0x000055be5525bec8 in get_oper_expr (expr=0x55be55d88b90, context=0x7fff462dc300) at ruleutils.c:9612 #12 0x000055be55258dc8 in get_rule_expr (node=0x55be55d88b90, context=0x7fff462dc300, showimplicit=false) at ruleutils.c:8453 #13 0x000055be5524d6af in deparse_expression_pretty (expr=0x55be55d88b90, dpcontext=0x55be55d89170, forceprefix=false, showimplicit=false, prettyFlags=7, startIndent=0) at ruleutils.c:3547 #14 0x000055be5524af4e in pg_get_constraintdef_worker (constraintId=16391, fullCommand=false, prettyFlags=7, missing_ok=true) at ruleutils.c:2419 #15 0x000055be5524a1ed in pg_get_constraintdef_ext (fcinfo=0x55be55d25290) at ruleutils.c:2089 #16 0x000055be54e9bf85 in ExecInterpExpr (state=0x55be55d24990, econtext=0x55be55d24478, isnull=0x7fff462dc79f) at execExprInterp.c:749 #17 0x000055be54e9e33c in ExecInterpExprStillValid (state=0x55be55d24990, econtext=0x55be55d24478, isNull=0x7fff462dc79f) at execExprInterp.c:1824 #18 0x000055be54eb8858 in ExecEvalExprSwitchContext (state=0x55be55d24990, econtext=0x55be55d24478, isNull=0x7fff462dc79f) at ../../../src/include/executor/executor.h:339 #19 0x000055be54eb88d0 in ExecProject (projInfo=0x55be55d24988) at ../../../src/include/executor/executor.h:373 #20 0x000055be54eb8daf in ExecScan (node=0x55be55d24360, accessMtd=0x55be54efaf88 <SeqNext>, recheckMtd=0x55be54efb031 <SeqRecheck>) at execScan.c:238 #21 0x000055be54efb08a in ExecSeqScan (pstate=0x55be55d24360) at nodeSeqscan.c:112 #22 0x000055be54eb491d in ExecProcNodeFirst (node=0x55be55d24360) at execProcnode.c:463 #23 0x000055be54efc75e in ExecProcNode (node=0x55be55d24360) at ../../../src/include/executor/executor.h:257 #24 0x000055be54efc8b1 in ExecSort (pstate=0x55be55d24148) at nodeSort.c:108 #25 0x000055be54eb491d in ExecProcNodeFirst (node=0x55be55d24148) at execProcnode.c:463 #26 0x000055be54ea8409 in ExecProcNode (node=0x55be55d24148) at ../../../src/include/executor/executor.h:257 #27 0x000055be54eab027 in ExecutePlan (estate=0x55be55d23f10, planstate=0x55be55d24148, use_parallel_mode=false, operation=CMD_SELECT, sendTuples=true, numberTuples=0, direction=ForwardScanDirection, dest=0x55be55d8ad08, execute_once=true) at execMain.c:1551 #28 0x000055be54ea8b40 in standard_ExecutorRun (queryDesc=0x55be55d14dc0, direction=ForwardScanDirection, count=0, execute_once=true) at execMain.c:361 #29 0x000055be54ea892b in ExecutorRun (queryDesc=0x55be55d14dc0, direction=ForwardScanDirection, count=0, execute_once=true) at execMain.c:305 #30 0x000055be55125a1e in PortalRunSelect (portal=0x55be55c8d7a0, forward=true, count=0, dest=0x55be55d8ad08) at pquery.c:921 #31 0x000055be55125642 in PortalRun (portal=0x55be55c8d7a0, count=9223372036854775807, isTopLevel=true, run_once=true, dest=0x55be55d8ad08, altdest=0x55be55d8ad08, qc=0x7fff462dcc80) at pquery.c:765 #32 0x000055be5511e52c in exec_simple_query ( query_string=0x55be55c1e8a0 "SELECT r.conname, pg_catalog.pg_get_constraintdef(r.oid, true)\nFROM pg_catalog.pg_constraint r\nWHERE r.conrelid = '16388' AND r.contype = 'c'\nORDER BY 1;") at postgres.c:1214 #33 0x000055be5512341b in PostgresMain (argc=1, argv=0x7fff462dcea0, dbname=0x55be55c4a9f8 "regression", username=0x55be55c4a9d8 "law") at postgres.c:4486 #34 0x000055be55047d64 in BackendRun (port=0x55be55c42110) at postmaster.c:4530 #35 0x000055be550475bf in BackendStartup (port=0x55be55c42110) at postmaster.c:4252 #36 0x000055be550433b4 in ServerLoop () at postmaster.c:1745 #37 0x000055be55042b11 in PostmasterMain (argc=3, argv=0x55be55c18910) at postmaster.c:1417 #38 0x000055be54f322fc in main (argc=3, argv=0x55be55c18910) at main.c:209 The outcome varies depending on the constant in the check. Without "partition by ..." I get "cannot alter table "pt" because column "t.a" uses its row type" error.
PG Bug reporting form <noreply@postgresql.org> writes: > When executing the following queries: > create table pt (a int, b int) partition by list (b); > create table t(a pt, check (a = '(1, 2)'::pt)); > alter table pt alter column a type char(4); > \d+ t > The server crashes with the following stack: Hmm. We really ought to reject the ALTER TABLE. We do if "pt" is a plain table: regression=# create table pt (a int, b int); CREATE TABLE regression=# create table t(a pt); CREATE TABLE regression=# alter table pt alter column a type char(4); ERROR: cannot alter table "pt" because column "t.a" uses its row type So something is mistakenly skipping that check for partitioned tables. I think we're also failing to worry about the rowtype of the constant in t's check constraint; it seems like that has to be complained of as well, even if the underyling columns aren't of type "pt". regards, tom lane
I wrote: > PG Bug reporting form <noreply@postgresql.org> writes: >> When executing the following queries: >> create table pt (a int, b int) partition by list (b); >> create table t(a pt, check (a = '(1, 2)'::pt)); >> alter table pt alter column a type char(4); >> \d+ t >> The server crashes with the following stack: > Hmm. We really ought to reject the ALTER TABLE. We do if "pt" > is a plain table: So the problem is that ATRewriteTables is supposed to make this check (by calling find_composite_type_dependencies), but first it does /* Relations without storage may be ignored here */ if (!RELKIND_HAS_STORAGE(tab->relkind)) continue; so the test is skipped for a partitioned table. There is a separate check in ATPrepAlterColumnType: if (tab->relkind == RELKIND_COMPOSITE_TYPE || tab->relkind == RELKIND_FOREIGN_TABLE) { /* * For composite types and foreign tables, do this check now. Regular * tables will check it later when the table is being rewritten. */ find_composite_type_dependencies(rel->rd_rel->reltype, rel, NULL); } The best fix seems to be to make that check use the inverse condition. I also experimented with moving the "Relations without storage" early exit down past the find_composite_type_dependencies step, in hopes of getting rid of the check in ATPrepAlterColumnType. But that fails to cover all cases, because we might not set the rewrite flag. > I think we're also failing to worry about the rowtype of the > constant in t's check constraint; it seems like that has to > be complained of as well, even if the underyling columns > aren't of type "pt". This seems to be all right, but I thought it'd be prudent to add a test case covering it. regards, tom lane diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 89bc865e28..23364d3c7e 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -12183,12 +12183,11 @@ ATPrepAlterColumnType(List **wqueue, errmsg("\"%s\" is not a table", RelationGetRelationName(rel)))); - if (tab->relkind == RELKIND_COMPOSITE_TYPE || - tab->relkind == RELKIND_FOREIGN_TABLE) + if (!RELKIND_HAS_STORAGE(tab->relkind)) { /* - * For composite types and foreign tables, do this check now. Regular - * tables will check it later when the table is being rewritten. + * For relations without storage, do this check now. Regular tables + * will check it later when the table is being rewritten. */ find_composite_type_dependencies(rel->rd_rel->reltype, rel, NULL); } diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 24d1c7cd28..16e0475663 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -2061,11 +2061,24 @@ begin; create table skip_wal_skip_rewrite_index (c varchar(10) primary key); alter table skip_wal_skip_rewrite_index alter c type varchar(20); commit; --- table's row type -create table tab1 (a int, b text); -create table tab2 (x int, y tab1); -alter table tab1 alter column b type varchar; -- fails -ERROR: cannot alter table "tab1" because column "tab2.y" uses its row type +-- We disallow changing table's row type if it's used for storage +create table at_tab1 (a int, b text); +create table at_tab2 (x int, y at_tab1); +alter table at_tab1 alter column b type varchar; -- fails +ERROR: cannot alter table "at_tab1" because column "at_tab2.y" uses its row type +drop table at_tab2; +-- Use of row type in an expression is defended differently +create table at_tab2 (x int, y text, check((x,y)::at_tab1 = (1,'42')::at_tab1)); +alter table at_tab1 alter column b type varchar; -- allowed, but ... +insert into at_tab2 values(1,'42'); -- ... this will fail +ERROR: ROW() column has type text instead of type character varying +drop table at_tab1, at_tab2; +-- Check it for a partitioned table, too +create table at_tab1 (a int, b text) partition by list(a); +create table at_tab2 (x int, y at_tab1); +alter table at_tab1 alter column b type varchar; -- fails +ERROR: cannot alter table "at_tab1" because column "at_tab2.y" uses its row type +drop table at_tab1, at_tab2; -- Alter column type that's part of a partitioned index create table at_partitioned (a int, b text) partition by range (a); create table at_part_1 partition of at_partitioned for values from (0) to (1000); diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 5fac2585d9..ac894c0602 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -1420,10 +1420,21 @@ create table skip_wal_skip_rewrite_index (c varchar(10) primary key); alter table skip_wal_skip_rewrite_index alter c type varchar(20); commit; --- table's row type -create table tab1 (a int, b text); -create table tab2 (x int, y tab1); -alter table tab1 alter column b type varchar; -- fails +-- We disallow changing table's row type if it's used for storage +create table at_tab1 (a int, b text); +create table at_tab2 (x int, y at_tab1); +alter table at_tab1 alter column b type varchar; -- fails +drop table at_tab2; +-- Use of row type in an expression is defended differently +create table at_tab2 (x int, y text, check((x,y)::at_tab1 = (1,'42')::at_tab1)); +alter table at_tab1 alter column b type varchar; -- allowed, but ... +insert into at_tab2 values(1,'42'); -- ... this will fail +drop table at_tab1, at_tab2; +-- Check it for a partitioned table, too +create table at_tab1 (a int, b text) partition by list(a); +create table at_tab2 (x int, y at_tab1); +alter table at_tab1 alter column b type varchar; -- fails +drop table at_tab1, at_tab2; -- Alter column type that's part of a partitioned index create table at_partitioned (a int, b text) partition by range (a);