Re: UPDATE on Domain Array that is based on a composite key crashes - Mailing list pgsql-hackers

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.