-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PG-1056 xmin and xmax correctness #292
Conversation
Performance test results: |
@dutow see my commit's message.
Am I missing something?
|
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
39ccd65
to
b451158
Compare
@dAdAbird my concern was that if we only get the tuple and do not copy it, we might find issues because we reuse the same memory again for the next tuple. If I understand the API correctly, this shouldn't happen, but it seemed safer to disable get, and force copy. Looks like I was wrong about that. |
Before merge I think we should remove test it this PR because it's isolated case from PG regression suite. Not sure that we need it in tde repo. What do you think? |
https://perconadev.atlassian.net/browse/PG-1056