Skip to content

Tighten up fallback to per-block encryption on Encryption sequencer#189

Merged
avalerio-tkd merged 7 commits into
mainfrom
av_integr_sequencer_011
Dec 9, 2025
Merged

Tighten up fallback to per-block encryption on Encryption sequencer#189
avalerio-tkd merged 7 commits into
mainfrom
av_integr_sequencer_011

Conversation

@avalerio-tkd

Copy link
Copy Markdown
Collaborator
  • On Encryption Sequencer, declare explicitly the conditions to allow fallback to per-block encryption.
  • Cleanup use of DBPSUnsupportedException across util functions.

… fallback to per-block encryption.

- Cleanup use of DBPSUnsupportedException across util functions.
Comment thread src/server/compression_utils.cpp Outdated

namespace dbps::compression {

// TODO(Issue #188): Add support for other compressions.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if this is something we want to tackle as part of the project, keep the TODO. if it's a future extension, I recommend removing the todo.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread src/server/encryption_sequencer.cpp Outdated
compression_ == CompressionCodec::SNAPPY);

// Format: Only PLAIN is supported
// TODO(Issue #187): Add support for other encodings.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similar to above - let's keep the TODO only if it's work we plan to complete in the project. Else, let's move these as notes for the future.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread src/server/parquet_utils.cpp
Comment thread src/server/encryption_sequencer.cpp
Comment thread src/server/encryption_sequencer.cpp Outdated
Comment on lines 188 to 189

@argmarco-tkd argmarco-tkd Dec 8, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can we move these two statements closer to the code which performs per-value encryption?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only place possible with the current code structure. We'll discuss that offline.

@avalerio-tkd avalerio-tkd left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review. Let's chat offline a bit about the structure of the ConvertAndEncrypt try-catch.

Comment thread src/server/compression_utils.cpp Outdated

namespace dbps::compression {

// TODO(Issue #188): Add support for other compressions.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread src/server/encryption_sequencer.cpp Outdated
compression_ == CompressionCodec::SNAPPY);

// Format: Only PLAIN is supported
// TODO(Issue #187): Add support for other encodings.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread src/server/encryption_sequencer.cpp
Comment thread src/server/encryption_sequencer.cpp Outdated
Comment on lines 188 to 189

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only place possible with the current code structure. We'll discuss that offline.

Comment thread src/server/parquet_utils.cpp
…llback.

- Moved setting per-value metadata closer to the rest of the code.
@avalerio-tkd

Copy link
Copy Markdown
Collaborator Author

Thanks @argmarco-tkd for the review. Could you PTAL?

@avalerio-tkd avalerio-tkd merged commit d355929 into main Dec 9, 2025
2 checks passed
@avalerio-tkd avalerio-tkd deleted the av_integr_sequencer_011 branch December 9, 2025 03:57
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