Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,13 @@ impl QueryPropertiesCompiler {
}
let source_measure_compiled =
evaluator_compiler.add_measure_evaluator(source_measure.clone())?;
let symbol = if let Ok(source_measure) = source_measure_compiled.as_measure() {
// A view measure is a reference wrapper whose type collapses to
// `number` (Calculated), which rejects additional filters. Resolve
// the reference chain to the owning cube measure so the filter is
// pushed inside the aggregation (`SUM(CASE WHEN ... END)`); a no-op
// for plain cube measures.
let resolved_source = source_measure_compiled.clone().resolve_reference_chain();

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.

@claude please double verify that without this change, test test_many_to_one_view_patched_measure_filter fails. Please verify it carefully

@claude claude Bot Jun 22, 2026

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.

Claude finished @ovr's task in 2m 45s —— View job


Verified: the test fails without the change ✔

  • Locate the change and the regression test
  • Trace the exact failure path (cargo is blocked by the sandbox in this env — verified by rigorous code-path analysis instead)
  • Confirm it fails (and how)
  • Report findings

Answer to @ovr: Yes — without this change test_many_to_one_view_patched_measure_filter fails, and it fails before the CASE WHEN assert or the PG snapshot ever run, on build_sql_from_options(...)? at member_expressions_on_views.rs:246.

⚠️ I was unable to run cargo test directly — cargo/git-mutation commands are blocked by the sandbox in this CI environment (every invocation returns "requires approval"). To compensate I traced the failure end-to-end through the source. If you want a live run, the earlier [View job] CI runs already exercised this test green with the fix; the trace below shows what happens once the fix is removed.

The failing path, step by step (with the fix reverted)

The view measure many_to_one_view.root_val_sum compiles to a MeasureSymbol with:

  • is_reference = true — set at measure_symbol.rs:748 (is_view && is_sql_is_direct_ref), and
  • kind = Calculated(Number) — because the view exposes the measure with measure_type = "number", so MeasureKind::from_type_str("number", …) yields Calculated (measure_symbol.rs:716).

With the fix removed, the branch reads if let Ok(source_measure) = source_measure_compiled.as_measure():

  1. as_measure() succeeds. The symbol is a MemberSymbol::Measure, so as_measure() returns Ok (member_symbol.rs:297-305). So we enter the new_patched branch — the error path, not the harmless else fallback.
  2. new_patched rejects the filter. add_filters is non-empty (the ad-hoc {…root_test_dim} = 'rt_x'), result_kind is Calculated(Number), and Calculated hits the _ => false arm of supports_additional_filters() (measure_kinds/mod.rs:184-197). So new_patched returns Err("Unsupported additional filters for measure root_val_sum type number") (measure_symbol.rs:194-200).
  3. The ? propagates. query_properties_compiler.rs:283 uses ?, so the Err bubbles out of the compile step → out of build_sql_from_options.
  4. The test fails. Line 246 is let sql = ctx.build_sql_from_options(options.clone())?; — the ? returns the Err as the test's Result, so the #[tokio::test] reports failure with that error message. (Earlier, before chore: up code quality, this was an .unwrap() and would have panicked at the same point — same outcome, louder.)

Why the fix makes it pass

resolve_reference_chain() walks reference_member() (member_symbol.rs:214-220) from the view measure to the owning cube measure, whose kind is Aggregated(Sum). Sum does support additional filters, so new_patched succeeds and the filter is rendered inside the aggregation (SUM(CASE WHEN … END)) — which is exactly what the assert!(sql.contains("CASE WHEN")) and the PG snapshot (30.00 = 10 + 20) then verify.

Conclusion: The regression guard is genuine. Remove the two changed lines and the test stops at step 4 with Unsupported additional filters for measure root_val_sum type number — it never reaches the assertion or snapshot. The fix is load-bearing.
· fix/tesseract-ad-hoc-measure-aggregation

let symbol = if let Ok(source_measure) = resolved_source.as_measure() {
let patched_measure =
source_measure.new_patched(new_measure_type, filters_to_add)?;
MemberSymbol::new_measure(patched_measure)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
use crate::cube_bridge::member_expression::MemberExpressionExpressionDef;
use crate::cube_bridge::member_expression::{ExpressionStruct, MemberExpressionExpressionDef};
use crate::cube_bridge::member_sql::MemberSql;
use crate::cube_bridge::options_member::OptionsMember;
use crate::test_fixtures::cube_bridge::{
members_from_strings, MockBaseQueryOptions, MockMemberExpressionDefinition, MockMemberSql,
MockSchema,
members_from_strings, MockBaseQueryOptions, MockExpressionStruct,
MockMemberExpressionDefinition, MockMemberSql, MockSchema, MockStructWithSqlMember,
};
use crate::test_fixtures::test_utils::TestContext;
use cubenativeutils::CubeError;
use indoc::indoc;
use std::rc::Rc;

Expand All @@ -25,6 +26,34 @@ fn make_member_expression(expression_name: &str, cube_name: &str, sql: &str) ->
OptionsMember::MemberExpression(Rc::new(expr))
}

// Builds a `PatchMeasure` member expression that adds one ad-hoc CASE-WHEN
// filter to `source_measure` — the SQL-API mechanism for pushing a filter
// inside a measure's aggregation.
fn make_patched_measure(
expression_name: &str,
cube_name: &str,
source_measure: &str,
filter_sql: &str,
) -> OptionsMember {
let filter = MockStructWithSqlMember::builder()
.sql(filter_sql.to_string())
.build();
let expr_struct = MockExpressionStruct::builder()
.expression_type("PatchMeasure".to_string())
.source_measure(Some(source_measure.to_string()))
.add_filters(Some(vec![Rc::new(filter)]))
.build();
let expr = MockMemberExpressionDefinition::builder()
.expression_name(Some(expression_name.to_string()))
.name(Some(expression_name.to_string()))
.cube_name(Some(cube_name.to_string()))
.expression(MemberExpressionExpressionDef::Struct(
Rc::new(expr_struct) as Rc<dyn ExpressionStruct>
))
.build();
OptionsMember::MemberExpression(Rc::new(expr))
}

fn build_options_with_member_expression(
ctx: &TestContext,
extra_measure: OptionsMember,
Expand Down Expand Up @@ -183,3 +212,51 @@ async fn test_many_to_one_view_child_distinct_dim() {
insta::assert_snapshot!(result);
}
}

// Regression for 58b5c96 (push ad-hoc filters into view measure aggregation).
// A PatchMeasure adding a CASE-WHEN filter to a measure exposed through a view
// (`root_val_sum`, a SUM) must resolve the reference chain to the owning cube
// measure so the filter is pushed inside the aggregation. Before the fix the
// view measure's type collapsed to `number` (Calculated) and build_sql failed
// with "Unsupported additional filters for measure ... type number".
// root_test_dim='rt_x' → roots 1,2 → SUM = 10 + 20 = 30.
#[tokio::test(flavor = "multi_thread")]
async fn test_many_to_one_view_patched_measure_filter() -> Result<(), CubeError> {
let ctx = create_test_context();
let expr = make_patched_measure(
"filtered_root_sum",
"many_to_one_view",
"many_to_one_view.root_val_sum",
"{many_to_one_view.root_test_dim} = 'rt_x'",
);

let options = Rc::new(
MockBaseQueryOptions::builder()
.cube_evaluator(ctx.query_tools().cube_evaluator().clone())
.base_tools(ctx.query_tools().base_tools().clone())
.join_graph(ctx.query_tools().join_graph().clone())
.security_context(ctx.security_context().clone())
.measures(Some(vec![expr]))
.build(),
);

// build_sql itself is the regression guard: before the fix it returned
// Err("Unsupported additional filters ...") and this propagated as a
// test failure.
let sql = ctx.build_sql_from_options(options.clone())?;
// The ad-hoc filter is pushed inside the aggregation (measure_filter.rs
// renders `CASE WHEN <filter> THEN <result> END`), not as an outer WHERE.
assert!(
sql.contains("CASE WHEN"),
"ad-hoc filter must be pushed inside the aggregation, got: {sql}"
);

if let Some(result) = ctx
.try_execute_pg_from_options(options, "many_to_one_tables.sql")
.await
{
insta::assert_snapshot!(result);
}

Ok(())
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
source: cubesqlplanner/src/tests/member_expressions_on_views.rs
expression: result
---
filtered_root_sum
-----------------
30.00
Loading