Re: Commit 86dc90056 - Rework planning and execution of UPDATE and DELETE

On Mon, Apr 19, 2021 at 10:00 PM Rushabh Lathia
<rushabh(dot)lathia(at)gmail(dot)com> wrote:
>
> Hi.
>
> With the commit mentioned in the $subject, I am seeing the
> change in behaviour with the varlena header size. Please
> consider the below test:
>
> postgres(at)83795=#CREATE TABLE test_storage_char(d char(20));
> CREATE TABLE
> postgres(at)83795=#INSERT INTO test_storage_char SELECT REPEAT('e', 20);
> INSERT 0 1
> postgres(at)83795=#SELECT d, pg_column_size(d) FROM test_storage_char;
> d | pg_column_size
> ----------------------+----------------
> eeeeeeeeeeeeeeeeeeee | 21
> (1 row)
>
> postgres(at)83795=#ALTER TABLE test_storage_char ALTER COLUMN d SET STORAGE PLAIN;
> ALTER TABLE
> postgres(at)83795=#SELECT d, pg_column_size(d) FROM test_storage_char;
> d | pg_column_size
> ----------------------+----------------
> eeeeeeeeeeeeeeeeeeee | 21
> (1 row)
>
> postgres(at)83795=#UPDATE test_storage_char SET d='ab' WHERE d LIKE '%e%';
> UPDATE 1
> postgres(at)83795=#SELECT d, pg_column_size(d) FROM test_storage_char;
> d | pg_column_size
> ----------------------+----------------
> ab | 24
> (1 row)
>
> After changing the STORAGE for the column and UPDATE, pg_column_size
> now returns the size as 24.
>
> BEFORE Commit 86dc90056:
>
> postgres(at)129158=#SELECT d, pg_column_size(d) FROM test_storage_char;
> d | pg_column_size
> ----------------------+----------------
> ab | 21
> (1 row)
>
> I am not sure whether this change is expected? Or missing something
> in the toasting the attribute?

I haven't studied this closely enough yet to say if the new behavior
is correct or not, but can say why this has changed.

Before 86dc90056, the new tuple to pass to ExecUpdate would be
computed with a TupleDesc that uses pg_type.typstorage for the column
instead of the column's actual pg_attribute.attstorage. That's
because the new tuple would be computed from the subplan's targetlist
and the TupleDesc for that targetlist is computed with no regard to
where the computed tuple will go; IOW ignoring the target table's
actual TupleDesc.

After 86dc90056, the new tuple is computed with the target table's
actual TupleDesc, so the new value respects the column's attstorage,
which makes me think the new behavior is not wrong.

Will look more closely tomorrow.

--
Amit Langote
EDB: http://www.enterprisedb.com