Skip to content

PG-1444 Implement deletion of relation keys when dropping relations #238

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

Open
wants to merge 2 commits into
base: TDE_REL_17_STABLE
Choose a base branch
from

Conversation

jeltz
Copy link
Collaborator

@jeltz jeltz commented Apr 19, 2025

This replaces the old way we deleted keys which was built for tde_heap_basic
with deleting the the relation key when smgr_unlink() is called on the
main fork. This function is always called after commit/abort when a
relation deletion has been registered, even if no main fork would exist.

This approach means we do not need to WAL log any event for deleting
relation keys, the normal SMGR unlink also handles that which fits well
into the current approach of doing most of the encryption at the SMGR
layer.

This code is dead and there is no plan to re-use it any time soon.
@jeltz jeltz requested review from dutow and dAdAbird as code owners April 19, 2025 02:11
@codecov-commenter
Copy link

codecov-commenter commented Apr 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 76.18%. Comparing base (ca37d73) to head (58dcf89).

❌ Your project status has failed because the head coverage (76.18%) is below the target coverage (90.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files
@@                  Coverage Diff                  @@
##           TDE_REL_17_STABLE     #238      +/-   ##
=====================================================
+ Coverage              76.15%   76.18%   +0.02%     
=====================================================
  Files                     23       22       -1     
  Lines                   2520     2456      -64     
  Branches                 393      384       -9     
=====================================================
- Hits                    1919     1871      -48     
+ Misses                   524      511      -13     
+ Partials                  77       74       -3     
Components Coverage Δ
access 72.90% <100.00%> (+1.38%) ⬆️
catalog 86.24% <ø> (ø)
common 92.50% <ø> (ø)
encryption 71.90% <ø> (ø)
keyring 69.87% <ø> (ø)
src 52.80% <ø> (-0.16%) ⬇️
smgr 96.96% <100.00%> (+0.19%) ⬆️
transam ∅ <ø> (∅)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jeltz jeltz force-pushed the tde/delete-relkeys branch from 3c1bb0e to 8ccb9e6 Compare April 19, 2025 02:20
Replaces the old way we deleted keys which was built for tde_heap_basic
with deleting the the relation key when smgr_unlink() is called on the
main fork. This function is always called after commit/abort when a
relation deletion has been registered, even if no main fork would exist.

This approach means we do not need to WAL log any event for deleting
relation keys, the normal SMGR unlink also handles that which fits well
into the current approach of doing most of the encryption at the SMGR
layer.

We also remove the subtransaction test which is no longer useful since
it tested things very specific to the old key deleteion.
@jeltz jeltz force-pushed the tde/delete-relkeys branch from 8ccb9e6 to 58dcf89 Compare April 19, 2025 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants