Skip to content

2.25.1.0-b50

2.25.1.0-b50: [#24701] YSQL: Always fetch wholerow attribute in non-fast path UPDATEs
@arpang arpang tagged this 18 Dec 16:02 
Summary:
PG in commit 86dc90056dfdbd9d1b891718d2e5614e3e432f35 reworked the UPDATE node's execution to get only the updated and junk columns from its child node. It (the UPDATE node) then fetches the existing "old" tuple and merges it with the output of the child node to construct the new tuple.

This led to performance degradation in Yugabyte because:
- Before this change, the UPDATE node sent only write operations that could be buffered (with some restrictions) and batch executed.
- With this change, the UPDATE node sent a read followed by a write operation per row, triggering 2 RPCs per row.

YB commit cf176f4cc5f7e47219bb982865f855d7be0735d7 / D31673 addressed the above problem by sending the old tuple as "wholerow" junk attribute to the UPDATE node. With this, the read operation inside the UPDATE node to fetch the old tuple was no longer required. While this approach was correct, it made the following mistakes:

- Justification: Citing a `failed to fetch tuple being updated`, it claimed that performing read inside the UPDATE node has a correctness issue that materializes only in Yugabyte (see D31673's summary for details). That claim was incorrect. This issue could have been resolved by using the correct `in_txn_limit`. (Skipping the details here as that approach is not being taken anyway).

- Over-optimization: YB does not need the old tuple in some cases. So, it tried to fetch the wholerow attribute only when necessary. While doing so, it missed (at least) one case: when a subset of foreign key columns are updated. Fetching the old row is necessary to check for foreign key violations in this case correctly. Consider:

```
CREATE TABLE pk(a INT, b INT, PRIMARY KEY (a, b));
CREATE TABLE fk(a INT, b INT, FOREIGN KEY(a, b) REFERENCES pk);
INSERT INTO pk VALUES (1, 1);
INSERT INTO fk VALUES (1, 1);
UPDATE fk set a = a + 1; -- should fail due to foreign key violation
```
Without the old tuple, during the UPDATE operation, the only tuple present in the relation becomes  (a = 2, b = null). Consequently, the foreign key violation is not thrown.

There can potentially be more cases where not fetching the wholerow can lead to incorrect results. Moreover, in most cases, the old tuple is needed anyway. This revision removes this optimization and fetches the wholerow attribute in all non-fast path UPDATEs.
Jira: DB-13777

Test Plan:
   ./yb_build.sh --java-test org.yb.pgsql.TestPgRegressForeignKey

Reviewers: amartsinchyk, kramanathan

Reviewed By: amartsinchyk

Subscribers: yql

Differential Revision: https://phorge.dev.yugabyte.com/D40671
Assets 2
Loading