Skip to content

chore: avoid reparsing partition expr in mito2 reader#8022

Draft
lyang24 wants to merge 1 commit intoGreptimeTeam:mainfrom
lyang24:fix-mito2-partition-expr-reparse
Draft

chore: avoid reparsing partition expr in mito2 reader#8022
lyang24 wants to merge 1 commit intoGreptimeTeam:mainfrom
lyang24:fix-mito2-partition-expr-reparse

Conversation

@lyang24
Copy link
Copy Markdown
Collaborator

@lyang24 lyang24 commented Apr 23, 2026

I hereby agree to the terms of the GreptimeDB CLA.

Refer to a related PR or issue link (optional)

What's changed and what's your intention?

in the read path there might be a opportunity to save a call on get region_partition_expr which link to parse_partition_expr with some json work.

PR Checklist

Please convert it to a draft if some of the following conditions are not met.

  • I have written the necessary rustdoc comments.
  • I have added the necessary unit tests and integration tests.
  • This PR requires documentation updates.
  • API changes are backward compatible.
  • Schema or data changes are backward compatible.

@github-actions github-actions Bot added size/XS docs-not-required This change does not impact docs. labels Apr 23, 2026
Signed-off-by: lyang24 <lanqingy93@gmail.com>
@lyang24 lyang24 force-pushed the fix-mito2-partition-expr-reparse branch from 56de6b0 to bf474b8 Compare April 23, 2026 06:18
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the ParquetReaderBuilder in src/mito2/src/sst/parquet/reader.rs by lifting the computation of region partition expressions out of the build_partition_filter method. The method signature is updated to accept these values as arguments, which are now passed from the caller where they were already being determined. I have no feedback to provide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs-not-required This change does not impact docs. size/XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant