BUG #17351: Altering a composite type created for a partitioned table can lead to a crash

Started by PG Bug reporting formover 4 years ago3 messagesbugs
Jump to latest
#1PG Bug reporting form
noreply@postgresql.org

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.

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: PG Bug reporting form (#1)
Re: BUG #17351: Altering a composite type created for a partitioned table can lead to a crash

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

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Tom Lane (#2)
Re: BUG #17351: Altering a composite type created for a partitioned table can lead to a crash

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.patchtext/x-diff; charset=us-ascii; name=prevent-alter-column-type-in-partitioned-table.patchDownload+36-11