Skip to content

API, Core: Make TestInclusiveMetricsEvaluator/TestInclusiveMetricsEvaluatorWithExtract/TestInclusiveMetricsEvaluatorWithTransforms extendable - #17430

Open
nastra wants to merge 4 commits into
apache:mainfrom
nastra:extend-metrics-eval-tests
Open

API, Core: Make TestInclusiveMetricsEvaluator/TestInclusiveMetricsEvaluatorWithExtract/TestInclusiveMetricsEvaluatorWithTransforms extendable#17430
nastra wants to merge 4 commits into
apache:mainfrom
nastra:extend-metrics-eval-tests

Conversation

@nastra

@nastra nastra commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

This makes the three test classes extendible so that we can introduce separate test classes that use a content stats evaluator but run through the same set of tests

AI Disclosure

  • Model: Claude Opus 5
  • Platform/Tool: Cursor
  • Human Oversight: fully reviewed
  • Prompt Summary: Refactor TestInclusiveMetricsEvaluator, TestInclusiveMetricsEvaluatorWithExtract, and TestInclusiveMetricsEvaluatorWithTransforms into extendable harnesses by hoisting inline file fixtures into protected factory methods and routing evaluation through a protected shouldRead hook, so subclasses can run the same expression suite against a different file statistics representation.

nastra added 3 commits July 30, 2026 10:23
The single_value_nulls fixtures in testNotEqWithSingleValue and
testNotInWithSingleValue reported 2 nulls for field 3, the "required"
column. A required column cannot contain nulls, so no writer produces
those metrics. Move both to the optional "some_empty" column,
which keeps the case the tests cover: a file whose bounds hold a single
value still has to be read when the column also contains nulls, because
those nulls match != and NOT IN.

The NOT IN fixture also reported a NaN count for a string column, which
only float and double columns track.
The range_of_values and single_value fixtures reported a NaN count for
field 3, a string column. Only float and double columns track NaN counts,
which is why the single_value fixture in testNotEqWithSingleValue already
passed null for them.

Generated-by: Cursor
`FILE_6` in `TestInclusiveMetricsEvaluator` carried value and null counts for the
`required_address` and `optional_address` structs. Parquet produces metrics for leaf
columns only in `ParquetMetrics.MetricsVisitor.struct` and returns just the concatenation of
its children and never emits a `FieldMetrics` for the struct — so no Parquet file yields
those entries. They made `notNull("optional_address")` appear prunable when in practice it is not
@nastra
nastra force-pushed the extend-metrics-eval-tests branch from 9f9524d to 90461f0 Compare July 30, 2026 09:33
…luatorWithExtract/TestInclusiveMetricsEvaluatorWithTransforms extendable
@nastra
nastra force-pushed the extend-metrics-eval-tests branch from 90461f0 to 60d7aaa Compare July 30, 2026 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant