Thread
Commits
-
Fix assignment to array of domain over composite.
- ae7b1dd59001 12.9 landed
- 30e61a8cdc86 13.5 landed
- 0d08c279b007 11.14 landed
- 3e310d837a9b 15.0 landed
- 04dae19f4d50 14.1 landed
-
UPDATE on Domain Array that is based on a composite key crashes
Onder Kalaci <onderk@microsoft.com> — 2021-10-19T07:38:49Z
Hi hackers, I couldn’t find a similar report to this one, so starting a new thread. I can reproduce this on v14.0 as well as PostgreSQL 12.5 (not tried below versions). Steps to reproduce: CREATE TYPE two_ints as (if1 int, if2 int); CREATE DOMAIN domain AS two_ints CHECK ((VALUE).if1 > 0); CREATE TABLE domain_indirection_test (f1 int, f3 domain, domain_array domain[]); INSERT INTO domain_indirection_test (f1,f3.if1) VALUES (0, 1); UPDATE domain_indirection_test SET domain_array[0].if2 = 5; server closed the connection unexpectedly This probably means the server terminated abnormally before or while processing the request. The connection to the server was lost. Attempting reset: Failed. Time: 3.990 ms @:-!> The backtrace on PG 12.5 (As far as I remember, PG14 looks very similar) : (lldb) bt * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0) * frame #0: 0x00000001036584b7 postgres`pg_detoast_datum(datum=0x0000000000000000) at fmgr.c:1741:6 frame #1: 0x0000000103439a86 postgres`ExecEvalFieldStoreDeForm(state=<unavailable>, op=0x00007f9212045df8, econtext=<unavailable>) at execExprInterp.c:3025:12 frame #2: 0x000000010343834e postgres`ExecInterpExpr(state=<unavailable>, econtext=<unavailable>, isnull=0x00007ffeec91fdc7) at execExprInterp.c:1337:4 frame #3: 0x000000010343742b postgres`ExecInterpExprStillValid(state=0x00007f921181db18, econtext=0x00007f921181d670, isNull=<unavailable>) at execExprInterp.c:1778:9 frame #4: 0x0000000103444e0d postgres`ExecEvalExprSwitchContext(state=0x00007f921181db18, econtext=0x00007f921181d670, isNull=<unavailable>) at executor.h:310:13 frame #5: 0x0000000103444cf0 postgres`ExecProject(projInfo=0x00007f921181db10) at executor.h:344:9 frame #6: 0x0000000103444af6 postgres`ExecScan(node=0x00007f921181d560, accessMtd=(postgres`SeqNext at nodeSeqscan.c:51), recheckMtd=(postgres`SeqRecheck at nodeSeqscan.c:90)) at execScan.c:239:12 frame #7: 0x0000000103461d17 postgres`ExecSeqScan(pstate=<unavailable>) at nodeSeqscan.c:112:9 frame #8: 0x000000010344375c postgres`ExecProcNodeFirst(node=0x00007f921181d560) at execProcnode.c:445:9 frame #9: 0x000000010345eefe postgres`ExecProcNode(node=0x00007f921181d560) at executor.h:242:9 frame #10: 0x000000010345e74f postgres`ExecModifyTable(pstate=0x00007f921181d090) at nodeModifyTable.c:2079:14 frame #11: 0x000000010344375c postgres`ExecProcNodeFirst(node=0x00007f921181d090) at execProcnode.c:445:9 frame #12: 0x000000010343f25e postgres`ExecProcNode(node=0x00007f921181d090) at executor.h:242:9 frame #13: 0x000000010343d80d postgres`ExecutePlan(estate=0x00007f921181cd10, planstate=0x00007f921181d090, use_parallel_mode=<unavailable>, operation=CMD_UPDATE, sendTuples=false, numberTuples=0, direction=ForwardScanDirection, dest=0x00007f9211818c38, execute_once=<unavailable>) at execMain.c:1646:10 frame #14: 0x000000010343d745 postgres`standard_ExecutorRun(queryDesc=0x00007f921180e310, direction=ForwardScanDirection, count=0, execute_once=true) at execMain.c:364:3 frame #15: 0x000000010343d67c postgres`ExecutorRun(queryDesc=0x00007f921180e310, direction=ForwardScanDirection, count=0, execute_once=<unavailable>) at execMain.c:308:3 frame #16: 0x00000001035784a8 postgres`ProcessQuery(plan=<unavailable>, sourceText=<unavailable>, params=<unavailable>, queryEnv=0x0000000000000000, dest=<unavailable>, completionTag="") at pquery.c:161:2 frame #17: 0x0000000103577c5e postgres`PortalRunMulti(portal=0x00007f9215024110, isTopLevel=true, setHoldSnapshot=false, dest=0x00007f9211818c38, altdest=0x00007f9211818c38, completionTag="") at pquery.c:0 frame #18: 0x000000010357763d postgres`PortalRun(portal=0x00007f9215024110, count=9223372036854775807, isTopLevel=<unavailable>, run_once=<unavailable>, dest=0x00007f9211818c38, altdest=0x00007f9211818c38, completionTag="") at pquery.c:796:5 frame #19: 0x0000000103574f87 postgres`exec_simple_query(query_string="UPDATE domain_indirection_test SET domain_array[0].if2 = 5;") at postgres.c:1215:10 frame #20: 0x00000001035746b8 postgres`PostgresMain(argc=<unavailable>, argv=<unavailable>, dbname=<unavailable>, username=<unavailable>) at postgres.c:0 frame #21: 0x000000010350d712 postgres`BackendRun(port=<unavailable>) at postmaster.c:4494:2 frame #22: 0x000000010350cffa postgres`BackendStartup(port=<unavailable>) at postmaster.c:4177:3 frame #23: 0x000000010350c59c postgres`ServerLoop at postmaster.c:1725:7 frame #24: 0x000000010350ac8d postgres`PostmasterMain(argc=3, argv=0x00007f9210d049c0) at postmaster.c:1398:11 frame #25: 0x000000010347fbdd postgres`main(argc=<unavailable>, argv=<unavailable>) at main.c:228:3 frame #26: 0x00007fff204e8f3d libdyld.dylib`start + 1 Thanks, Onder KALACI Software Engineer at Microsoft & Developing the Citus database extension for PostgreSQL -
Re: UPDATE on Domain Array that is based on a composite key crashes
Zhihong Yu <zyu@yugabyte.com> — 2021-10-19T09:12:46Z
On Tue, Oct 19, 2021 at 12:39 AM Onder Kalaci <onderk@microsoft.com> wrote: > Hi hackers, > > > > I couldn’t find a similar report to this one, so starting a new thread. I > can reproduce this on v14.0 as well as PostgreSQL 12.5 (not tried below > versions). > > > > Steps to reproduce: > > > > CREATE TYPE two_ints as (if1 int, if2 int); > > CREATE DOMAIN domain AS two_ints CHECK ((VALUE).if1 > 0); > > CREATE TABLE domain_indirection_test (f1 int, f3 domain, domain_array > domain[]); > > INSERT INTO domain_indirection_test (f1,f3.if1) VALUES (0, 1); > > UPDATE domain_indirection_test SET domain_array[0].if2 = 5; > > server closed the connection unexpectedly > > This probably means the server terminated abnormally > > before or while processing the request. > > The connection to the server was lost. Attempting reset: Failed. > > Time: 3.990 ms > > @:-!> > > > > > > The backtrace on PG 12.5 (As far as I remember, PG14 looks very similar) : > > > > (lldb) bt > > * thread #1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS > (code=1, address=0x0) > > * frame #0: 0x00000001036584b7 > postgres`pg_detoast_datum(datum=0x0000000000000000) at fmgr.c:1741:6 > > frame #1: 0x0000000103439a86 > postgres`ExecEvalFieldStoreDeForm(state=<unavailable>, > op=0x00007f9212045df8, econtext=<unavailable>) at execExprInterp.c:3025:12 > > frame #2: 0x000000010343834e > postgres`ExecInterpExpr(state=<unavailable>, econtext=<unavailable>, > isnull=0x00007ffeec91fdc7) at execExprInterp.c:1337:4 > > frame #3: 0x000000010343742b > postgres`ExecInterpExprStillValid(state=0x00007f921181db18, > econtext=0x00007f921181d670, isNull=<unavailable>) at > execExprInterp.c:1778:9 > > frame #4: 0x0000000103444e0d > postgres`ExecEvalExprSwitchContext(state=0x00007f921181db18, > econtext=0x00007f921181d670, isNull=<unavailable>) at executor.h:310:13 > > frame #5: 0x0000000103444cf0 > postgres`ExecProject(projInfo=0x00007f921181db10) at executor.h:344:9 > > frame #6: 0x0000000103444af6 > postgres`ExecScan(node=0x00007f921181d560, accessMtd=(postgres`SeqNext at > nodeSeqscan.c:51), recheckMtd=(postgres`SeqRecheck at nodeSeqscan.c:90)) at > execScan.c:239:12 > > frame #7: 0x0000000103461d17 > postgres`ExecSeqScan(pstate=<unavailable>) at nodeSeqscan.c:112:9 > > frame #8: 0x000000010344375c > postgres`ExecProcNodeFirst(node=0x00007f921181d560) at execProcnode.c:445:9 > > frame #9: 0x000000010345eefe > postgres`ExecProcNode(node=0x00007f921181d560) at executor.h:242:9 > > frame #10: 0x000000010345e74f > postgres`ExecModifyTable(pstate=0x00007f921181d090) at > nodeModifyTable.c:2079:14 > > frame #11: 0x000000010344375c > postgres`ExecProcNodeFirst(node=0x00007f921181d090) at execProcnode.c:445:9 > > frame #12: 0x000000010343f25e > postgres`ExecProcNode(node=0x00007f921181d090) at executor.h:242:9 > > frame #13: 0x000000010343d80d > postgres`ExecutePlan(estate=0x00007f921181cd10, > planstate=0x00007f921181d090, use_parallel_mode=<unavailable>, > operation=CMD_UPDATE, sendTuples=false, numberTuples=0, > direction=ForwardScanDirection, dest=0x00007f9211818c38, > execute_once=<unavailable>) at execMain.c:1646:10 > > frame #14: 0x000000010343d745 > postgres`standard_ExecutorRun(queryDesc=0x00007f921180e310, > direction=ForwardScanDirection, count=0, execute_once=true) at > execMain.c:364:3 > > frame #15: 0x000000010343d67c > postgres`ExecutorRun(queryDesc=0x00007f921180e310, > direction=ForwardScanDirection, count=0, execute_once=<unavailable>) at > execMain.c:308:3 > > frame #16: 0x00000001035784a8 > postgres`ProcessQuery(plan=<unavailable>, sourceText=<unavailable>, > params=<unavailable>, queryEnv=0x0000000000000000, dest=<unavailable>, > completionTag="") at pquery.c:161:2 > > frame #17: 0x0000000103577c5e > postgres`PortalRunMulti(portal=0x00007f9215024110, isTopLevel=true, > setHoldSnapshot=false, dest=0x00007f9211818c38, altdest=0x00007f9211818c38, > completionTag="") at pquery.c:0 > > frame #18: 0x000000010357763d > postgres`PortalRun(portal=0x00007f9215024110, count=9223372036854775807, > isTopLevel=<unavailable>, run_once=<unavailable>, dest=0x00007f9211818c38, > altdest=0x00007f9211818c38, completionTag="") at pquery.c:796:5 > > frame #19: 0x0000000103574f87 > postgres`exec_simple_query(query_string="UPDATE domain_indirection_test SET > domain_array[0].if2 = 5;") at postgres.c:1215:10 > > frame #20: 0x00000001035746b8 > postgres`PostgresMain(argc=<unavailable>, argv=<unavailable>, > dbname=<unavailable>, username=<unavailable>) at postgres.c:0 > > frame #21: 0x000000010350d712 postgres`BackendRun(port=<unavailable>) > at postmaster.c:4494:2 > > frame #22: 0x000000010350cffa > postgres`BackendStartup(port=<unavailable>) at postmaster.c:4177:3 > > frame #23: 0x000000010350c59c postgres`ServerLoop at > postmaster.c:1725:7 > > frame #24: 0x000000010350ac8d postgres`PostmasterMain(argc=3, > argv=0x00007f9210d049c0) at postmaster.c:1398:11 > > frame #25: 0x000000010347fbdd postgres`main(argc=<unavailable>, > argv=<unavailable>) at main.c:228:3 > > frame #26: 0x00007fff204e8f3d libdyld.dylib`start + 1 > > > > > > Thanks, > > Onder KALACI > > Software Engineer at Microsoft & > > Developing the Citus database extension for PostgreSQL > Hi, It seems the following change would fix the crash: diff --git a/src/postgres/src/backend/executor/execExprInterp.c b/src/postgres/src/backend/executor/execExprInterp.c index 622cab9d4..50cb4f014 100644 --- a/src/postgres/src/backend/executor/execExprInterp.c +++ b/src/postgres/src/backend/executor/execExprInterp.c @@ -3038,6 +3038,9 @@ ExecEvalFieldStoreDeForm(ExprState *state, ExprEvalStep *op, ExprContext *econte HeapTupleHeader tuphdr; HeapTupleData tmptup; + if (DatumGetPointer(tupDatum) == NULL) { + return; + } tuphdr = DatumGetHeapTupleHeader(tupDatum); tmptup.t_len = HeapTupleHeaderGetDatumLength(tuphdr); ItemPointerSetInvalid(&(tmptup.t_self)); Here is the result after the update statement: # UPDATE domain_indirection_test SET domain_array[0].if2 = 5; UPDATE 1 # select * from domain_indirection_test; f1 | f3 | domain_array ----+------+---------------- 0 | (1,) | [0:0]={"(,5)"} (1 row) Cheers -
Re: UPDATE on Domain Array that is based on a composite key crashes
Japin Li <japinli@hotmail.com> — 2021-10-19T10:33:36Z
On Tue, 19 Oct 2021 at 17:12, Zhihong Yu <zyu@yugabyte.com> wrote: > On Tue, Oct 19, 2021 at 12:39 AM Onder Kalaci <onderk@microsoft.com> wrote: > >> Hi hackers, >> >> >> >> I couldn’t find a similar report to this one, so starting a new thread. I >> can reproduce this on v14.0 as well as PostgreSQL 12.5 (not tried below >> versions). >> >> >> >> Steps to reproduce: >> >> >> >> CREATE TYPE two_ints as (if1 int, if2 int); >> >> CREATE DOMAIN domain AS two_ints CHECK ((VALUE).if1 > 0); >> >> CREATE TABLE domain_indirection_test (f1 int, f3 domain, domain_array >> domain[]); >> >> INSERT INTO domain_indirection_test (f1,f3.if1) VALUES (0, 1); >> >> UPDATE domain_indirection_test SET domain_array[0].if2 = 5; >> >> server closed the connection unexpectedly >> >> This probably means the server terminated abnormally >> >> before or while processing the request. >> >> The connection to the server was lost. Attempting reset: Failed. >> >> Time: 3.990 ms >> > > Hi, > It seems the following change would fix the crash: > > diff --git a/src/postgres/src/backend/executor/execExprInterp.c > b/src/postgres/src/backend/executor/execExprInterp.c > index 622cab9d4..50cb4f014 100644 > --- a/src/postgres/src/backend/executor/execExprInterp.c > +++ b/src/postgres/src/backend/executor/execExprInterp.c > @@ -3038,6 +3038,9 @@ ExecEvalFieldStoreDeForm(ExprState *state, > ExprEvalStep *op, ExprContext *econte > HeapTupleHeader tuphdr; > HeapTupleData tmptup; > > + if (DatumGetPointer(tupDatum) == NULL) { > + return; > + } > tuphdr = DatumGetHeapTupleHeader(tupDatum); > tmptup.t_len = HeapTupleHeaderGetDatumLength(tuphdr); > ItemPointerSetInvalid(&(tmptup.t_self)); > > Here is the result after the update statement: > > # UPDATE domain_indirection_test SET domain_array[0].if2 = 5; > UPDATE 1 > # select * from domain_indirection_test; > f1 | f3 | domain_array > ----+------+---------------- > 0 | (1,) | [0:0]={"(,5)"} > (1 row) > Yeah, it fixes the core dump. However, When I test the patch, I find the update will replace all data in `domain` if we only update one field. postgres=# UPDATE domain_indirection_test SET domain_array[0].if2 = 5; UPDATE 1 postgres=# select * from domain_indirection_test ; f1 | f3 | domain_array ----+------+---------------- 0 | (1,) | [0:0]={"(,5)"} (1 row) postgres=# UPDATE domain_indirection_test SET domain_array[0].if1 = 10; UPDATE 1 postgres=# select * from domain_indirection_test ; f1 | f3 | domain_array ----+------+----------------- 0 | (1,) | [0:0]={"(10,)"} (1 row) So I try to update all field in `domain`, and find only the last one will be stored. postgres=# UPDATE domain_indirection_test SET domain_array[0].if1 = 10, domain_array[0].if2 = 5; UPDATE 1 postgres=# select * from domain_indirection_test ; f1 | f3 | domain_array ----+------+---------------- 0 | (1,) | [0:0]={"(,5)"} (1 row) postgres=# UPDATE domain_indirection_test SET domain_array[0].if2 = 10, domain_array[0].if1 = 5; UPDATE 1 postgres=# select * from domain_indirection_test ; f1 | f3 | domain_array ----+------+---------------- 0 | (1,) | [0:0]={"(5,)"} (1 row) Does this worked as expected? For me, For me, I think this is a bug. -- Regrads, Japin Li. ChengDu WenWu Information Technology Co.,Ltd. -
Re: UPDATE on Domain Array that is based on a composite key crashes
Zhihong Yu <zyu@yugabyte.com> — 2021-10-19T15:17:34Z
On Tue, Oct 19, 2021 at 2:12 AM Zhihong Yu <zyu@yugabyte.com> wrote: > > > On Tue, Oct 19, 2021 at 12:39 AM Onder Kalaci <onderk@microsoft.com> > wrote: > >> Hi hackers, >> >> >> >> I couldn’t find a similar report to this one, so starting a new thread. I >> can reproduce this on v14.0 as well as PostgreSQL 12.5 (not tried below >> versions). >> >> >> >> Steps to reproduce: >> >> >> >> CREATE TYPE two_ints as (if1 int, if2 int); >> >> CREATE DOMAIN domain AS two_ints CHECK ((VALUE).if1 > 0); >> >> CREATE TABLE domain_indirection_test (f1 int, f3 domain, domain_array >> domain[]); >> >> INSERT INTO domain_indirection_test (f1,f3.if1) VALUES (0, 1); >> >> UPDATE domain_indirection_test SET domain_array[0].if2 = 5; >> >> server closed the connection unexpectedly >> >> This probably means the server terminated abnormally >> >> before or while processing the request. >> >> The connection to the server was lost. Attempting reset: Failed. >> >> Time: 3.990 ms >> >> @:-!> >> >> >> >> >> >> The backtrace on PG 12.5 (As far as I remember, PG14 looks very similar) : >> >> >> >> (lldb) bt >> >> * thread #1, queue = 'com.apple.main-thread', stop reason = >> EXC_BAD_ACCESS (code=1, address=0x0) >> >> * frame #0: 0x00000001036584b7 >> postgres`pg_detoast_datum(datum=0x0000000000000000) at fmgr.c:1741:6 >> >> frame #1: 0x0000000103439a86 >> postgres`ExecEvalFieldStoreDeForm(state=<unavailable>, >> op=0x00007f9212045df8, econtext=<unavailable>) at execExprInterp.c:3025:12 >> >> frame #2: 0x000000010343834e >> postgres`ExecInterpExpr(state=<unavailable>, econtext=<unavailable>, >> isnull=0x00007ffeec91fdc7) at execExprInterp.c:1337:4 >> >> frame #3: 0x000000010343742b >> postgres`ExecInterpExprStillValid(state=0x00007f921181db18, >> econtext=0x00007f921181d670, isNull=<unavailable>) at >> execExprInterp.c:1778:9 >> >> frame #4: 0x0000000103444e0d >> postgres`ExecEvalExprSwitchContext(state=0x00007f921181db18, >> econtext=0x00007f921181d670, isNull=<unavailable>) at executor.h:310:13 >> >> frame #5: 0x0000000103444cf0 >> postgres`ExecProject(projInfo=0x00007f921181db10) at executor.h:344:9 >> >> frame #6: 0x0000000103444af6 >> postgres`ExecScan(node=0x00007f921181d560, accessMtd=(postgres`SeqNext at >> nodeSeqscan.c:51), recheckMtd=(postgres`SeqRecheck at nodeSeqscan.c:90)) at >> execScan.c:239:12 >> >> frame #7: 0x0000000103461d17 >> postgres`ExecSeqScan(pstate=<unavailable>) at nodeSeqscan.c:112:9 >> >> frame #8: 0x000000010344375c >> postgres`ExecProcNodeFirst(node=0x00007f921181d560) at execProcnode.c:445:9 >> >> frame #9: 0x000000010345eefe >> postgres`ExecProcNode(node=0x00007f921181d560) at executor.h:242:9 >> >> frame #10: 0x000000010345e74f >> postgres`ExecModifyTable(pstate=0x00007f921181d090) at >> nodeModifyTable.c:2079:14 >> >> frame #11: 0x000000010344375c >> postgres`ExecProcNodeFirst(node=0x00007f921181d090) at execProcnode.c:445:9 >> >> frame #12: 0x000000010343f25e >> postgres`ExecProcNode(node=0x00007f921181d090) at executor.h:242:9 >> >> frame #13: 0x000000010343d80d >> postgres`ExecutePlan(estate=0x00007f921181cd10, >> planstate=0x00007f921181d090, use_parallel_mode=<unavailable>, >> operation=CMD_UPDATE, sendTuples=false, numberTuples=0, >> direction=ForwardScanDirection, dest=0x00007f9211818c38, >> execute_once=<unavailable>) at execMain.c:1646:10 >> >> frame #14: 0x000000010343d745 >> postgres`standard_ExecutorRun(queryDesc=0x00007f921180e310, >> direction=ForwardScanDirection, count=0, execute_once=true) at >> execMain.c:364:3 >> >> frame #15: 0x000000010343d67c >> postgres`ExecutorRun(queryDesc=0x00007f921180e310, >> direction=ForwardScanDirection, count=0, execute_once=<unavailable>) at >> execMain.c:308:3 >> >> frame #16: 0x00000001035784a8 >> postgres`ProcessQuery(plan=<unavailable>, sourceText=<unavailable>, >> params=<unavailable>, queryEnv=0x0000000000000000, dest=<unavailable>, >> completionTag="") at pquery.c:161:2 >> >> frame #17: 0x0000000103577c5e >> postgres`PortalRunMulti(portal=0x00007f9215024110, isTopLevel=true, >> setHoldSnapshot=false, dest=0x00007f9211818c38, altdest=0x00007f9211818c38, >> completionTag="") at pquery.c:0 >> >> frame #18: 0x000000010357763d >> postgres`PortalRun(portal=0x00007f9215024110, count=9223372036854775807, >> isTopLevel=<unavailable>, run_once=<unavailable>, dest=0x00007f9211818c38, >> altdest=0x00007f9211818c38, completionTag="") at pquery.c:796:5 >> >> frame #19: 0x0000000103574f87 >> postgres`exec_simple_query(query_string="UPDATE domain_indirection_test SET >> domain_array[0].if2 = 5;") at postgres.c:1215:10 >> >> frame #20: 0x00000001035746b8 >> postgres`PostgresMain(argc=<unavailable>, argv=<unavailable>, >> dbname=<unavailable>, username=<unavailable>) at postgres.c:0 >> >> frame #21: 0x000000010350d712 postgres`BackendRun(port=<unavailable>) >> at postmaster.c:4494:2 >> >> frame #22: 0x000000010350cffa >> postgres`BackendStartup(port=<unavailable>) at postmaster.c:4177:3 >> >> frame #23: 0x000000010350c59c postgres`ServerLoop at >> postmaster.c:1725:7 >> >> frame #24: 0x000000010350ac8d postgres`PostmasterMain(argc=3, >> argv=0x00007f9210d049c0) at postmaster.c:1398:11 >> >> frame #25: 0x000000010347fbdd postgres`main(argc=<unavailable>, >> argv=<unavailable>) at main.c:228:3 >> >> frame #26: 0x00007fff204e8f3d libdyld.dylib`start + 1 >> >> >> >> >> >> Thanks, >> >> Onder KALACI >> >> Software Engineer at Microsoft & >> >> Developing the Citus database extension for PostgreSQL >> > > Hi, > It seems the following change would fix the crash: > > diff --git a/src/postgres/src/backend/executor/execExprInterp.c > b/src/postgres/src/backend/executor/execExprInterp.c > index 622cab9d4..50cb4f014 100644 > --- a/src/postgres/src/backend/executor/execExprInterp.c > +++ b/src/postgres/src/backend/executor/execExprInterp.c > @@ -3038,6 +3038,9 @@ ExecEvalFieldStoreDeForm(ExprState *state, > ExprEvalStep *op, ExprContext *econte > HeapTupleHeader tuphdr; > HeapTupleData tmptup; > > + if (DatumGetPointer(tupDatum) == NULL) { > + return; > + } > tuphdr = DatumGetHeapTupleHeader(tupDatum); > tmptup.t_len = HeapTupleHeaderGetDatumLength(tuphdr); > ItemPointerSetInvalid(&(tmptup.t_self)); > > Here is the result after the update statement: > > # UPDATE domain_indirection_test SET domain_array[0].if2 = 5; > UPDATE 1 > # select * from domain_indirection_test; > f1 | f3 | domain_array > ----+------+---------------- > 0 | (1,) | [0:0]={"(,5)"} > (1 row) > > Cheers > Hi, Here is the patch. If the new test should be placed in a different .sql file, please let me know. The update issue Japin mentioned seems to be orthogonal to the crash. Cheers -
Re: UPDATE on Domain Array that is based on a composite key crashes
Japin Li <japinli@hotmail.com> — 2021-10-19T16:38:31Z
On Tue, 19 Oct 2021 at 23:17, Zhihong Yu <zyu@yugabyte.com> wrote: > On Tue, Oct 19, 2021 at 2:12 AM Zhihong Yu <zyu@yugabyte.com> wrote: > >> >> >> On Tue, Oct 19, 2021 at 12:39 AM Onder Kalaci <onderk@microsoft.com> >> wrote: >> >>> Hi hackers, >>> >>> >>> >>> I couldn’t find a similar report to this one, so starting a new thread. I >>> can reproduce this on v14.0 as well as PostgreSQL 12.5 (not tried below >>> versions). >>> >>> >>> >>> Steps to reproduce: >>> >>> >>> >>> CREATE TYPE two_ints as (if1 int, if2 int); >>> >>> CREATE DOMAIN domain AS two_ints CHECK ((VALUE).if1 > 0); >>> >>> CREATE TABLE domain_indirection_test (f1 int, f3 domain, domain_array >>> domain[]); >>> >>> INSERT INTO domain_indirection_test (f1,f3.if1) VALUES (0, 1); >>> >>> UPDATE domain_indirection_test SET domain_array[0].if2 = 5; >>> >>> server closed the connection unexpectedly >>> >>> This probably means the server terminated abnormally >>> >>> before or while processing the request. >>> >>> The connection to the server was lost. Attempting reset: Failed. >>> >>> Time: 3.990 ms >>> >>> @:-!> >>> >>> >>> >>> >>> >>> The backtrace on PG 12.5 (As far as I remember, PG14 looks very similar) : >>> >>> >>> >>> (lldb) bt >>> >>> * thread #1, queue = 'com.apple.main-thread', stop reason = >>> EXC_BAD_ACCESS (code=1, address=0x0) >>> >>> * frame #0: 0x00000001036584b7 >>> postgres`pg_detoast_datum(datum=0x0000000000000000) at fmgr.c:1741:6 >>> >>> frame #1: 0x0000000103439a86 >>> postgres`ExecEvalFieldStoreDeForm(state=<unavailable>, >>> op=0x00007f9212045df8, econtext=<unavailable>) at execExprInterp.c:3025:12 >>> >>> frame #2: 0x000000010343834e >>> postgres`ExecInterpExpr(state=<unavailable>, econtext=<unavailable>, >>> isnull=0x00007ffeec91fdc7) at execExprInterp.c:1337:4 >>> >>> frame #3: 0x000000010343742b >>> postgres`ExecInterpExprStillValid(state=0x00007f921181db18, >>> econtext=0x00007f921181d670, isNull=<unavailable>) at >>> execExprInterp.c:1778:9 >>> >>> frame #4: 0x0000000103444e0d >>> postgres`ExecEvalExprSwitchContext(state=0x00007f921181db18, >>> econtext=0x00007f921181d670, isNull=<unavailable>) at executor.h:310:13 >>> >>> frame #5: 0x0000000103444cf0 >>> postgres`ExecProject(projInfo=0x00007f921181db10) at executor.h:344:9 >>> >>> frame #6: 0x0000000103444af6 >>> postgres`ExecScan(node=0x00007f921181d560, accessMtd=(postgres`SeqNext at >>> nodeSeqscan.c:51), recheckMtd=(postgres`SeqRecheck at nodeSeqscan.c:90)) at >>> execScan.c:239:12 >>> >>> frame #7: 0x0000000103461d17 >>> postgres`ExecSeqScan(pstate=<unavailable>) at nodeSeqscan.c:112:9 >>> >>> frame #8: 0x000000010344375c >>> postgres`ExecProcNodeFirst(node=0x00007f921181d560) at execProcnode.c:445:9 >>> >>> frame #9: 0x000000010345eefe >>> postgres`ExecProcNode(node=0x00007f921181d560) at executor.h:242:9 >>> >>> frame #10: 0x000000010345e74f >>> postgres`ExecModifyTable(pstate=0x00007f921181d090) at >>> nodeModifyTable.c:2079:14 >>> >>> frame #11: 0x000000010344375c >>> postgres`ExecProcNodeFirst(node=0x00007f921181d090) at execProcnode.c:445:9 >>> >>> frame #12: 0x000000010343f25e >>> postgres`ExecProcNode(node=0x00007f921181d090) at executor.h:242:9 >>> >>> frame #13: 0x000000010343d80d >>> postgres`ExecutePlan(estate=0x00007f921181cd10, >>> planstate=0x00007f921181d090, use_parallel_mode=<unavailable>, >>> operation=CMD_UPDATE, sendTuples=false, numberTuples=0, >>> direction=ForwardScanDirection, dest=0x00007f9211818c38, >>> execute_once=<unavailable>) at execMain.c:1646:10 >>> >>> frame #14: 0x000000010343d745 >>> postgres`standard_ExecutorRun(queryDesc=0x00007f921180e310, >>> direction=ForwardScanDirection, count=0, execute_once=true) at >>> execMain.c:364:3 >>> >>> frame #15: 0x000000010343d67c >>> postgres`ExecutorRun(queryDesc=0x00007f921180e310, >>> direction=ForwardScanDirection, count=0, execute_once=<unavailable>) at >>> execMain.c:308:3 >>> >>> frame #16: 0x00000001035784a8 >>> postgres`ProcessQuery(plan=<unavailable>, sourceText=<unavailable>, >>> params=<unavailable>, queryEnv=0x0000000000000000, dest=<unavailable>, >>> completionTag="") at pquery.c:161:2 >>> >>> frame #17: 0x0000000103577c5e >>> postgres`PortalRunMulti(portal=0x00007f9215024110, isTopLevel=true, >>> setHoldSnapshot=false, dest=0x00007f9211818c38, altdest=0x00007f9211818c38, >>> completionTag="") at pquery.c:0 >>> >>> frame #18: 0x000000010357763d >>> postgres`PortalRun(portal=0x00007f9215024110, count=9223372036854775807, >>> isTopLevel=<unavailable>, run_once=<unavailable>, dest=0x00007f9211818c38, >>> altdest=0x00007f9211818c38, completionTag="") at pquery.c:796:5 >>> >>> frame #19: 0x0000000103574f87 >>> postgres`exec_simple_query(query_string="UPDATE domain_indirection_test SET >>> domain_array[0].if2 = 5;") at postgres.c:1215:10 >>> >>> frame #20: 0x00000001035746b8 >>> postgres`PostgresMain(argc=<unavailable>, argv=<unavailable>, >>> dbname=<unavailable>, username=<unavailable>) at postgres.c:0 >>> >>> frame #21: 0x000000010350d712 postgres`BackendRun(port=<unavailable>) >>> at postmaster.c:4494:2 >>> >>> frame #22: 0x000000010350cffa >>> postgres`BackendStartup(port=<unavailable>) at postmaster.c:4177:3 >>> >>> frame #23: 0x000000010350c59c postgres`ServerLoop at >>> postmaster.c:1725:7 >>> >>> frame #24: 0x000000010350ac8d postgres`PostmasterMain(argc=3, >>> argv=0x00007f9210d049c0) at postmaster.c:1398:11 >>> >>> frame #25: 0x000000010347fbdd postgres`main(argc=<unavailable>, >>> argv=<unavailable>) at main.c:228:3 >>> >>> frame #26: 0x00007fff204e8f3d libdyld.dylib`start + 1 >>> >>> >>> >>> >>> >>> Thanks, >>> >>> Onder KALACI >>> >>> Software Engineer at Microsoft & >>> >>> Developing the Citus database extension for PostgreSQL >>> >> >> Hi, >> It seems the following change would fix the crash: >> >> diff --git a/src/postgres/src/backend/executor/execExprInterp.c >> b/src/postgres/src/backend/executor/execExprInterp.c >> index 622cab9d4..50cb4f014 100644 >> --- a/src/postgres/src/backend/executor/execExprInterp.c >> +++ b/src/postgres/src/backend/executor/execExprInterp.c >> @@ -3038,6 +3038,9 @@ ExecEvalFieldStoreDeForm(ExprState *state, >> ExprEvalStep *op, ExprContext *econte >> HeapTupleHeader tuphdr; >> HeapTupleData tmptup; >> >> + if (DatumGetPointer(tupDatum) == NULL) { >> + return; >> + } >> tuphdr = DatumGetHeapTupleHeader(tupDatum); >> tmptup.t_len = HeapTupleHeaderGetDatumLength(tuphdr); >> ItemPointerSetInvalid(&(tmptup.t_self)); >> >> Here is the result after the update statement: >> >> # UPDATE domain_indirection_test SET domain_array[0].if2 = 5; >> UPDATE 1 >> # select * from domain_indirection_test; >> f1 | f3 | domain_array >> ----+------+---------------- >> 0 | (1,) | [0:0]={"(,5)"} >> (1 row) >> >> Cheers >> > Hi, > Here is the patch. Thanks for your updated the patch. A minor code style, we can remove the braces when there is only one statement, this is more consenting with the codebase. Others looks good to me. > If the new test should be placed in a different .sql file, please let me > know. > > The update issue Japin mentioned seems to be orthogonal to the crash. > I start a new thread to discuss it [1]. [1] https://www.postgresql.org/message-id/MEYP282MB1669BED5CEFE711E00C7421EB6BD9@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM -- Regrads, Japin Li. ChengDu WenWu Information Technology Co.,Ltd. -
Re: UPDATE on Domain Array that is based on a composite key crashes
Tom Lane <tgl@sss.pgh.pa.us> — 2021-10-19T17:04:39Z
[ please do not quote the entire thread when replying ] Zhihong Yu <zyu@yugabyte.com> writes: > Here is the patch. This patch seems quite misguided to me. The proximate cause of the crash is that we're arriving at ExecEvalFieldStoreDeForm with *op->resnull and *op->resvalue both zero, which is a completely invalid situation for a pass-by-reference datatype; so something upstream of this messed up. Even if there were an argument for acting as though that were a valid NULL value, this patch fails to do so; that'd require setting all the output fieldstore.nulls[] entries to true, which you didn't. Moreover, experiment quickly shows that the problem only shows up with an array of domain over composite, not an array of plain composite. The proposed patch doesn't seem to have anything to do with that observation. After some digging around, I see where the issue actually is: the expression tree we're dealing with looks like {SUBSCRIPTINGREF :refexpr {VAR } :refassgnexpr {COERCETODOMAIN :arg {FIELDSTORE :arg {CASETESTEXPR } } } } The array element we intend to replace has to be passed down to the CaseTestExpr, but that isn't happening. That's because isAssignmentIndirectionExpr fails to recognize a tree like this, so ExecInitSubscriptingRef doesn't realize it needs to arrange for that. I believe the attached is a correct fix. regards, tom lane -
Re: UPDATE on Domain Array that is based on a composite key crashes
Zhihong Yu <zyu@yugabyte.com> — 2021-10-19T17:29:40Z
On Tue, Oct 19, 2021 at 10:04 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: > [ please do not quote the entire thread when replying ] > > Zhihong Yu <zyu@yugabyte.com> writes: > > Here is the patch. > > This patch seems quite misguided to me. The proximate cause of > the crash is that we're arriving at ExecEvalFieldStoreDeForm with > *op->resnull and *op->resvalue both zero, which is a completely > invalid situation for a pass-by-reference datatype; so something > upstream of this messed up. Even if there were an argument for > acting as though that were a valid NULL value, this patch fails to > do so; that'd require setting all the output fieldstore.nulls[] > entries to true, which you didn't. > > Moreover, experiment quickly shows that the problem only shows up with > an array of domain over composite, not an array of plain composite. > The proposed patch doesn't seem to have anything to do with that > observation. > > After some digging around, I see where the issue actually is: > the expression tree we're dealing with looks like > > {SUBSCRIPTINGREF > :refexpr > {VAR > } > :refassgnexpr > {COERCETODOMAIN > :arg > {FIELDSTORE > :arg > {CASETESTEXPR > } > } > } > } > > The array element we intend to replace has to be passed down to > the CaseTestExpr, but that isn't happening. That's because > isAssignmentIndirectionExpr fails to recognize a tree like > this, so ExecInitSubscriptingRef doesn't realize it needs to > arrange for that. > > I believe the attached is a correct fix. > > regards, tom lane > > Hi, Tom's patch fixes the update of individual field inside the domain type as well. Tom's patch looks good to me.