Skip to content

fix(tesseract): Honor memberToAlias for granularized time dimensions#11142

Open
ovr wants to merge 2 commits into
masterfrom
fix/tesseract-member-to-alias-granularitzed-td
Open

fix(tesseract): Honor memberToAlias for granularized time dimensions#11142
ovr wants to merge 2 commits into
masterfrom
fix/tesseract-member-to-alias-granularitzed-td

Conversation

@ovr

@ovr ovr commented Jun 23, 2026

Copy link
Copy Markdown
Member

In a wrapped SQL-API query, cubesql pushes a grouped CubeScan whose granularized time-dimension column it references by an explicit alias, sent via memberToAlias keyed {member}.{granularity} (e.g. orders.created_at.month -> datetrunc_utf8__). Regular dimensions and measures already honor this override, but TimeDimensionSymbol computed its alias as {base alias}_{granularity} (orders__created_at_month) and ignored the override. The wrapper then referenced a column the CubeScan never emitted -> column "datetrunc_utf8__" does not exist.

Look up the memberToAlias override for the granularized member in compile_time_dimensions and pass it through a new TimeDimensionSymbol::new_with_alias. When no override is present the alias is unchanged, so non-pushdown queries are unaffected.

@github-actions github-actions Bot added the rust Pull requests that update Rust code label Jun 23, 2026
@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Claude finished @ovr's task in 1m 8s —— View job


Code Review — Verdict: LGTM (1 low, 1 nit)

Review details
  • Read diff for time_dimension_symbol.rs, query_properties_compiler.rs, common_sql_generation.rs
  • Verify alias_for_member lookup key matches cubesql memberToAlias convention
  • Check derived-symbol code paths (change_granularity, reference_member, get_dependencies_as_time_dimensions)
  • Test coverage

Summary

The fix is well-scoped and correct: when compile_time_dimensions builds a TimeDimensionSymbol, it now consults evaluator_compiler.alias_for_member("{member}.{granularity}") and threads the result through a new TimeDimensionSymbol::new_with_alias. When no override is present, the alias falls back to {base alias}_{granularity}, so non-pushdown paths are untouched. Comments explain the dotted-key convention from cubesql.

Observations

Low — derived-symbol constructors still call Self::new and drop the override
time_dimension_symbol.rs:119,172,257 (change_granularity, get_dependencies_as_time_dimensions, reference_member) all construct a fresh TimeDimensionSymbol via Self::new, which forwards alias_override = None. For change_granularity this is arguably correct — the override is keyed by the old granularity, so a re-granularized symbol shouldn't inherit it. For reference_member and get_dependencies_as_time_dimensions the behavior is murkier: a wrapped-reference variant of the same logical member loses the explicit projection alias. If the SQL-API wrapper ever flows through one of those paths the original bug could resurface. Worth a follow-up to either (a) propagate the override or (b) document why these paths are guaranteed not to reach the wrapper.

Nit — alias_override = Some("") would silently win over the default
time_dimension_symbol.rs:53-54 uses unwrap_or_else, so an empty-string override would produce an empty SQL alias. cubesql shouldn't send that today, but a .filter(|s| !s.is_empty()) on the lookup site would make this self-defensive.

Praise

  • The doc-comments on the call site and on new_with_alias clearly capture why the dotted key is used and what the fallback is — exactly the kind of WHY that survives refactors.
  • The test (test_member_to_alias_time_dimension_granularity) asserts both the positive (override appears) and negative (default alias is absent) cases — that pairing is what catches a regression where both aliases get emitted.
