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

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.