Skip to content

Commit

Permalink
Fix UPDATE SET ... RETURNING processing for encrypted tuples
Browse files Browse the repository at this point in the history
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
  • Loading branch information
dAdAbird committed Sep 27, 2024
1 parent 9bf409a commit b451158
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 3 deletions.
4 changes: 2 additions & 2 deletions expected/update.out
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions meson.build
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
13 changes: 12 additions & 1 deletion src/access/pg_tde_slot.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit b451158

Please sign in to comment.