pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #17351: Altering a composite type created for a partitioned table can lead to a crash
3+ messages / 2 participants
[nested] [flat]
Commits from this thread:
2022-01-06 7ead9925ff ccc7c3ad86 2ce113a4f0 e5b044c84e 86d4bbb56a f285d95839 Prevent altering partitioned table's rowtype, if it's used elsewhere.
* BUG #17351: Altering a composite type created for a partitioned table can lead to a crash
@ 2022-01-01 18:00 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 3+ messages in thread
From: PG Bug reporting form @ 2022-01-01 18:00 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: exclusion@gmail.com
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.
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: BUG #17351: Altering a composite type created for a partitioned table can lead to a crash
@ 2022-01-01 21:27 Tom Lane <tgl@sss.pgh.pa.us>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 3+ messages in thread
From: Tom Lane @ 2022-01-01 21:27 UTC (permalink / raw)
To: exclusion@gmail.com; +Cc: pgsql-bugs@lists.postgresql.org
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
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: BUG #17351: Altering a composite type created for a partitioned table can lead to a crash
@ 2022-01-06 20:51 Tom Lane <tgl@sss.pgh.pa.us>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 3+ messages in thread
From: Tom Lane @ 2022-01-06 20:51 UTC (permalink / raw)
To: exclusion@gmail.com; +Cc: pgsql-bugs@lists.postgresql.org
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
Attachments:
prevent-alter-column-type-in-partitioned-table.patch (text/x-diff, 4.0K) download
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);
end of thread, other threads:[~2022-01-06 20:51 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2022-01-01 18:00 BUG #17351: Altering a composite type created for a partitioned table can lead to a crash PG Bug reporting form <noreply@postgresql.org>
2022-01-01 21:27 ` Tom Lane <tgl@sss.pgh.pa.us>
2022-01-06 20:51 ` Tom Lane <tgl@sss.pgh.pa.us>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox