UPDATE on Domain Array that is based on a composite key crashes

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

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.

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

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]/messages/by-id/MEYP282MB1669BED5CEFE711E00C7421EB6BD9@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM.

[1]: /messages/by-id/MEYP282MB1669BED5CEFE711E00C7421EB6BD9@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.

[ 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

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.