Repository navigation
feat: approximate-by-default measures with exact_expression and an inline exact chip - #10032
Conversation
…inline exact chip Measures can define an `exact_expression` next to their approximate `expression`. Queries flag measures with `exact`; the runtime resolves the exact set once per query, skips rollups for it and honours the `rill.metrics.exact_measures` kill switch. Explore and canvas dashboards show a `≈`/`=` chip next to such measures and keep the selection in tab memory, per surface (one chip per chart in canvas).
AdityaHegde
left a comment
There was a problem hiding this comment.
High level review, just one comment about executor code.
| return nil, err | ||
| } | ||
|
|
||
| exactMeasures, err := e.resolveExactMeasures(qry) |
There was a problem hiding this comment.
Instead of passing around exact measures everywhere how about running a rewrite at once place? Not sure if there is a good path for this.
There was a problem hiding this comment.
Done: rewriteQueryExactMeasures resolves the set once at the start of Query and Export; nothing is passed around any more (details in the exact.go threads).
- match the Connect `MetricsViewAggregation` endpoint instead of the REST path - give the `publishers` measure a deterministic default expression - rename the fixture to `AdBids_precision_metrics` so that it sorts after `AdBids_metrics` and does not become the canvas editor's default metrics view
…measures # Conflicts: # proto/gen/rill/runtime/v1/resources.pb.go
…measures # Conflicts: # proto/gen/rill/runtime/v1/resources.pb.go # runtime/server/downloads.go
… passing the set around - `rewriteQueryExactMeasures` runs after validation in `Query` and `Export`: it resolves the exact set, writes it back into the query's `exact` flags (filter subqueries included) and applies the kill switch - `NewAST` resolves the set from the query it receives; derived queries (percent-of-total totals, two-phase base query, Druid TopN inner query, where-subqueries) copy the flags with the measures - removes `NewASTWithExactMeasures`, `resolveExactMeasures`, the private `query` and the extra parameter on the four rewrites - the set is closed under the derived-measure rule the frontend already uses: a measure calculated from an exact measure makes its other exact-capable inputs exact - tests for the fixpoint, `FlagExactMeasures` and a two-phase base query sharing a filter subquery
…e-resolving them per AST - `Query.ExactMeasures` (not part of the wire format) holds the set resolved once by `rewriteQueryExactMeasures` - derived queries (percent-of-total totals, two-phase base query, Druid TopN inner query, where-subqueries) copy it like their other context fields; the rollup check reads it - `NewAST` reads it and resolves from the flags only for queries that did not pass through the executor (schema probe, search) - removes `FlagExactMeasures`; the percent-of-total base measure is built as on main again
AdityaHegde
left a comment
There was a problem hiding this comment.
TDD's totals rows seem to be dependant on chart total's toggle rather than the toggle in TDD table.
AdityaHegde
left a comment
There was a problem hiding this comment.
Just a few nits remaining, approving
|
Do we need to allow exact behaviour for derived and ad hoc measures? Not supporting it will remove the parsing of sql which seems unnecessary for an optional feature or if we need to support exact on individual measures rather than at query level enabling for all measures. |
| typ = runtimev1.MetricsViewSpec_MEASURE_TYPE_TIME_COMPARISON | ||
| default: | ||
| return fmt.Errorf(`invalid measure type %q (allowed values: simple, derived, time_comparison)`, measure.Type) | ||
| } |
There was a problem hiding this comment.
disallow it on non simple measures? add a check here after switch like this
if measure.ExactExpression != "" && (typ != runtimev1.MetricsViewSpec_MEASURE_TYPE_SIMPLE || window != nil) {
return fmt.Errorf(`measure %q: "exact_expression" can only be set on simple measures`, measure.Name)
}
There was a problem hiding this comment.
Done, with windowed measures excluded as well.
- `exact_expression` is only allowed on simple measures without a window (parser error otherwise) - the exact set is the flagged measures that define an exact_expression; no closure over referenced measures, no fixpoint, no parsing of ad hoc expressions. A derived measure is calculated from the measures as the query computes them - UI: derived and ad hoc measures show no chip; the dashed "inherited" chip state and its message are removed - the search paths run `rewriteQueryExactMeasures` - the Time Dimension Detail page is one surface: its chart, totals and table follow the chip in its header - the canvas chart builds its exact request set from its selection alone, without reading the metrics view spec or converting ad hoc measures - docs, schema description and the proto comment on `MetricsViewAggregationMeasure.exact` updated, bindings and reference docs regenerated
|
@pjain1 agreed and done in a83302a: @AdityaHegde the TDD totals row is fixed in the same push: the Time Dimension Detail page is one surface now, so its chart, totals row and table all follow the chip in the TDD header. |
…ected Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…inline exact chip (#10032) * feat: approximate-by-default measures with `exact_expression` and an inline exact chip Measures can define an `exact_expression` next to their approximate `expression`. Queries flag measures with `exact`; the runtime resolves the exact set once per query, skips rollups for it and honours the `rill.metrics.exact_measures` kill switch. Explore and canvas dashboards show a `≈`/`=` chip next to such measures and keep the selection in tab memory, per surface (one chip per chart in canvas). * test: fix the exact measures e2e spec and its fixture - match the Connect `MetricsViewAggregation` endpoint instead of the REST path - give the `publishers` measure a deterministic default expression - rename the fixture to `AdBids_precision_metrics` so that it sorts after `AdBids_metrics` and does not become the canvas editor's default metrics view * refactor: resolve exact measures once with a query rewrite instead of passing the set around - `rewriteQueryExactMeasures` runs after validation in `Query` and `Export`: it resolves the exact set, writes it back into the query's `exact` flags (filter subqueries included) and applies the kill switch - `NewAST` resolves the set from the query it receives; derived queries (percent-of-total totals, two-phase base query, Druid TopN inner query, where-subqueries) copy the flags with the measures - removes `NewASTWithExactMeasures`, `resolveExactMeasures`, the private `query` and the extra parameter on the four rewrites - the set is closed under the derived-measure rule the frontend already uses: a measure calculated from an exact measure makes its other exact-capable inputs exact - tests for the fixpoint, `FlagExactMeasures` and a two-phase base query sharing a filter subquery * refactor: store the resolved exact measures on the query instead of re-resolving them per AST - `Query.ExactMeasures` (not part of the wire format) holds the set resolved once by `rewriteQueryExactMeasures` - derived queries (percent-of-total totals, two-phase base query, Druid TopN inner query, where-subqueries) copy it like their other context fields; the rollup check reads it - `NewAST` reads it and resolves from the flags only for queries that did not pass through the executor (schema probe, search) - removes `FlagExactMeasures`; the percent-of-total base measure is built as on main again * refactor: drop exact behavior for derived and ad hoc measures - `exact_expression` is only allowed on simple measures without a window (parser error otherwise) - the exact set is the flagged measures that define an exact_expression; no closure over referenced measures, no fixpoint, no parsing of ad hoc expressions. A derived measure is calculated from the measures as the query computes them - UI: derived and ad hoc measures show no chip; the dashed "inherited" chip state and its message are removed - the search paths run `rewriteQueryExactMeasures` - the Time Dimension Detail page is one surface: its chart, totals and table follow the chip in its header - the canvas chart builds its exact request set from its selection alone, without reading the metrics view spec or converting ad hoc measures - docs, schema description and the proto comment on `MetricsViewAggregationMeasure.exact` updated, bindings and reference docs regenerated * deprecate exact field on query level * perf: skip serializing the query context when no exact measure is selected Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Parag Jain <pjain1@apache.org> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 9c1fdd2)
Measures can define an
exact_expressionnext to their approximateexpression, and dashboards let viewers switch a measure to its exact value on demand.exact_expressionon measures (parsed, validated, in the reference docs);exactonMetricsViewAggregationMeasureandexact_measure_namesonMetricsViewTimeSeriesRequest. The executor resolves the exact set once per query (closure over referenced measures) and passes it to every rewrite and AST; rollups are skipped for exact queries;rill.metrics.exact_measuresis a kill switch. The AIquery_metrics_viewtool exposes the flag.≈/=chip next to measures with an exact variant on big numbers, leaderboard and dimension table headers, the TDD header and table, and pivot column headers. A chip switches its own surface only (the big-number row, one leaderboard, the dimension table, the TDD table or the pivot). The selection is tab memory, never written to the URL, and resets when filters, time controls or the view change. One-shot exports follow what is on screen.exact_expressionin the measures reference, a guide page for the chip, and the canvas widget docs.AdBids_exact_metricsfixture.Checklist: