Skip to content

refactor: optimize Table accessors and TimePoint passing - #414

Merged
wgtmac merged 1 commit into
apache:mainfrom
wgtmac:minor_refactor
Dec 17, 2025
Merged

refactor: optimize Table accessors and TimePoint passing#414
wgtmac merged 1 commit into
apache:mainfrom
wgtmac:minor_refactor

Conversation

@wgtmac

@wgtmac wgtmac commented Dec 16, 2025

Copy link
Copy Markdown
Member
  • Update Table::location() and Table::metadata_file_location() to return std::string_view instead of const std::string& to avoid unnecessary reference indirection.
  • Change TimePointMs and TimePointNs arguments and return types to pass-by-value in Table and utility functions, as they are small types.
  • Expose Table::metadata() accessor to allow access to the underlying table metadata.
  • Clean up unused variable declarations in avro_schema_util.cc.

- Update `Table::location()` and `Table::metadata_file_location()` to return `std::string_view` instead of `const std::string&` to avoid unnecessary reference indirection.
- Change `TimePointMs` and `TimePointNs` arguments and return types to pass-by-value in `Table` and utility functions, as they are small types.
- Expose `Table::metadata()` accessor to allow access to the underlying table metadata.
- Clean up unused variable declarations in `avro_schema_util.cc`.
const auto& expected_value_field = map_type.value();

FieldProjection result, key_projection, value_projection;
FieldProjection result;

@HuaHuaY HuaHuaY Dec 16, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How about adding readability-isolate-declaration in .clang-tidy? We can do this in next PR.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sounds good.

@wgtmac
wgtmac merged commit 25daf33 into apache:main Dec 17, 2025
10 checks passed
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