Skip to content

refactor(expr): evaluate inclusive metrics from borrowed statistics - #3347

Open
unikdahal wants to merge 1 commit into
apache:mainfrom
unikdahal:expr/borrowed-metrics-evaluator
Open

unikdahal wants to merge 1 commit into
apache:mainfrom
unikdahal:expr/borrowed-metrics-evaluator

Conversation

@unikdahal

Copy link
Copy Markdown

Which issue does this PR close?

What changes are included in this PR?

InclusiveMetricsEvaluator only reads a data file's record count and its value, null and NaN counts and lower/upper bounds, but it requires a whole DataFile.

  • Add a crate-internal FileMetrics<'a> view that borrows those statistics, with From<&DataFile>.
  • Add InclusiveMetricsEvaluator::eval_metrics(filter, FileMetrics, include_empty_files); eval(filter, &DataFile, include_empty_files) now converts and delegates.
  • A record count known to be zero still excludes the file unless include_empty_files is set. An unknown record count does not by itself exclude the file; the available bounds and counts may still prune it.

No behaviour change for existing callers and no public API change (everything new is pub(crate)). This lets follow-up work evaluate whole-file statistics carried with a scan task without reconstructing a DataFile.

Are these changes tested?

Yes:

  • Existing inclusive metrics evaluator tests pass unchanged through the delegating eval.
  • New: FileMetrics built from independently constructed maps (no DataFile) prunes and keeps files from bounds and null counts with both a known and an unknown record count, keeps files whose column has no statistics, and skips a known empty file unless empty files are included.

AI Disclosure

This change was developed with assistance from Claude Code and reviewed by me. Tests and CI results come from this branch.

The inclusive metrics evaluator only reads a data file's record count and
its value, null and NaN counts and bounds, yet it requires a whole
DataFile. Introduce a crate-internal FileMetrics view that borrows those
statistics, evaluate through it, and keep the DataFile entry point as a
thin conversion.

Statistics held outside a DataFile, such as whole-file statistics carried
with a scan task, can then be evaluated without rebuilding one. A record
count known to be zero still excludes the file unless empty files are
included; an unknown record count does not by itself exclude it, while
the available bounds and counts may still prune it. Behaviour for
DataFile evaluation is unchanged.
Copilot AI balanced review requested due to automatic review settings October 4, 2026 21:58

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

This branch has not been deployed

No deployments
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.

Evaluate inclusive metrics from borrowed file statistics

2 participants