• [branch](https://github.com/cube-js/cube/tree/fix/tesseract-member-to-alias-granularitzed-td)

@ovr ovr force-pushed the fix/tesseract-member-to-alias-granularitzed-td branch from cba842e to bdec914 Compare June 23, 2026 12:58
@ovr ovr requested a review from a team as a code owner June 23, 2026 12:58
In a wrapped SQL-API query, cubesql pushes a grouped CubeScan whose
granularized time-dimension column it references by an explicit alias,
sent via `memberToAlias` keyed `{member}.{granularity}` (e.g.
`orders.created_at.month` -> `datetrunc_utf8__`). Regular dimensions and
measures already honor this override, but `TimeDimensionSymbol` computed
its alias as `{base alias}_{granularity}` (`orders__created_at_month`)
and ignored the override. The wrapper then referenced a column the
CubeScan never emitted -> `column "datetrunc_utf8__" does not exist`.

Look up the `memberToAlias` override for the granularized member in
`compile_time_dimensions` and pass it through a new
`TimeDimensionSymbol::new_with_alias`. When no override is present the
alias is unchanged, so non-pushdown queries are unaffected.
@github-actions github-actions Bot added the javascript Pull requests that update Javascript code label Jun 23, 2026
@claude

claude Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

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


Code Review

Verdict: LGTM — small, well-scoped fix. No issues found.

Details
  • Read PR diff (4 files, +121/-7)
  • Verify implementation against described root cause
  • Check test coverage (Rust unit + JS integration)
  • Check for dedup vs existing review threads (none)

What the fix does

TimeDimensionSymbol previously hard-coded its alias to {base alias}_{granularity}, ignoring memberToAlias. The fix:

  • Adds TimeDimensionSymbol::new_with_alias(...) accepting an Option<String> override, with the old new(...) delegating to it (None). Backward-compatible — non-pushdown queries unaffected.
  • In compile_time_dimensions, looks up memberToAlias for the key {base.full_name}.{granularity} (the dotted key cubesql actually sends) and threads it through. Lookup only runs when d.granularity.is_some(), which matches the wrapper's behavior.

Correctness notes

  • query_properties_compiler.rs:253-259: the dotted-key format matches the cubesql side that emits memberToAlias for granularized columns — confirmed against the PR description.
  • time_dimension_symbol.rs:53-54: when override is absent, falls back to format!("{}_{}", base_symbol.alias(), name_suffix) — identical to prior behavior. The base symbol's alias itself already honors its own memberToAlias override, so the un-granularized case is untouched.
  • name_suffix defaults to "day" when granularity is None; the override lookup is gated on granularity.is_some(), so a user keying memberToAlias as member.day without an explicit granularity wouldn't be honored. That's fine — cubesql sends an explicit granularity when it sends the dotted alias key — but worth being aware of.

Tests

  • common_sql_generation.rs: unit test asserts both that the override appears ("td_month_alias") and that the default fallback does not (visitors__created_at_month). Good positive + negative assertion.
  • sql-generation.test.ts: end-to-end integration test exercises the full SQL API path with multiple memberToAlias entries (regular dim + measures + granularized td) and checks result-row column names. Good coverage.

Nits / non-blocking

  • The // Honor an explicit memberToAlias override … comment block in query_properties_compiler.rs:249-252 is a touch heavy on per-CLAUDE.md "comments explain WHY not WHAT" — but in this case the why (dotted-key contract with cubesql) is genuinely non-obvious to a future reader, so it earns its place. Fine as-is.
  • No changelog entry needed — Lerna will handle it on release.
• Branch: `fix/tesseract-member-to-alias-granularitzed-td`

@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.18%. Comparing base (4d4b8cd) to head (47b7cc3).

Additional details and impacted files
@@           Coverage Diff           @@
##           master   #11142   +/-   ##
=======================================
  Coverage   79.18%   79.18%           
=======================================
  Files         474      474           
  Lines       96880    96880           
  Branches     3558     3558           
=======================================
+ Hits        76712    76714    +2     
+ Misses      19650    19648    -2     
  Partials      518      518           
Flag Coverage Δ
cube-backend 58.75% <ø> (+0.01%) ⬆️
cubesql 83.68% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

javascript Pull requests that update Javascript code rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant