Skip to content

Renaming Format to Encoding for parquet encodings#207

Merged
avalerio-tkd merged 3 commits into
mainfrom
av_format_to_encodings_017
Jan 24, 2026
Merged

Renaming Format to Encoding for parquet encodings#207
avalerio-tkd merged 3 commits into
mainfrom
av_format_to_encodings_017

Conversation

@avalerio-tkd

@avalerio-tkd avalerio-tkd commented Jan 23, 2026

Copy link
Copy Markdown
Collaborator
  • Straight rename of Format to Encoding to match parquet::Encoding
  • Update all related code to use the new Encoding enum

@avalerio-tkd
- Straight rename of Format to Encoding to match parquet::Encoding
bbe9cdd 
- Update all related code to use the new Encoding enum
@avalerio-tkd

avalerio-tkd commented Jan 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

@argmarco-tkd a question on this PR is if when merged we will need to update a dependency/copy of enums.h or dbpa_interface.h or another dependency. In the latter, we're just updating a comment, so not sure. If it's too much trouble just for a comment, I can revert the change on that file.

@avalerio-tkd avalerio-tkd mentioned this pull request Jan 24, 2026
Closed
@argmarco-tkd

argmarco-tkd commented Jan 24, 2026

Copy link
Copy Markdown
Collaborator

@argmarco-tkd a question on this PR is if when merged we will need to update a dependency/copy of enums.h or dbpa_interface.h or another dependency. In the latter, we're just updating a comment, so not sure. If it's too much trouble just for a comment, I can revert the change on that file.

I think we should be OK. The interface itself in dbpa_interface.h did not change, we should be able to consume it without any changes/work on the Arrow side.

argmarco-tkd
argmarco-tkd approved these changes Jan 24, 2026
View reviewed changes

@argmarco-tkd argmarco-tkd left a comment

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.

LGTM - it's a simple (albeit long :) ) refactor job. Ship it!

Comment thread src/server/encryption_sequencer.cpp Outdated
}

// TODO: Rename this method so it captures better the flow of decompress/format and encrypt/decrypt operations.
// TODO: Rename this method so it captures better the flow of decompress/encoding and encrypt/decrypt operations.

@argmarco-tkd argmarco-tkd Jan 24, 2026

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.

orthogonal to this refactor: are we going to do this? if so, let's create a task.

@avalerio-tkd avalerio-tkd Jan 24, 2026

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.

How about DecodeAndEncrypt and DecryptAndEncode ? @argmarco-tkd

@argmarco-tkd argmarco-tkd Jan 24, 2026

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.

Thanks - those names work IMO! @avalerio-tkd

@avalerio-tkd avalerio-tkd Jan 24, 2026

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.

Perfect. Will rename them as such and merge afterwards. Thanks for the review and comments.

@avalerio-tkd
avalerio-tkd merged commit 44b2e79 into main Jan 24, 2026
2 checks passed
@avalerio-tkd
avalerio-tkd deleted the av_format_to_encodings_017 branch January 24, 2026 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@argmarco-tkd argmarco-tkd argmarco-tkd approved these changes
@sofia-tekdatum sofia-tekdatum Awaiting requested review from sofia-tekdatum

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@avalerio-tkd @argmarco-tkd