-
Notifications
You must be signed in to change notification settings - Fork 22
support specify field ids for table read context #59
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
base: main
Are you sure you want to change the base?
support specify field ids for table read context #59
Conversation
| /// @return Reference to this builder for method chaining. | ||
| /// @note Currently supports top-level field selection. Future versions may support | ||
| /// nested field selection using ArrowSchema for more granular projection | ||
| ReadContextBuilder& SetReadFieldIds(const std::vector<int32_t>& read_field_ids); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider add comments to clarify the behavior when both SetReadFieldIds and SetReadSchema are called. E.g., SetReadFieldIds and SetReadSchema are mutually exclusive. Calling both may result in undefined behavior. Only one should be used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or fieldId take priority? Or should this be treated as an error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, will update according to your suggestion.
| read_data_fields.reserve(context->GetReadFieldIds().size()); | ||
| for (const auto& field_id : context->GetReadFieldIds()) { | ||
| // if enable row tracking or data evolution, check special fields | ||
| if (core_options.RowTrackingEnabled() && field_id == SpecialFields::RowId().Id()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If users need to read certain special fields, they will need to know the corresponding field IDs. In that case, we should consider exposing the SpecialFields to users in /include directory.
Purpose
paimon-cpp currently only support select columns using column name, the patch is used to support select columns using field id as well.
Linked issue: #34
Tests
add UT internal_read_context_test.cpp#TestReadWithSpecifiedFieldId
update UT read_context_test.cpp
API and Format
add SetReadFieldIds(const std::vector<int32_t>& read_field_ids) in class ReadContextBuilder
Documentation