From b4511586a997bc712f76a67ad09ed6308e189e41 Mon Sep 17 00:00:00 2001 From: Andrew Pogrebnoy Date: Fri, 27 Sep 2024 14:08:46 +0300 Subject: [PATCH] Fix UPDATE SET ... RETURNING processing for encrypted tuples If `get_heap_tuple` is NULL, the core uses `copy_heap_tuple` instead. The former returns a pointer to a tuple in the slot and the latter makes a copy of such a tuple. For UPDATE SET, the core uses the slot for INSERT and later for RETURNING processing. If we copy the tuple the next happens: 1. The core creates a slot with the generic tuple. 2. It passed to `pg_tdeam_tuple_update()` and it gets a copy of the tuple here [https://github.com/Percona-Lab/pg_tde/blob/6d4f7e5b7bd2507ce65deb315ea8d5647f474cf9/src17/access/pg_tdeam_handler.c#L336]. 3. This generic tuple is filled with the proper data and used for the update here [https://github.com/Percona-Lab/pg_tde/blob/6d4f7e5b7bd2507ce65deb315ea8d5647f474cf9/src17/access/pg_tdeam_handler.c#L343]. 4. Later on, RETURNING processing uses the slot's tuple but is still a generic unmodified one because of the copy. 5. That results in wrong RETURNING data. To avoid this, we should return a pointer to the slot's tuple instead of copying it. Fixes PG-1056 --- expected/update.out | 4 ++-- meson.build | 1 + src/access/pg_tde_slot.c | 13 ++++++++++++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/expected/update.out b/expected/update.out index c4932b53..c81d2c0a 100644 --- a/expected/update.out +++ b/expected/update.out @@ -15,11 +15,11 @@ CREATE TABLE update_test ( a INT DEFAULT 10, b INT, c TEXT -); +) USING tde_heap_basic; CREATE TABLE upsert_test ( a INT PRIMARY KEY, b TEXT -); +) USING tde_heap_basic; INSERT INTO update_test VALUES (5, 10, 'foo'); INSERT INTO update_test(b, a) VALUES (15, 10); INSERT INTO upsert_test VALUES (2, 'Beeble') ON CONFLICT(a) diff --git a/meson.build b/meson.build index aa45a232..3198a59f 100644 --- a/meson.build +++ b/meson.build @@ -93,6 +93,7 @@ tests += { 'move_large_tuples', 'non_sorted_off_compact', 'update_compare_indexes', + 'update', 'pg_tde_is_encrypted', 'test_issue_153_fix', 'multi_insert', diff --git a/src/access/pg_tde_slot.c b/src/access/pg_tde_slot.c index cc989d20..8db45d2a 100644 --- a/src/access/pg_tde_slot.c +++ b/src/access/pg_tde_slot.c @@ -241,6 +241,17 @@ tdeheap_tts_buffer_heap_copyslot(TupleTableSlot *dstslot, TupleTableSlot *srcslo } } +static HeapTuple +tdeheap_tts_buffer_heap_get_heap_tuple(TupleTableSlot *slot) +{ + BufferHeapTupleTableSlot *bslot = (BufferHeapTupleTableSlot *) slot; + Assert(!TTS_EMPTY(slot)); + + if (!bslot->base.tuple) + tdeheap_tts_buffer_heap_materialize(slot); + return bslot->base.tuple; +} + static HeapTuple tdeheap_tts_buffer_heap_copy_heap_tuple(TupleTableSlot *slot) { @@ -462,7 +473,7 @@ const TupleTableSlotOps TTSOpsTDEBufferHeapTuple = { .is_current_xact_tuple = tdeheap_buffer_is_current_xact_tuple, #endif .copyslot = tdeheap_tts_buffer_heap_copyslot, - .get_heap_tuple = NULL, + .get_heap_tuple = tdeheap_tts_buffer_heap_get_heap_tuple, /* A buffer heap tuple table slot can not "own" a minimal tuple. */ .get_minimal_tuple = NULL,