Skip to content

PKG-895: Allow citus and timescaledb to compile #469

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 1 commit into
base: TDE_REL_17_STABLE
Choose a base branch
from

Conversation

dutow
Copy link
Collaborator

@dutow dutow commented Jul 14, 2025

This commit reintroduces the old method signatures of some methods we changed to support pg_tde. The changed method signatures still exists with a "2" suffix, and all internal code calls the "2" methods.

The reintroduced original methods also check a global flag which can disable them - if this happens, they report a FATAL error instead. This variable is activated by pg_tde, which means that if something tries to use these while pg_tde is loaded, and with that, possibly ignores its file tracking mechanism, it reports an error.

In practice this means that timescaledb, or the columnar storage in citus won't work if pg_tde is loaded in the shared_preload_libraries, but they can be used with our distribution without pg_tde enabled.

Also verified that both citus and timescaledb do compile with these changes, and their basic test suite can be run againts our fork.

This commit reintroduces the old method signatures of some methods we
changed to support pg_tde. The changed method signatures still exists
with a "2" suffix, and all internal code calls the "2" methods.

The reintroduced original methods also check a global flag which can
disable them - if this happens, they report a FATAL error instead. This
variable is activated by pg_tde, which means that if something tries to
use these while pg_tde is loaded, and with that, possibly ignores its
file tracking mechanism, it reports an error.

In practice this means that timescaledb, or the columnar storage in
citus won't work if pg_tde is loaded in the shared_preload_libraries,
but they can be used with our distribution without pg_tde enabled.

Also verified that both citus and timescaledb do compile with these
changes, and their basic test suite can be run againts our fork.
@dutow dutow requested a review from dAdAbird as a code owner July 14, 2025 21:16
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.47%. Comparing base (09b2af7) to head (3e9191e).
Report is 2 commits behind head on TDE_REL_17_STABLE.

❌ Your project status has failed because the head coverage (83.47%) 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     #469      +/-   ##
=====================================================
- Coverage              84.22%   83.47%   -0.75%     
=====================================================
  Files                     21       21              
  Lines                   2687     2766      +79     
  Branches                 422      434      +12     
=====================================================
+ Hits                    2263     2309      +46     
- Misses                   341      371      +30     
- Partials                  83       86       +3     
Components Coverage Δ
access 81.11% <ø> (ø)
catalog 87.89% <ø> (ø)
common 77.77% <ø> (ø)
encryption 73.45% <ø> (ø)
keyring 73.21% <85.55%> (+1.90%) ⬆️
src 87.55% <60.93%> (-4.01%) ⬇️
smgr 94.85% <ø> (-0.03%) ⬇️
transam ∅ <ø> (∅)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AndersAstrand
Copy link
Collaborator

Is the strategy to duplicate all functions we modify in the core code?

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.

3 participants