From e22241521856d4565f3c89fa421453c380471f4e Mon Sep 17 00:00:00 2001 From: Mike Nitsenko Date: Thu, 30 Jul 2026 22:02:50 +0500 Subject: [PATCH 1/2] docs: document dbt pull primary key detection and reverse joins (#11424) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * docs: document dbt pull primary key detection and reverse joins Describe the three-tier primary key ladder (dbt constraints → strict unique + not_null tests → column-name suffixes), the new pull options, and add troubleshooting for missing/incorrect keys and one-directional joins. Co-Authored-By: Claude Opus 5 (1M context) * docs: address review feedback on dbt primary key section - "Every cube needs a primary_key" — a cube can end up without one - constraints snippet now includes the enforced model contract dbt requires, plus a pointer to the test-based path for projects without contracts - name the parent option instead of "the option above"/"below" - attribute pull-time behavior to "the pull", not "Cube" Co-Authored-By: Claude Opus 5 (1M context) * docs: correct the join relationship type dbt pull generates Generated joins are always many_to_one from the cube owning the foreign key; one_to_many/one_to_one were never emitted for forward joins. Co-Authored-By: Claude Opus 5 (1M context) * docs: cover forced join keys and all-or-nothing constraints in dbt pull Verified the page against the implementation: document that join target columns are keyed regardless of the tiers, that a primary_key constraint naming an undocumented column is skipped entirely, and that foreign-key avoidance is a preference with a fallback. Order the pull options to match the settings card. Co-Authored-By: Claude Opus 5 (1M context) * docs: clarify join-target keys and drop references to server-side logs Join-referenced columns are additive on top of the tiers and apply to the referenced cube only, so the FK owner is what typically ends up keyless. Replace pointers to pull log output, which isn't surfaced to users, with the compile error and the generated .yml. Co-Authored-By: Claude Opus 5 (1M context) * docs: qualify the keyless-cube symptom in dbt pull The compile error only fires when the cube has an aggregatable measure; without one the join is dropped with no error. Quote the full message. Co-Authored-By: Claude Opus 5 (1M context) * docs: full consistency pass on the dbt primary key content - name the measure types that trigger the compile error, and the runtime "Can't find join path" failure when none is present - drop the unqualified "Cube requires a key on both ends of every join" - Limitations no longer claims tests are unused; they feed key and join detection - note that warehouse enforcement is irrelevant (the pull only parses) - a dbt relationship describes one side, rather than dbt being unable to declare both Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) --- docs-mintlify/docs/integrations/dbt.mdx | 117 ++++++++++++++++++++++-- 1 file changed, 110 insertions(+), 7 deletions(-) diff --git a/docs-mintlify/docs/integrations/dbt.mdx b/docs-mintlify/docs/integrations/dbt.mdx index 6d1f5349a688b..2e367431a8f68 100644 --- a/docs-mintlify/docs/integrations/dbt.mdx +++ b/docs-mintlify/docs/integrations/dbt.mdx @@ -144,12 +144,85 @@ automated — uses the same configuration: | **Title prefix** | `(dbt) ` | Prefix added to each generated cube's display title. | | **Model selector** | _(empty)_ | Optional dbt selector using `dbt ls --select` syntax to limit which models are pulled, e.g. `tag:cube` or `marts.*`. Leave empty to pull all models. | | **Only pull marts** | Off | When enabled, only pulls models whose path starts with the **Marts folder** value (e.g. `marts`). | -| **Auto-detect primary keys** | On | Marks `id` / `*_id` columns (and columns with a dbt `primary_key` constraint) as the cube's primary key. | +| **Auto-detect primary keys from column names** | On | Falls back to a column-name guess when dbt declares no key. See [Primary key detection](#primary-key-detection). | +| **Primary key column suffixes** | `_id` | Comma-separated suffixes treated as key columns, e.g. `_sk, _key`. The default `_id` also matches a column named `id`. Only shown when **Auto-detect primary keys from column names** is on. | +| **Detect primary keys from `unique` + `not_null` tests** | On | Treats a column carrying both dbt tests as the cube's primary key. Outranks the column-name guess. | | **Add default measures** | On | Adds a `count` measure to every cube, and `sum` measures for additive numeric columns. | | **Include descriptions** | On | Carries dbt model and column descriptions into cubes and dimensions. | | **Generate joins** | On | Infers joins between cubes from dbt `relationships` tests and foreign-key constraints. | +| **Also generate reverse joins** | Off | Adds a `one_to_many` join back to the referencing cube, so a query can start from either side. Only shown when **Generate joins** is on. | | **Infer column types from dbt catalog** | Off | Reads real warehouse column types from dbt's `catalog.json` instead of guessing from column names. See [below](#infer-column-types-from-dbt-catalog). | +### Primary key detection + +A cube's `primary_key` states the model's grain, and Cube relies on it to aggregate +correctly across joins. The pull looks for it in three tiers and uses the first one that +yields a key: + +1. **dbt `primary_key` constraints** — both column-level and model-level (composite) + constraint blocks. Always honored, even when both detection options are off. +2. **Strict `unique` + `not_null` tests** on the same column. Only tests that actually + guarantee uniqueness count — a test with `where:`, `severity: warn`, or a relaxed + `error_if` is ignored. +3. **Column-name suffixes**, `_id` by default. + +Only tier 1 can produce a **composite** key, because it's the only place where your dbt +project states the grain. Tiers 2 and 3 mark exactly one column: two independently unique +columns are two candidate keys, not a composite key. When several columns qualify, the +pull prefers the one named after the model — `stg_customers` → `customer_id`, with layer +prefixes (`stg_`, `dim_`, `fct_`, …) stripped and plurals reduced — and deprioritizes +columns dbt declares as foreign keys, unless that would leave no candidate at all (on a +1:1 satellite table, the foreign key *is* the table's own key). + +Separately from the tiers, a column that another cube joins to is marked as a key on the +**referenced** cube — Cube can't resolve the join otherwise. This is additive: it applies +whether or not the tiers already found a key, so a cube can end up with a declared key +*plus* a joined-to column. The cube on the other side — the one that owns the foreign +key — gets no key from this, which is the usual reason a cube in a join ends up without +one. + +To key a cube on something the pull wouldn't guess — a composite key, or a column your +suffixes don't cover — declare it in dbt. Constraints require an +[enforced model contract](https://docs.getdbt.com/reference/resource-configs/contract), +which in turn requires a `data_type` on every column of the model: + +```yaml +models: + - name: dim_accounts + config: + contract: + enforced: true + constraints: + - type: primary_key + columns: [account_sk, valid_from] + columns: + - name: account_sk + data_type: varchar + - name: valid_from + data_type: timestamp + # …every other column of the model needs a data_type too +``` + +Your warehouse doesn't have to enforce primary keys for this to work — most don't. The +pull only runs `dbt parse`, and reads the declaration out of `manifest.json`. + +If your project doesn't use contracts, tier 2 is the way to declare a single-column key: +add `unique` and `not_null` tests to it. + +A declared key is never overridden or widened by the guessing tiers — only a joined-to +column can add to it. If a cube that participates in a join ends up with no key at all, +the pull still writes the file, and what happens next depends on the cube's measures: + +- **With a `count`, `sum`, `avg`, or `number` measure** — including the `count` that + **Add default measures** adds to every cube — the data model fails to compile with + `primary key for '' is required when join is defined in order to make aggregates + work properly`. +- **Without one**, there's no compile error, but the join is dropped: a query that touches + both cubes then fails with `Can't find join path to join '', ''`. + +Either way, the generated `.yml` is where to check what the pull decided — every key it +detected is a dimension with `primary_key: true`. + ### Infer column types from dbt catalog @@ -355,12 +428,15 @@ For each dbt model in your project: - **A `count` measure** is added to every cube. - **`total_` sum measures** are added for numeric columns whose names suggest an additive metric (names containing `amount`, `price`, `cost`, `total`, or `value`). -- **Primary keys** are detected from `id` / `*_id` columns and dbt `primary_key` - constraints, and the matching dimensions are marked `primary_key`. +- **Primary keys** are detected from dbt `primary_key` constraints, then `unique` + + `not_null` tests, then column-name suffixes — see + [Primary key detection](#primary-key-detection). - **Joins between cubes** are generated from dbt `relationships` tests and - foreign-key constraints, with the relationship type (`many_to_one`, - `one_to_many`, or `one_to_one`) inferred from the models — so the generated data - model is queryable across cubes out of the box. + foreign-key constraints — so the generated data model is queryable across cubes out of + the box. The foreign-key column is on the "many" side, so every generated join is + `many_to_one` from the cube that owns it. A dbt relationship describes the reference + from the referencing side only, so nothing points back — enable **Also generate reverse + joins** to emit a `one_to_many` join back to the referencing cube too. Models named `metricflow_time_spine` and any non-model resources (sources, seeds, snapshots, etc.) are skipped. @@ -538,7 +614,9 @@ Each push creates exactly two new files: - **Supported warehouses:** Snowflake, Amazon Redshift, PostgreSQL, Google BigQuery, Databricks, and Amazon Athena. - **Imports models, columns, and relationships only** — not dbt metrics, semantic - models, tests, or exposures. + models, or exposures. Data tests aren't converted into anything either; `unique`, + `not_null`, and `relationships` tests are only read as evidence for + [primary keys](#primary-key-detection) and joins. - **Pull is one-directional** — dbt pull never writes back to your dbt repository. Promoting cubes into dbt is the [dbt push](#push-cubes-to-dbt) direction (in preview). - **No warehouse connection** — a pull doesn't trigger a `dbt run`; it assumes your @@ -635,6 +713,31 @@ into the **dbt models schema** you configured, and that the schema matches. + + +- **No key at all** — the model has no `primary_key` constraint, no strict `unique` + + `not_null` pair, and no column matching the configured suffixes. Either declare the key + in dbt, or set **Primary key column suffixes** to your project's convention (e.g. `_sk`). +- **A declared key was ignored** — a `primary_key` constraint is applied all-or-nothing. + If it names a column the model's `columns:` block doesn't document, the whole + declaration is skipped (rather than emitting a narrower, wrong grain) and the pull falls + back to the guessing tiers. Check that every column the constraint lists is also + documented under `columns:`. +- **Wrong column** — declare the key in dbt and the pull will use it verbatim. + +Open the generated `.yml` to see what the pull decided: the key is whichever dimensions +carry `primary_key: true`. See [Primary key detection](#primary-key-detection). + + + + + +Joins generated from dbt follow the direction dbt declares them, and Cube's join graph is +directed — so a query rooted at the referenced cube can't reach the cube that references +it. Enable **Also generate reverse joins** in the pull settings and re-run the pull. + + + The `sql_table` is derived from your dbt project and the **dbt models schema** From d8d009b80a78b51d000c8d7aaf12011ab8bfefba Mon Sep 17 00:00:00 2001 From: waralexrom <108349432+waralexrom@users.noreply.github.com> Date: Thu, 30 Jul 2026 19:32:59 +0200 Subject: [PATCH 2/2] refactor(tesseract): unify MemberSymbol dependency traversal behind a single declaration (#11409) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(tesseract): unify MemberSymbol dependency traversal behind a single declaration Each symbol node now declares its dependency slots exactly once — via the symbol_deps! macro for structs (full destructuring, so a new field fails to compile until classified) or a hand-written SymbolDeps impl for enums — and both the read side (collect_deps, collect_cube_refs) and the rebuild side (apply_recursive, apply_to_deps) are derived from that declaration by generic algorithms in planner/symbols/deps.rs. This replaces the per-node hand-written get_dependencies / extract_symbol_deps / get_cube_refs / extract_cube_refs / apply_to_deps bodies, which enumerated the same slots independently. Semantic points: - A dependency is a symbol or cube ref whose rendered SQL enters the node's SQL. Multi-stage grain/filter member lists are annotations matched by full_name, not dependencies: apply_to_deps no longer rewrites them. - TimeDimensionSymbol::base_symbol is a transparent slot: reads emit the base's dependencies, transforms receive the slot itself. - Transform semantics are uniform: f is applied to every symbol node exactly once, recursion is handled by the algorithm. - Slot order is a contract (declaration order); reference symbols resolve to their first dependency. - iter_sql_calls/validate, owned_by_cube and DebugSql stay bespoke. * test(tesseract): pin the TimeDimension dependency contract The base symbol is a granularity view, not a dependency: reads look through it (emitting the base's own dependencies), while transforms receive it as a replaceable slot and the wrapper keeps its identity. * test(tesseract): cover switch and case-switch dimensions in multi-stage include grain Include grain refs are selected inside the multi-stage CTEs; case pruning and calc-group value resolution follow each leaf's own filters (the leaf rebuilds its QueryProperties, re-applying static filters), so a leaf whose filter directive excludes the switch filter keeps the full CASE while a filtered leaf renders the pruned branch. --- .../cubesqlplanner/src/planner/sql_call.rs | 45 ++-- .../src/planner/symbols/common/case.rs | 206 ++++++---------- .../src/planner/symbols/common/multi_stage.rs | 39 ---- .../src/planner/symbols/deps.rs | 221 ++++++++++++++++++ .../symbols/dimension_kinds/case_dimension.rs | 47 +--- .../planner/symbols/dimension_kinds/geo.rs | 36 +-- .../planner/symbols/dimension_kinds/mod.rs | 45 ++-- .../symbols/dimension_kinds/regular.rs | 32 +-- .../planner/symbols/dimension_kinds/switch.rs | 43 +--- .../src/planner/symbols/dimension_symbol.rs | 58 ++--- .../symbols/measure_kinds/aggregated.rs | 42 +--- .../symbols/measure_kinds/calculated.rs | 42 +--- .../planner/symbols/measure_kinds/count.rs | 70 ++---- .../src/planner/symbols/measure_kinds/mod.rs | 56 ++--- .../src/planner/symbols/measure_symbol.rs | 115 +++------ .../symbols/member_expression_symbol.rs | 73 +++--- .../src/planner/symbols/member_symbol.rs | 69 +++--- .../cubesqlplanner/src/planner/symbols/mod.rs | 1 + .../planner/symbols/time_dimension_symbol.rs | 60 ++--- .../src/planner/time_dimension/granularity.rs | 26 ++- .../common/integration_multi_stage.yaml | 72 ++++++ .../integration/multi_stage/include_switch.rs | 118 ++++++++++ .../src/tests/integration/multi_stage/mod.rs | 1 + ...nclude_switch__include_calc_group_dim.snap | 8 + ..._switch__include_case_over_calc_group.snap | 10 + ...clude_switch__include_case_switch_dim.snap | 8 + ...ch_dim_with_filter_excluded_from_leaf.snap | 8 + .../cubesqlplanner/src/tests/mod.rs | 1 + .../src/tests/time_dimension_symbol.rs | 164 +++++++++++++ 29 files changed, 984 insertions(+), 732 deletions(-) create mode 100644 rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/deps.rs create mode 100644 rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/include_switch.rs create mode 100644 rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__include_switch__include_calc_group_dim.snap create mode 100644 rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__include_switch__include_case_over_calc_group.snap create mode 100644 rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__include_switch__include_case_switch_dim.snap create mode 100644 rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__include_switch__include_case_switch_dim_with_filter_excluded_from_leaf.snap create mode 100644 rust/cube/cubesqlplanner/cubesqlplanner/src/tests/time_dimension_symbol.rs diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call.rs index de4a6f0eba293..76115c9cc4437 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/sql_call.rs @@ -1,3 +1,4 @@ +use super::symbols::deps::{DepVisitor, DepVisitorMut, SymbolDeps}; use super::symbols::MemberSymbol; use crate::cube_bridge::member_sql::{FilterParamsColumn, SecutityContextProps, SqlTemplate}; use crate::physical_plan::sql_nodes::{SqlNode, SqlNodesFactory}; @@ -281,9 +282,10 @@ impl SqlCall { } pub fn get_cube_refs(&self) -> Vec { - let mut result = vec![]; - self.extract_cube_refs(&mut result); - result + self.deps + .iter() + .filter_map(|d| d.as_cube_ref().cloned()) + .collect() } fn prepare_template_params( @@ -528,37 +530,28 @@ impl SqlCall { _ => false, }) } +} - pub fn extract_symbol_deps(&self, result: &mut Vec>) { - for dep in self.deps.iter() { - if let Some(s) = dep.as_symbol() { - result.push(s.clone()) - } - } - } - - pub fn extract_cube_refs(&self, result: &mut Vec) { +impl SymbolDeps for Rc { + fn visit_deps(&self, visitor: &mut dyn DepVisitor) -> std::ops::ControlFlow<()> { for dep in self.deps.iter() { - if let SqlDependency::CubeRef(cr) = dep { - result.push(cr.clone()); + match dep { + SqlDependency::Symbol(s) => visitor.symbol(s)?, + SqlDependency::CubeRef(cr) => visitor.cube_ref(cr)?, } } + std::ops::ControlFlow::Continue(()) } - /// Returns a new `SqlCall` with `f` applied recursively to every - /// member-symbol dependency. Cube refs and other placeholders - /// pass through unchanged. - pub fn apply_recursive) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result, CubeError> { - let mut result = self.clone(); - for dep in result.deps.iter_mut() { - if let SqlDependency::Symbol(ref s) = dep { - *dep = SqlDependency::Symbol(s.apply_recursive(f)?); + fn visit_deps_mut(&mut self, visitor: &mut dyn DepVisitorMut) -> Result<(), CubeError> { + let mut call = (**self).clone(); + for dep in call.deps.iter_mut() { + if let SqlDependency::Symbol(s) = dep { + visitor.symbol(s)?; } } - Ok(Rc::new(result)) + *self = Rc::new(call); + Ok(()) } } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/common/case.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/common/case.rs index a4730b839fb28..8bf4d4e97eb81 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/common/case.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/common/case.rs @@ -1,13 +1,15 @@ +use super::super::deps::{symbol_deps, DepVisitor, DepVisitorMut, SymbolDeps}; use crate::planner::filter::FilterItem; use crate::{ cube_bridge::{ case_switch_definition::CaseSwitchDefinition as NativeCaseSwitchDefinition, case_variant::CaseVariant, string_or_sql::StringOrSql, }, - planner::{find_value_restriction, Compiler, CubeRef, MemberSymbol, SqlCall}, + planner::{find_value_restriction, Compiler, MemberSymbol, SqlCall}, }; use cubenativeutils::CubeError; use itertools::Itertools; +use std::ops::ControlFlow; use std::rc::Rc; #[derive(Clone)] @@ -16,68 +18,49 @@ pub enum CaseLabel { Sql(Rc), } +impl SymbolDeps for CaseLabel { + fn visit_deps(&self, visitor: &mut dyn DepVisitor) -> ControlFlow<()> { + match self { + Self::String(_) => ControlFlow::Continue(()), + Self::Sql(sql) => sql.visit_deps(visitor), + } + } + + fn visit_deps_mut(&mut self, visitor: &mut dyn DepVisitorMut) -> Result<(), CubeError> { + match self { + Self::String(_) => Ok(()), + Self::Sql(sql) => sql.visit_deps_mut(visitor), + } + } +} + #[derive(Clone)] pub struct CaseWhenItem { pub sql: Rc, pub label: CaseLabel, } +symbol_deps! { + CaseWhenItem { + sql: dep, + label: dep, + } +} + #[derive(Clone)] pub struct CaseDefinition { pub items: Vec, pub else_label: CaseLabel, } -impl CaseDefinition { - fn extract_cube_refs(&self, result: &mut Vec) { - for itm in self.items.iter() { - itm.sql.extract_cube_refs(result); - if let CaseLabel::Sql(sql) = &itm.label { - sql.extract_cube_refs(result); - } - } - if let CaseLabel::Sql(sql) = &self.else_label { - sql.extract_cube_refs(result); - } - } - - fn extract_symbol_deps(&self, result: &mut Vec>) { - for itm in self.items.iter() { - itm.sql.extract_symbol_deps(result); - if let CaseLabel::Sql(sql) = &itm.label { - sql.extract_symbol_deps(result); - } - } - if let CaseLabel::Sql(sql) = &self.else_label { - sql.extract_symbol_deps(result); - } - } - fn apply_to_deps) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result { - let items = self - .items - .iter() - .map(|itm| -> Result<_, CubeError> { - let label = match &itm.label { - CaseLabel::String(_) => itm.label.clone(), - CaseLabel::Sql(sql_call) => CaseLabel::Sql(sql_call.apply_recursive(f)?), - }; - Ok(CaseWhenItem { - sql: itm.sql.apply_recursive(f)?, - label, - }) - }) - .collect::, _>>()?; - let else_label = match &self.else_label { - CaseLabel::String(_) => self.else_label.clone(), - CaseLabel::Sql(sql_call) => CaseLabel::Sql(sql_call.apply_recursive(f)?), - }; - let res = CaseDefinition { items, else_label }; - Ok(res) +symbol_deps! { + CaseDefinition { + items: dep, + else_label: dep, } +} +impl CaseDefinition { fn iter_sql_calls(&self) -> Box> + '_> { Box::new(self.items.iter().map(|item| &item.sql)) } @@ -97,38 +80,36 @@ pub struct CaseSwitchWhenItem { pub sql: Rc, } +symbol_deps! { + CaseSwitchWhenItem { + value: skip, + sql: dep, + } +} + #[derive(Clone)] pub enum CaseSwitchItem { Sql(Rc), Member(Rc), } -impl CaseSwitchItem { - fn extract_cube_refs(&self, result: &mut Vec) { +impl SymbolDeps for CaseSwitchItem { + fn visit_deps(&self, visitor: &mut dyn DepVisitor) -> ControlFlow<()> { match self { - CaseSwitchItem::Sql(sql_call) => sql_call.extract_cube_refs(result), - CaseSwitchItem::Member(_) => {} + Self::Sql(sql_call) => sql_call.visit_deps(visitor), + Self::Member(member) => visitor.symbol(member), } } - fn extract_symbol_deps(&self, result: &mut Vec>) { + fn visit_deps_mut(&mut self, visitor: &mut dyn DepVisitorMut) -> Result<(), CubeError> { match self { - CaseSwitchItem::Sql(sql_call) => sql_call.extract_symbol_deps(result), - CaseSwitchItem::Member(member_symbol) => result.push(member_symbol.clone()), + Self::Sql(sql_call) => sql_call.visit_deps_mut(visitor), + Self::Member(member) => visitor.symbol(member), } } +} - fn apply_to_deps) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result { - let res = match self { - CaseSwitchItem::Sql(sql_call) => CaseSwitchItem::Sql(sql_call.apply_recursive(f)?), - CaseSwitchItem::Member(member) => CaseSwitchItem::Member(member.apply_recursive(f)?), - }; - Ok(res) - } - +impl CaseSwitchItem { fn iter_sql_calls(&self) -> Box> + '_> { match self { CaseSwitchItem::Sql(sql_call) => Box::new(std::iter::once(sql_call)), @@ -144,17 +125,15 @@ pub struct CaseSwitchDefinition { pub else_sql: Option>, } -impl CaseSwitchDefinition { - fn extract_cube_refs(&self, result: &mut Vec) { - self.switch.extract_cube_refs(result); - for itm in self.items.iter() { - itm.sql.extract_cube_refs(result); - } - if let Some(else_sql) = &self.else_sql { - else_sql.extract_cube_refs(result); - } +symbol_deps! { + CaseSwitchDefinition { + switch: dep, + items: dep, + else_sql: dep, } +} +impl CaseSwitchDefinition { pub fn try_new( cube_name: &String, definition: Rc, @@ -219,15 +198,6 @@ impl CaseSwitchDefinition { owned } - fn extract_symbol_deps(&self, result: &mut Vec>) { - self.switch.extract_symbol_deps(result); - for itm in self.items.iter() { - itm.sql.extract_symbol_deps(result); - } - if let Some(else_sql) = &self.else_sql { - else_sql.extract_symbol_deps(result); - } - } fn get_switch_values(&self) -> Option> { if let CaseSwitchItem::Member(member) = &self.switch { if let Ok(switch_dim) = member.as_dimension() { @@ -293,33 +263,6 @@ impl CaseSwitchDefinition { } None } - pub fn apply_to_deps) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result { - let switch = self.switch.apply_to_deps(f)?; - let items = self - .items - .iter() - .map(|itm| -> Result<_, CubeError> { - Ok(CaseSwitchWhenItem { - sql: itm.sql.apply_recursive(f)?, - value: itm.value.clone(), - }) - }) - .collect::, _>>()?; - let else_sql = if let Some(else_sql) = &self.else_sql { - Some(else_sql.apply_recursive(f)?) - } else { - None - }; - let res = CaseSwitchDefinition { - switch, - items, - else_sql, - }; - Ok(res) - } } /// Body of a case-defined member, mapped from the `case` field of @@ -377,19 +320,6 @@ impl Case { Ok(res) } - pub fn extract_cube_refs(&self, result: &mut Vec) { - match self { - Case::Case(def) => def.extract_cube_refs(result), - Case::CaseSwitch(def) => def.extract_cube_refs(result), - } - } - - pub fn extract_symbol_deps(&self, result: &mut Vec>) { - match self { - Case::Case(def) => def.extract_symbol_deps(result), - Case::CaseSwitch(def) => def.extract_symbol_deps(result), - } - } pub fn case_switch_dimension(&self) -> Option> { if let Case::CaseSwitch(case) = &self { if let CaseSwitchItem::Member(member) = &case.switch { @@ -407,16 +337,6 @@ impl Case { .map(|r| Case::CaseSwitch(r)), } } - pub fn apply_to_deps) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result { - let res = match self { - Case::Case(case) => Case::Case(case.apply_to_deps(f)?), - Case::CaseSwitch(case) => Case::CaseSwitch(case.apply_to_deps(f)?), - }; - Ok(res) - } pub fn is_single_value(&self) -> bool { match self { Case::Case(_) => false, @@ -438,6 +358,22 @@ impl Case { } } +impl SymbolDeps for Case { + fn visit_deps(&self, visitor: &mut dyn DepVisitor) -> ControlFlow<()> { + match self { + Case::Case(def) => def.visit_deps(visitor), + Case::CaseSwitch(def) => def.visit_deps(visitor), + } + } + + fn visit_deps_mut(&mut self, visitor: &mut dyn DepVisitorMut) -> Result<(), CubeError> { + match self { + Case::Case(def) => def.visit_deps_mut(visitor), + Case::CaseSwitch(def) => def.visit_deps_mut(visitor), + } + } +} + impl crate::utils::debug::DebugSql for Case { fn debug_sql(&self, expand_deps: bool) -> String { match self { diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/common/multi_stage.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/common/multi_stage.rs index 4f64597c10ee0..aa18cea030724 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/common/multi_stage.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/common/multi_stage.rs @@ -117,45 +117,6 @@ impl MultiStageProperties { time_shift: None, })) } - - pub fn apply_to_deps) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result { - let map_refs = |refs: &Option>>| -> Result<_, CubeError> { - match refs { - Some(items) => Ok(Some(items.iter().map(f).collect::, _>>()?)), - None => Ok(None), - } - }; - - let filter = match &self.filter { - Some(f_old) => Some(MultiStageFilter { - mode: f_old.mode.clone(), - exclude: map_refs(&f_old.exclude)?, - keep_only: map_refs(&f_old.keep_only)?, - // include_* items are FilterItems that already hold their own - // resolved member references; transformations of dependency - // chains apply at the symbol level, so we keep them as-is. - include_dimension: f_old.include_dimension.clone(), - include_time_dimension: f_old.include_time_dimension.clone(), - include_measure: f_old.include_measure.clone(), - }), - None => None, - }; - - let grain = MultiStageGrain { - exclude: map_refs(&self.grain.exclude)?, - keep_only: map_refs(&self.grain.keep_only)?, - include: map_refs(&self.grain.include)?, - }; - - Ok(Self { - grain, - filter, - time_shift: self.time_shift.clone(), - }) - } } fn resolve_reference_paths( diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/deps.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/deps.rs new file mode 100644 index 0000000000000..747da036d8111 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/deps.rs @@ -0,0 +1,221 @@ +//! Single mechanism for enumerating and transforming symbol dependencies. +//! +//! A *dependency* of a symbol is another member symbol (or a cube +//! reference) whose rendered SQL becomes part of that symbol's SQL. +//! Symbol references that only name members without rendering them — +//! multi-stage grain / filter member lists, time-shift targets — are +//! annotations, not dependencies: they are matched by `full_name` and +//! are invisible to the traversals defined here. +//! +//! Every node of a symbol's body declares its dependency slots exactly +//! once — with the [`symbol_deps!`] macro for structs, or a hand-written +//! [`SymbolDeps`] impl for enums (an exhaustive `match` gives the same +//! new-variant safety the macro's full destructuring gives for new +//! fields). Both the read-side walk ([`collect_deps`], +//! [`collect_cube_refs`]) and the rebuild-side transform +//! ([`apply_recursive`], [`apply_to_deps`]) are derived from that one +//! declaration, so they can never disagree on what the dependencies are. +//! +//! Slots are visited in declaration order; the order is a contract — +//! a reference symbol resolves to its *first* dependency. + +use super::MemberSymbol; +use crate::planner::CubeRef; +use cubenativeutils::CubeError; +use std::ops::ControlFlow; +use std::rc::Rc; + +/// Read-side sink for dependency slots. +pub trait DepVisitor { + fn symbol(&mut self, symbol: &Rc) -> ControlFlow<()>; + + fn cube_ref(&mut self, _cube_ref: &CubeRef) -> ControlFlow<()> { + ControlFlow::Continue(()) + } +} + +/// Rebuild-side sink for dependency slots: receives every direct +/// member-symbol slot and may replace it. +pub trait DepVisitorMut { + fn symbol(&mut self, slot: &mut Rc) -> Result<(), CubeError>; +} + +/// A node whose dependency slots can be walked (read) or rebuilt +/// (transform) from the same declaration. +pub trait SymbolDeps { + fn visit_deps(&self, visitor: &mut dyn DepVisitor) -> ControlFlow<()>; + + fn visit_deps_mut(&mut self, visitor: &mut dyn DepVisitorMut) -> Result<(), CubeError>; +} + +impl SymbolDeps for Option { + fn visit_deps(&self, visitor: &mut dyn DepVisitor) -> ControlFlow<()> { + if let Some(item) = self { + item.visit_deps(visitor)?; + } + ControlFlow::Continue(()) + } + + fn visit_deps_mut(&mut self, visitor: &mut dyn DepVisitorMut) -> Result<(), CubeError> { + if let Some(item) = self { + item.visit_deps_mut(visitor)?; + } + Ok(()) + } +} + +impl SymbolDeps for Vec { + fn visit_deps(&self, visitor: &mut dyn DepVisitor) -> ControlFlow<()> { + for item in self.iter() { + item.visit_deps(visitor)?; + } + ControlFlow::Continue(()) + } + + fn visit_deps_mut(&mut self, visitor: &mut dyn DepVisitorMut) -> Result<(), CubeError> { + for item in self.iter_mut() { + item.visit_deps_mut(visitor)?; + } + Ok(()) + } +} + +/// Declares the dependency slots of a struct once, generating both +/// `SymbolDeps` methods from the same field list. +/// +/// Every field must be listed with one of the modes: +/// - `dep` — a dependency slot; delegates to the field's `SymbolDeps` +/// impl (`Rc`, composite bodies, `Option` / `Vec` of those). +/// - `dep_symbol` — a direct `Rc` dependency: emitted as +/// a leaf on read, offered for replacement on rebuild. +/// - `dep_transparent` — an `Rc` the node is a view of: +/// read looks through it (emits the symbol's own dependencies, not +/// the symbol), rebuild offers the slot itself for replacement. +/// - `skip` — not a dependency (plain data or an annotation). +/// +/// The generated code destructures the struct without `..`, so adding +/// a field fails to compile until it is classified here. +macro_rules! symbol_deps { + ($ty:ident { $($field:ident: $mode:ident),+ $(,)? }) => { + impl $crate::planner::symbols::deps::SymbolDeps for $ty { + fn visit_deps( + &self, + visitor: &mut dyn $crate::planner::symbols::deps::DepVisitor, + ) -> ::std::ops::ControlFlow<()> { + let Self { $($field),+ } = self; + $($crate::planner::symbols::deps::symbol_deps!(@visit $mode, $field, visitor);)+ + ::std::ops::ControlFlow::Continue(()) + } + + fn visit_deps_mut( + &mut self, + visitor: &mut dyn $crate::planner::symbols::deps::DepVisitorMut, + ) -> Result<(), ::cubenativeutils::CubeError> { + let Self { $($field),+ } = self; + $($crate::planner::symbols::deps::symbol_deps!(@visit_mut $mode, $field, visitor);)+ + Ok(()) + } + } + }; + + (@visit skip, $field:ident, $visitor:ident) => { + let _ = $field; + }; + (@visit dep, $field:ident, $visitor:ident) => { + $crate::planner::symbols::deps::SymbolDeps::visit_deps($field, $visitor)?; + }; + (@visit dep_symbol, $field:ident, $visitor:ident) => { + $visitor.symbol($field)?; + }; + (@visit dep_transparent, $field:ident, $visitor:ident) => { + $crate::planner::symbols::deps::SymbolDeps::visit_deps($field.as_ref(), $visitor)?; + }; + + (@visit_mut skip, $field:ident, $visitor:ident) => { + let _ = $field; + }; + (@visit_mut dep, $field:ident, $visitor:ident) => { + $crate::planner::symbols::deps::SymbolDeps::visit_deps_mut($field, $visitor)?; + }; + (@visit_mut dep_symbol, $field:ident, $visitor:ident) => { + $visitor.symbol($field)?; + }; + (@visit_mut dep_transparent, $field:ident, $visitor:ident) => { + $visitor.symbol($field)?; + }; +} +pub(crate) use symbol_deps; + +/// All direct member-symbol dependencies of a node, in slot +/// declaration order. +pub fn collect_deps(node: &dyn SymbolDeps) -> Vec> { + struct Collector(Vec>); + + impl DepVisitor for Collector { + fn symbol(&mut self, symbol: &Rc) -> ControlFlow<()> { + self.0.push(symbol.clone()); + ControlFlow::Continue(()) + } + } + + let mut collector = Collector(vec![]); + let _ = node.visit_deps(&mut collector); + collector.0 +} + +/// All cube references of a node, in slot declaration order. +pub fn collect_cube_refs(node: &dyn SymbolDeps) -> Vec { + struct Collector(Vec); + + impl DepVisitor for Collector { + fn symbol(&mut self, _symbol: &Rc) -> ControlFlow<()> { + ControlFlow::Continue(()) + } + + fn cube_ref(&mut self, cube_ref: &CubeRef) -> ControlFlow<()> { + self.0.push(cube_ref.clone()); + ControlFlow::Continue(()) + } + } + + let mut collector = Collector(vec![]); + let _ = node.visit_deps(&mut collector); + collector.0 +} + +struct ApplyVisitor<'a, F> { + f: &'a F, +} + +impl DepVisitorMut for ApplyVisitor<'_, F> +where + F: Fn(&Rc) -> Result, CubeError>, +{ + fn symbol(&mut self, slot: &mut Rc) -> Result<(), CubeError> { + *slot = apply_recursive(slot, self.f)?; + Ok(()) + } +} + +/// Applies `f` to this symbol, then recurses into the dependencies of +/// the result returned by `f` — not of the original symbol. `f` is +/// applied to every symbol node exactly once. +pub fn apply_recursive(symbol: &Rc, f: &F) -> Result, CubeError> +where + F: Fn(&Rc) -> Result, CubeError>, +{ + let result = f(symbol)?; + apply_to_deps(&result, f) +} + +/// Rebuilds the symbol with every dependency replaced by +/// `apply_recursive` of itself; the symbol's own node is not passed +/// to `f`. +pub fn apply_to_deps(symbol: &Rc, f: &F) -> Result, CubeError> +where + F: Fn(&Rc) -> Result, CubeError>, +{ + let mut result = symbol.as_ref().clone(); + result.visit_deps_mut(&mut ApplyVisitor { f })?; + Ok(Rc::new(result)) +} diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/case_dimension.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/case_dimension.rs index 262d2421e64e3..852b038048cac 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/case_dimension.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/case_dimension.rs @@ -1,7 +1,6 @@ use super::super::common::{Case, DimensionType}; -use super::super::MemberSymbol; -use crate::planner::{CubeRef, SqlCall}; -use cubenativeutils::CubeError; +use super::super::deps::symbol_deps; +use crate::planner::SqlCall; use std::rc::Rc; /// Dimension whose value is defined via the `case` field of the @@ -14,6 +13,14 @@ pub struct CaseDimension { member_sql: Option>, } +symbol_deps! { + CaseDimension { + dimension_type: skip, + member_sql: dep, + case: dep, + } +} + impl CaseDimension { pub fn new(dimension_type: DimensionType, case: Case, member_sql: Option>) -> Self { Self { @@ -43,44 +50,10 @@ impl CaseDimension { } } - pub fn get_dependencies(&self) -> Vec> { - let mut deps = vec![]; - if let Some(member_sql) = &self.member_sql { - member_sql.extract_symbol_deps(&mut deps); - } - self.case.extract_symbol_deps(&mut deps); - deps - } - - pub fn apply_to_deps) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result { - let member_sql = if let Some(sql) = &self.member_sql { - Some(sql.apply_recursive(f)?) - } else { - None - }; - Ok(Self { - dimension_type: self.dimension_type, - case: self.case.apply_to_deps(f)?, - member_sql, - }) - } - pub fn iter_sql_calls(&self) -> Box> + '_> { Box::new(self.member_sql.iter().chain(self.case.iter_sql_calls())) } - pub fn get_cube_refs(&self) -> Vec { - let mut refs = vec![]; - if let Some(member_sql) = &self.member_sql { - member_sql.extract_cube_refs(&mut refs); - } - self.case.extract_cube_refs(&mut refs); - refs - } - pub fn is_owned_by_cube(&self) -> bool { let mut owned = false; if let Some(sql) = &self.member_sql { diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/geo.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/geo.rs index 07ead104bf554..3c01846929252 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/geo.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/geo.rs @@ -1,6 +1,5 @@ -use super::super::MemberSymbol; -use crate::planner::{CubeRef, SqlCall}; -use cubenativeutils::CubeError; +use super::super::deps::symbol_deps; +use crate::planner::SqlCall; use std::rc::Rc; /// `type: geo` dimension from the data model: a geographic dimension @@ -11,6 +10,13 @@ pub struct GeoDimension { longitude: Rc, } +symbol_deps! { + GeoDimension { + latitude: dep, + longitude: dep, + } +} + impl GeoDimension { pub fn new(latitude: Rc, longitude: Rc) -> Self { Self { @@ -27,34 +33,10 @@ impl GeoDimension { &self.longitude } - pub fn get_dependencies(&self) -> Vec> { - let mut deps = vec![]; - self.latitude.extract_symbol_deps(&mut deps); - self.longitude.extract_symbol_deps(&mut deps); - deps - } - - pub fn apply_to_deps) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result { - Ok(Self { - latitude: self.latitude.apply_recursive(f)?, - longitude: self.longitude.apply_recursive(f)?, - }) - } - pub fn iter_sql_calls(&self) -> Box> + '_> { Box::new(std::iter::once(&self.latitude).chain(std::iter::once(&self.longitude))) } - pub fn get_cube_refs(&self) -> Vec { - let mut refs = vec![]; - self.latitude.extract_cube_refs(&mut refs); - self.longitude.extract_cube_refs(&mut refs); - refs - } - pub fn is_owned_by_cube(&self) -> bool { self.latitude.is_owned_by_cube() || self.longitude.is_owned_by_cube() } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/mod.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/mod.rs index defaaf27aab85..9d7b8f267c48f 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/mod.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/mod.rs @@ -9,9 +9,10 @@ pub use regular::*; pub use switch::*; use super::common::DimensionType; -use super::MemberSymbol; -use crate::planner::{CubeRef, SqlCall}; +use super::deps::{DepVisitor, DepVisitorMut, SymbolDeps}; +use crate::planner::SqlCall; use cubenativeutils::CubeError; +use std::ops::ControlFlow; use std::rc::Rc; /// Form of a dimension's value, classified from its data-model @@ -32,28 +33,27 @@ pub enum DimensionKind { Case(CaseDimension), } -impl DimensionKind { - pub fn get_dependencies(&self) -> Vec> { +impl SymbolDeps for DimensionKind { + fn visit_deps(&self, visitor: &mut dyn DepVisitor) -> ControlFlow<()> { match self { - Self::Regular(r) => r.get_dependencies(), - Self::Geo(g) => g.get_dependencies(), - Self::Switch(s) => s.get_dependencies(), - Self::Case(c) => c.get_dependencies(), + Self::Regular(r) => r.visit_deps(visitor), + Self::Geo(g) => g.visit_deps(visitor), + Self::Switch(s) => s.visit_deps(visitor), + Self::Case(c) => c.visit_deps(visitor), } } - pub fn apply_to_deps) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result { - Ok(match self { - Self::Regular(r) => Self::Regular(r.apply_to_deps(f)?), - Self::Geo(g) => Self::Geo(g.apply_to_deps(f)?), - Self::Switch(s) => Self::Switch(s.apply_to_deps(f)?), - Self::Case(c) => Self::Case(c.apply_to_deps(f)?), - }) + fn visit_deps_mut(&mut self, visitor: &mut dyn DepVisitorMut) -> Result<(), CubeError> { + match self { + Self::Regular(r) => r.visit_deps_mut(visitor), + Self::Geo(g) => g.visit_deps_mut(visitor), + Self::Switch(s) => s.visit_deps_mut(visitor), + Self::Case(c) => c.visit_deps_mut(visitor), + } } +} +impl DimensionKind { pub fn iter_sql_calls(&self) -> Box> + '_> { match self { Self::Regular(r) => r.iter_sql_calls(), @@ -63,15 +63,6 @@ impl DimensionKind { } } - pub fn get_cube_refs(&self) -> Vec { - match self { - Self::Regular(r) => r.get_cube_refs(), - Self::Geo(g) => g.get_cube_refs(), - Self::Switch(s) => s.get_cube_refs(), - Self::Case(c) => c.get_cube_refs(), - } - } - pub fn is_owned_by_cube(&self) -> bool { match self { Self::Regular(r) => r.is_owned_by_cube(), diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/regular.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/regular.rs index a75e0c44fe477..76ba8690f70be 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/regular.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/regular.rs @@ -1,7 +1,6 @@ use super::super::common::DimensionType; -use super::super::MemberSymbol; -use crate::planner::{CubeRef, SqlCall}; -use cubenativeutils::CubeError; +use super::super::deps::symbol_deps; +use crate::planner::SqlCall; use std::rc::Rc; /// Plain dimension from the data model — a single `sql` expression @@ -12,6 +11,13 @@ pub struct RegularDimension { member_sql: Rc, } +symbol_deps! { + RegularDimension { + dimension_type: skip, + member_sql: dep, + } +} + impl RegularDimension { pub fn new(dimension_type: DimensionType, member_sql: Rc) -> Self { Self { @@ -28,26 +34,6 @@ impl RegularDimension { &self.member_sql } - pub fn get_dependencies(&self) -> Vec> { - let mut deps = vec![]; - self.member_sql.extract_symbol_deps(&mut deps); - deps - } - - pub fn get_cube_refs(&self) -> Vec { - self.member_sql.get_cube_refs() - } - - pub fn apply_to_deps) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result { - Ok(Self { - dimension_type: self.dimension_type, - member_sql: self.member_sql.apply_recursive(f)?, - }) - } - pub fn iter_sql_calls(&self) -> Box> + '_> { Box::new(std::iter::once(&self.member_sql)) } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/switch.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/switch.rs index be378c9cb57ec..030b86764c9ed 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/switch.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_kinds/switch.rs @@ -1,6 +1,5 @@ -use super::super::MemberSymbol; -use crate::planner::{CubeRef, SqlCall}; -use cubenativeutils::CubeError; +use super::super::deps::symbol_deps; +use crate::planner::SqlCall; use std::rc::Rc; /// `type: switch` dimension from the data model: an enum with a @@ -14,6 +13,13 @@ pub struct SwitchDimension { member_sql: Option>, } +symbol_deps! { + SwitchDimension { + values: skip, + member_sql: dep, + } +} + impl SwitchDimension { pub fn new(values: Vec, member_sql: Option>) -> Self { Self { values, member_sql } @@ -34,41 +40,10 @@ impl SwitchDimension { self.member_sql.is_none() } - pub fn get_dependencies(&self) -> Vec> { - let mut deps = vec![]; - if let Some(member_sql) = &self.member_sql { - member_sql.extract_symbol_deps(&mut deps); - } - deps - } - - pub fn apply_to_deps) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result { - let member_sql = if let Some(sql) = &self.member_sql { - Some(sql.apply_recursive(f)?) - } else { - None - }; - Ok(Self { - values: self.values.clone(), - member_sql, - }) - } - pub fn iter_sql_calls(&self) -> Box> + '_> { Box::new(self.member_sql.iter()) } - pub fn get_cube_refs(&self) -> Vec { - let mut refs = vec![]; - if let Some(member_sql) = &self.member_sql { - member_sql.extract_cube_refs(&mut refs); - } - refs - } - pub fn is_owned_by_cube(&self) -> bool { false } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_symbol.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_symbol.rs index 9db8ddcad6f79..4d17518723fc7 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_symbol.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/dimension_symbol.rs @@ -1,6 +1,7 @@ use super::common::Case; use super::common::CompiledMemberPath; use super::common::MultiStageProperties; +use super::deps::{self, symbol_deps}; use super::dimension_kinds::{ CaseDimension, DimensionKind, GeoDimension, RegularDimension, SwitchDimension, }; @@ -13,7 +14,7 @@ use crate::planner::sql_templates::PlanSqlTemplates; use crate::planner::GranularityHelper; use crate::planner::SqlInterval; use crate::planner::TimeDimensionSymbol; -use crate::planner::{Compiler, CubeRef, SqlCall}; +use crate::planner::{Compiler, SqlCall}; use cubenativeutils::CubeError; use std::rc::Rc; @@ -45,6 +46,22 @@ pub struct DimensionSymbol { mask_sql: Option>, } +symbol_deps! { + DimensionSymbol { + kind: dep, + mask_sql: dep, + compiled_path: skip, + is_reference: skip, + is_view: skip, + multi_stage: skip, + time_shift: skip, + time_shift_pk_full_name: skip, + is_self_time_shift_pk: skip, + is_sub_query: skip, + propagate_filters_to_sub_query: skip, + } +} + impl DimensionSymbol { pub fn new( compiled_path: CompiledMemberPath, @@ -220,26 +237,11 @@ impl DimensionSymbol { if !self.is_reference() { return None; } - let deps = self.get_dependencies(); - if deps.is_empty() { - return None; - } - deps.first().cloned() + self.get_dependencies().first().cloned() } - pub fn apply_to_deps) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result, CubeError> { - let mut result = self.clone(); - result.kind = self.kind.apply_to_deps(f)?; - if let Some(mask) = &self.mask_sql { - result.mask_sql = Some(mask.apply_recursive(f)?); - } - if let Some(ms) = &self.multi_stage { - result.multi_stage = Some(ms.apply_to_deps(f)?); - } - Ok(MemberSymbol::new_dimension(Rc::new(result))) + pub fn get_dependencies(&self) -> Vec> { + deps::collect_deps(self) } /// SQL calls inside the kind body. `mask_sql` is intentionally @@ -251,24 +253,6 @@ impl DimensionSymbol { self.kind.iter_sql_calls() } - /// All member dependencies of the dimension. - pub fn get_dependencies(&self) -> Vec> { - let mut deps = self.kind.get_dependencies(); - if let Some(mask) = &self.mask_sql { - mask.extract_symbol_deps(&mut deps); - } - deps - } - - /// All cube references of the dimension. - pub fn get_cube_refs(&self) -> Vec { - let mut refs = self.kind.get_cube_refs(); - if let Some(mask) = &self.mask_sql { - mask.extract_cube_refs(&mut refs); - } - refs - } - pub fn cube_name(&self) -> String { self.compiled_path.cube_name().clone() } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/aggregated.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/aggregated.rs index 732e679a93f97..1bf162111efed 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/aggregated.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/aggregated.rs @@ -1,7 +1,6 @@ -use super::super::super::MemberSymbol; use super::super::common::AggregationType; -use crate::planner::{CubeRef, SqlCall}; -use cubenativeutils::CubeError; +use super::super::deps::symbol_deps; +use crate::planner::SqlCall; use std::rc::Rc; /// `Aggregated` measure kind. `sql` may be absent when the measure @@ -12,6 +11,13 @@ pub struct AggregatedMeasure { member_sql: Option>, } +symbol_deps! { + AggregatedMeasure { + agg_type: skip, + member_sql: dep, + } +} + impl AggregatedMeasure { pub fn new(agg_type: AggregationType, member_sql: Rc) -> Self { Self { @@ -35,40 +41,10 @@ impl AggregatedMeasure { self.member_sql.as_ref() } - pub fn get_dependencies(&self) -> Vec> { - let mut deps = vec![]; - if let Some(sql) = &self.member_sql { - sql.extract_symbol_deps(&mut deps); - } - deps - } - - pub fn apply_to_deps) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result { - Ok(Self { - agg_type: self.agg_type, - member_sql: self - .member_sql - .as_ref() - .map(|sql| sql.apply_recursive(f)) - .transpose()?, - }) - } - pub fn iter_sql_calls(&self) -> Box> + '_> { Box::new(self.member_sql.iter()) } - pub fn get_cube_refs(&self) -> Vec { - let mut refs = vec![]; - if let Some(sql) = &self.member_sql { - sql.extract_cube_refs(&mut refs); - } - refs - } - pub fn is_owned_by_cube(&self) -> bool { self.member_sql .as_ref() diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/calculated.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/calculated.rs index 97992769070d0..4e8c711ad9b04 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/calculated.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/calculated.rs @@ -1,6 +1,5 @@ -use super::super::super::MemberSymbol; -use crate::planner::{CubeRef, SqlCall}; -use cubenativeutils::CubeError; +use super::super::deps::symbol_deps; +use crate::planner::SqlCall; use std::rc::Rc; /// Value type of a calculated (non-aggregating) measure as declared @@ -42,6 +41,13 @@ pub struct CalculatedMeasure { member_sql: Option>, } +symbol_deps! { + CalculatedMeasure { + calc_type: skip, + member_sql: dep, + } +} + impl CalculatedMeasure { pub fn new(calc_type: CalculatedMeasureType, member_sql: Rc) -> Self { Self { @@ -65,40 +71,10 @@ impl CalculatedMeasure { self.member_sql.as_ref() } - pub fn get_dependencies(&self) -> Vec> { - let mut deps = vec![]; - if let Some(sql) = &self.member_sql { - sql.extract_symbol_deps(&mut deps); - } - deps - } - - pub fn apply_to_deps) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result { - Ok(Self { - calc_type: self.calc_type, - member_sql: self - .member_sql - .as_ref() - .map(|sql| sql.apply_recursive(f)) - .transpose()?, - }) - } - pub fn iter_sql_calls(&self) -> Box> + '_> { Box::new(self.member_sql.iter()) } - pub fn get_cube_refs(&self) -> Vec { - let mut refs = vec![]; - if let Some(sql) = &self.member_sql { - sql.extract_cube_refs(&mut refs); - } - refs - } - pub fn is_owned_by_cube(&self) -> bool { self.member_sql .as_ref() diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/count.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/count.rs index cd527a1bdf631..98adf0af9d522 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/count.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/count.rs @@ -1,6 +1,7 @@ -use super::super::MemberSymbol; -use crate::planner::{CubeRef, SqlCall}; +use super::super::deps::{symbol_deps, DepVisitor, DepVisitorMut, SymbolDeps}; +use crate::planner::SqlCall; use cubenativeutils::CubeError; +use std::ops::ControlFlow; use std::rc::Rc; /// Source of a `Count` measure's SQL. @@ -14,6 +15,22 @@ pub enum CountSql { Explicit(Rc), } +impl SymbolDeps for CountSql { + fn visit_deps(&self, visitor: &mut dyn DepVisitor) -> ControlFlow<()> { + match self { + Self::Auto(pk_sqls) => pk_sqls.visit_deps(visitor), + Self::Explicit(sql) => sql.visit_deps(visitor), + } + } + + fn visit_deps_mut(&mut self, visitor: &mut dyn DepVisitorMut) -> Result<(), CubeError> { + match self { + Self::Auto(pk_sqls) => pk_sqls.visit_deps_mut(visitor), + Self::Explicit(sql) => sql.visit_deps_mut(visitor), + } + } +} + /// `Count` measure kind: counts rows of the underlying source. /// Without an explicit `sql` falls back to counting the cube's /// primary-key tuples. @@ -22,6 +39,12 @@ pub struct CountMeasure { sql: CountSql, } +symbol_deps! { + CountMeasure { + sql: dep, + } +} + impl CountMeasure { pub fn new(sql: CountSql) -> Self { Self { sql } @@ -38,36 +61,6 @@ impl CountMeasure { matches!(&self.sql, CountSql::Auto(pks) if !pks.is_empty()) } - pub fn get_dependencies(&self) -> Vec> { - let mut deps = vec![]; - match &self.sql { - CountSql::Explicit(sql) => sql.extract_symbol_deps(&mut deps), - CountSql::Auto(pk_sqls) => { - for pk in pk_sqls { - pk.extract_symbol_deps(&mut deps); - } - } - } - deps - } - - pub fn apply_to_deps) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result { - let sql = match &self.sql { - CountSql::Explicit(sql) => CountSql::Explicit(sql.apply_recursive(f)?), - CountSql::Auto(pk_sqls) => { - let new_pks = pk_sqls - .iter() - .map(|pk| pk.apply_recursive(f)) - .collect::, _>>()?; - CountSql::Auto(new_pks) - } - }; - Ok(Self { sql }) - } - pub fn iter_sql_calls(&self) -> Box> + '_> { match &self.sql { CountSql::Explicit(sql) => Box::new(std::iter::once(sql)), @@ -75,19 +68,6 @@ impl CountMeasure { } } - pub fn get_cube_refs(&self) -> Vec { - let mut refs = vec![]; - match &self.sql { - CountSql::Explicit(sql) => sql.extract_cube_refs(&mut refs), - CountSql::Auto(pk_sqls) => { - for pk in pk_sqls { - pk.extract_cube_refs(&mut refs); - } - } - } - refs - } - pub fn is_owned_by_cube(&self) -> bool { matches!(self.sql, CountSql::Auto(_)) } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/mod.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/mod.rs index 5ec4591d53c99..5f93315d88e78 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/mod.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_kinds/mod.rs @@ -7,9 +7,10 @@ pub use calculated::*; pub use count::*; use super::common::AggregationType; -use super::MemberSymbol; -use crate::planner::{CubeRef, SqlCall}; +use super::deps::{DepVisitor, DepVisitorMut, SymbolDeps}; +use crate::planner::SqlCall; use cubenativeutils::CubeError; +use std::ops::ControlFlow; use std::rc::Rc; /// How a measure kind wraps its inner SQL when rendered: no wrapper @@ -77,28 +78,6 @@ impl MeasureKind { } } - pub fn get_dependencies(&self) -> Vec> { - match self { - Self::Count(c) | Self::MultipliedCount(c) => c.get_dependencies(), - Self::Aggregated(a) => a.get_dependencies(), - Self::Calculated(c) => c.get_dependencies(), - Self::Rank => vec![], - } - } - - pub fn apply_to_deps) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result { - Ok(match self { - Self::Count(c) => Self::Count(c.apply_to_deps(f)?), - Self::MultipliedCount(c) => Self::MultipliedCount(c.apply_to_deps(f)?), - Self::Aggregated(a) => Self::Aggregated(a.apply_to_deps(f)?), - Self::Calculated(c) => Self::Calculated(c.apply_to_deps(f)?), - Self::Rank => Self::Rank, - }) - } - pub fn iter_sql_calls(&self) -> Box> + '_> { match self { Self::Count(c) | Self::MultipliedCount(c) => c.iter_sql_calls(), @@ -108,15 +87,6 @@ impl MeasureKind { } } - pub fn get_cube_refs(&self) -> Vec { - match self { - Self::Count(c) | Self::MultipliedCount(c) => c.get_cube_refs(), - Self::Aggregated(a) => a.get_cube_refs(), - Self::Calculated(c) => c.get_cube_refs(), - Self::Rank => vec![], - } - } - pub fn is_owned_by_cube(&self) -> bool { match self { Self::Count(c) | Self::MultipliedCount(c) => c.is_owned_by_cube(), @@ -281,3 +251,23 @@ impl MeasureKind { } } } + +impl SymbolDeps for MeasureKind { + fn visit_deps(&self, visitor: &mut dyn DepVisitor) -> ControlFlow<()> { + match self { + Self::Count(c) | Self::MultipliedCount(c) => c.visit_deps(visitor), + Self::Aggregated(a) => a.visit_deps(visitor), + Self::Calculated(c) => c.visit_deps(visitor), + Self::Rank => ControlFlow::Continue(()), + } + } + + fn visit_deps_mut(&mut self, visitor: &mut dyn DepVisitorMut) -> Result<(), CubeError> { + match self { + Self::Count(c) | Self::MultipliedCount(c) => c.visit_deps_mut(visitor), + Self::Aggregated(a) => a.visit_deps_mut(visitor), + Self::Calculated(c) => c.visit_deps_mut(visitor), + Self::Rank => Ok(()), + } + } +} diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_symbol.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_symbol.rs index 3589e3b2a60b7..c283ce5fb47f4 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_symbol.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/measure_symbol.rs @@ -1,4 +1,5 @@ use super::common::{Case, CompiledMemberPath, MultiStageProperties}; +use super::deps::{self, symbol_deps}; use super::measure_kinds::{CalculatedMeasure, CalculatedMeasureType, MeasureKind}; use super::SymbolPath; use super::{MemberSymbol, SymbolFactory}; @@ -8,7 +9,7 @@ use crate::cube_bridge::member_sql::MemberSql; use crate::planner::collectors::find_owned_by_cube_child; use crate::planner::sql_templates::PlanSqlTemplates; use crate::planner::SqlInterval; -use crate::planner::{Compiler, CubeRef, SqlCall}; +use crate::planner::{Compiler, SqlCall}; use cubenativeutils::CubeError; use itertools::Itertools; use std::cmp::{Eq, PartialEq}; @@ -34,15 +35,18 @@ impl MeasureOrderBy { &self.sql_call } - pub fn set_sql_call(&mut self, sql_call: Rc) { - self.sql_call = sql_call; - } - pub fn direction(&self) -> &String { &self.direction } } +symbol_deps! { + MeasureOrderBy { + sql_call: dep, + direction: skip, + } +} + /// Time-shift entry attached to a specific time dimension. Shifts /// that dimension's date range by either a fixed interval or a named /// slot. @@ -96,6 +100,23 @@ pub struct MeasureSymbol { mask_sql: Option>, } +symbol_deps! { + MeasureSymbol { + kind: dep, + measure_filters: dep, + measure_drill_filters: dep, + measure_order_by: dep, + case: dep, + mask_sql: dep, + compiled_path: skip, + rolling_window: skip, + multi_stage: skip, + is_reference: skip, + is_view: skip, + is_splitted_source: skip, + } +} + impl MeasureSymbol { pub fn new( compiled_path: CompiledMemberPath, @@ -279,40 +300,6 @@ impl MeasureSymbol { } } - pub fn apply_to_deps) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result, CubeError> { - let mut result = self.clone(); - result.kind = result.kind.apply_to_deps(f)?; - - for sql in result.measure_filters.iter_mut() { - *sql = sql.apply_recursive(f)? - } - - for sql in result.measure_drill_filters.iter_mut() { - *sql = sql.apply_recursive(f)? - } - - for order in result.measure_order_by.iter_mut() { - order.set_sql_call(order.sql_call().apply_recursive(f)?); - } - - if let Some(case) = &self.case { - result.case = Some(case.apply_to_deps(f)?) - } - - if let Some(mask) = &self.mask_sql { - result.mask_sql = Some(mask.apply_recursive(f)?); - } - - if let Some(ms) = &self.multi_stage { - result.multi_stage = Some(ms.apply_to_deps(f)?); - } - - Ok(MemberSymbol::new_measure(Rc::new(result))) - } - /// SQL calls inside the measure's kind and `case` body. /// `mask_sql` is intentionally excluded: it is compiled against /// the cube that owns the measure, which differs from the symbol's @@ -328,46 +315,6 @@ impl MeasureSymbol { Box::new(result) } - pub fn get_dependencies(&self) -> Vec> { - let mut deps = self.kind.get_dependencies(); - for filter in self.measure_filters.iter() { - filter.extract_symbol_deps(&mut deps); - } - for filter in self.measure_drill_filters.iter() { - filter.extract_symbol_deps(&mut deps); - } - for order in self.measure_order_by.iter() { - order.sql_call().extract_symbol_deps(&mut deps); - } - if let Some(case) = &self.case { - case.extract_symbol_deps(&mut deps); - } - if let Some(mask) = &self.mask_sql { - mask.extract_symbol_deps(&mut deps); - } - deps - } - - pub fn get_cube_refs(&self) -> Vec { - let mut refs = self.kind.get_cube_refs(); - for filter in self.measure_filters.iter() { - filter.extract_cube_refs(&mut refs); - } - for filter in self.measure_drill_filters.iter() { - filter.extract_cube_refs(&mut refs); - } - for order in self.measure_order_by.iter() { - order.sql_call().extract_cube_refs(&mut refs); - } - if let Some(case) = &self.case { - case.extract_cube_refs(&mut refs); - } - if let Some(mask) = &self.mask_sql { - mask.extract_cube_refs(&mut refs); - } - refs - } - /// Render form of this measure when it sits under a row-multiplying /// join: a `count` switches to a distinct `MultipliedCount`, every /// other kind is returned unchanged. @@ -427,11 +374,11 @@ impl MeasureSymbol { if !self.is_reference() { return None; } - let deps = self.get_dependencies(); - if deps.is_empty() { - return None; - } - deps.first().cloned() + self.get_dependencies().first().cloned() + } + + pub fn get_dependencies(&self) -> Vec> { + deps::collect_deps(self) } pub fn measure_type(&self) -> &str { diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/member_expression_symbol.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/member_expression_symbol.rs index 1f3cdb7ca78a5..4eacf87aae1e5 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/member_expression_symbol.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/member_expression_symbol.rs @@ -1,8 +1,9 @@ use super::common::CompiledMemberPath; +use super::deps::{self, symbol_deps, DepVisitor, DepVisitorMut, SymbolDeps}; use super::MemberSymbol; use crate::planner::collectors::member_childs; use crate::planner::sql_templates::PlanSqlTemplates; -use crate::planner::{CubeRef, CubeTableSymbol, SqlCall}; +use crate::planner::{CubeTableSymbol, SqlCall}; use crate::utils::debug::DebugSql; use cubenativeutils::CubeError; use itertools::Itertools; @@ -20,6 +21,22 @@ pub enum MemberExpressionExpression { PatchedSymbol(Rc), } +impl SymbolDeps for MemberExpressionExpression { + fn visit_deps(&self, visitor: &mut dyn DepVisitor) -> std::ops::ControlFlow<()> { + match self { + Self::SqlCall(sql_call) => sql_call.visit_deps(visitor), + Self::PatchedSymbol(symbol) => visitor.symbol(symbol), + } + } + + fn visit_deps_mut(&mut self, visitor: &mut dyn DepVisitorMut) -> Result<(), CubeError> { + match self { + Self::SqlCall(sql_call) => sql_call.visit_deps_mut(visitor), + Self::PatchedSymbol(symbol) => visitor.symbol(symbol), + } + } +} + /// `MemberSymbol::MemberExpression` body: a synthetic member built /// at query time from a SQL expression or from another member with /// query-time modifications. Not declared in the data model. Its @@ -38,6 +55,17 @@ pub struct MemberExpressionSymbol { is_segment: bool, } +symbol_deps! { + MemberExpressionSymbol { + expression: dep, + compiled_path: skip, + definition: skip, + is_reference: skip, + parenthesized: skip, + is_segment: skip, + } +} + impl MemberExpressionSymbol { pub fn try_new( cube: Rc, @@ -125,50 +153,11 @@ impl MemberExpressionSymbol { if !self.is_reference() { return None; } - let deps = self.get_dependencies(); - if deps.is_empty() { - return None; - } - deps.first().cloned() - } - - pub fn apply_to_deps) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result, CubeError> { - let mut result = self.clone(); - match &mut result.expression { - MemberExpressionExpression::SqlCall(sql_call) => { - *sql_call = sql_call.apply_recursive(f)? - } - MemberExpressionExpression::PatchedSymbol(member_symbol) => { - *member_symbol = f(member_symbol)? - } - } - - Ok(MemberSymbol::new_member_expression(Rc::new(result))) + self.get_dependencies().first().cloned() } pub fn get_dependencies(&self) -> Vec> { - let mut deps = vec![]; - match &self.expression { - MemberExpressionExpression::SqlCall(sql_call) => { - sql_call.extract_symbol_deps(&mut deps) - } - MemberExpressionExpression::PatchedSymbol(member_symbol) => { - deps.push(member_symbol.clone()) - } - } - deps - } - - pub fn get_cube_refs(&self) -> Vec { - let mut refs = vec![]; - match &self.expression { - MemberExpressionExpression::SqlCall(sql_call) => sql_call.extract_cube_refs(&mut refs), - MemberExpressionExpression::PatchedSymbol(_) => {} - } - refs + deps::collect_deps(self) } /// If every leaf member referenced by the expression is a diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/member_symbol.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/member_symbol.rs index fe578228c11f3..c16b296347198 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/member_symbol.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/member_symbol.rs @@ -4,8 +4,10 @@ use itertools::Itertools; use crate::planner::{Case, CubeRef, SqlCall}; use super::common::CompiledMemberPath; +use super::deps::{self, DepVisitor, DepVisitorMut, SymbolDeps}; use super::{DimensionSymbol, MeasureSymbol, MemberExpressionSymbol, TimeDimensionSymbol}; use std::fmt::Debug; +use std::ops::ControlFlow; use std::rc::Rc; /// First-class business object of the planner: the atomic unit of @@ -23,6 +25,7 @@ use std::rc::Rc; /// Indivisible: renders as a single SQL expression. A symbol may depend /// on other symbols (`get_dependencies`); whether those deps are /// inlined or pushed into a CTE / subquery is a physical-plan decision. +#[derive(Clone)] pub enum MemberSymbol { Dimension(Rc), TimeDimension(Rc), @@ -154,38 +157,15 @@ impl MemberSymbol { self: &Rc, f: &F, ) -> Result, CubeError> { - let result = f(self)?; - result.apply_to_deps(f) - } - - pub fn apply_to_deps) -> Result, CubeError>>( - self: &Rc, - f: &F, - ) -> Result, CubeError> { - match self.as_ref() { - Self::Dimension(d) => d.apply_to_deps(f), - Self::TimeDimension(d) => d.apply_to_deps(f), - Self::Measure(m) => m.apply_to_deps(f), - Self::MemberExpression(e) => e.apply_to_deps(f), - } + deps::apply_recursive(self, f) } pub fn get_dependencies(&self) -> Vec> { - match self { - Self::Dimension(d) => d.get_dependencies(), - Self::TimeDimension(d) => d.get_dependencies(), - Self::Measure(m) => m.get_dependencies(), - Self::MemberExpression(e) => e.get_dependencies(), - } + deps::collect_deps(self) } pub fn get_cube_refs(&self) -> Vec { - match self { - Self::Dimension(d) => d.get_cube_refs(), - Self::TimeDimension(d) => d.get_cube_refs(), - Self::Measure(m) => m.get_cube_refs(), - Self::MemberExpression(e) => e.get_cube_refs(), - } + deps::collect_cube_refs(self) } /// True if the symbol is a transparent alias for another member, with @@ -404,6 +384,43 @@ impl MemberSymbol { } } +impl SymbolDeps for MemberSymbol { + fn visit_deps(&self, visitor: &mut dyn DepVisitor) -> ControlFlow<()> { + match self { + Self::Dimension(d) => d.as_ref().visit_deps(visitor), + Self::TimeDimension(d) => d.as_ref().visit_deps(visitor), + Self::Measure(m) => m.as_ref().visit_deps(visitor), + Self::MemberExpression(e) => e.as_ref().visit_deps(visitor), + } + } + + fn visit_deps_mut(&mut self, visitor: &mut dyn DepVisitorMut) -> Result<(), CubeError> { + match self { + Self::Dimension(d) => { + let mut body = (**d).clone(); + body.visit_deps_mut(visitor)?; + *d = Rc::new(body); + } + Self::TimeDimension(d) => { + let mut body = (**d).clone(); + body.visit_deps_mut(visitor)?; + *d = Rc::new(body); + } + Self::Measure(m) => { + let mut body = (**m).clone(); + body.visit_deps_mut(visitor)?; + *m = Rc::new(body); + } + Self::MemberExpression(e) => { + let mut body = (**e).clone(); + body.visit_deps_mut(visitor)?; + *e = Rc::new(body); + } + } + Ok(()) + } +} + impl crate::utils::debug::DebugSql for MemberSymbol { fn debug_sql(&self, expand_deps: bool) -> String { match self { diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/mod.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/mod.rs index c7ec09111a11b..dc5b14cba2721 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/mod.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/mod.rs @@ -1,5 +1,6 @@ mod common; mod cube_symbol; +pub mod deps; pub mod dimension_kinds; mod dimension_symbol; pub mod measure_kinds; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/time_dimension_symbol.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/time_dimension_symbol.rs index 79bb8b3e07988..f1d0fe0a4bc8b 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/time_dimension_symbol.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/symbols/time_dimension_symbol.rs @@ -1,9 +1,9 @@ use super::common::CompiledMemberPath; +use super::deps::{self, symbol_deps}; use super::MemberSymbol; use crate::planner::query_tools::QueryTools; use crate::planner::state::State; use crate::planner::time_dimension::Granularity; -use crate::planner::CubeRef; use crate::planner::{GranularityHelper, QueryDateTime, QueryDateTimeHelper}; use chrono::Duration; use chrono_tz::Tz; @@ -25,6 +25,18 @@ pub struct TimeDimensionSymbol { alias_override: Option, } +symbol_deps! { + TimeDimensionSymbol { + granularity_obj: dep, + base_symbol: dep_transparent, + compiled_path: skip, + granularity: skip, + date_range: skip, + alias_suffix: skip, + alias_override: skip, + } +} + impl TimeDimensionSymbol { pub fn new( base_symbol: Rc, @@ -165,13 +177,18 @@ impl TimeDimensionSymbol { self.base_symbol.owned_by_cube() } + pub fn get_dependencies(&self) -> Vec> { + deps::collect_deps(self) + } + pub fn date_range_vec(&self) -> Option> { self.date_range.clone().map(|(from, to)| vec![from, to]) } - /// Like `get_dependencies`, but wraps any time-dimension dep in - /// a `TimeDimensionSymbol` carrying this symbol's granularity and - /// date range. Non-time-dimension deps pass through unchanged. + /// Dependencies of this symbol, with any time-dimension dep + /// wrapped in a `TimeDimensionSymbol` carrying this symbol's + /// granularity and date range. Non-time-dimension deps pass + /// through unchanged. pub fn get_dependencies_as_time_dimensions(&self) -> Vec> { self.get_dependencies() .into_iter() @@ -195,41 +212,6 @@ impl TimeDimensionSymbol { .collect() } - pub fn apply_to_deps) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result, CubeError> { - let mut result = self.clone(); - if let Some(granularity_obj) = &self.granularity_obj { - result.granularity_obj = Some(granularity_obj.apply_to_deps(f)?); - } - result.base_symbol = f(&self.base_symbol)?; - Ok(MemberSymbol::new_time_dimension(Rc::new(result))) - } - - pub fn get_dependencies(&self) -> Vec> { - let mut deps = vec![]; - if let Some(granularity_obj) = &self.granularity_obj { - if let Some(calendar_sql) = granularity_obj.calendar_sql() { - calendar_sql.extract_symbol_deps(&mut deps); - } - } - - deps.append(&mut self.base_symbol.get_dependencies()); - deps - } - - pub fn get_cube_refs(&self) -> Vec { - let mut refs = vec![]; - if let Some(granularity_obj) = &self.granularity_obj { - if let Some(calendar_sql) = granularity_obj.calendar_sql() { - calendar_sql.extract_cube_refs(&mut refs); - } - } - refs.append(&mut self.base_symbol.get_cube_refs()); - refs - } - pub fn cube_name(&self) -> String { self.compiled_path.cube_name().clone() } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/granularity.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/granularity.rs index ec4685fde7968..9e12aa2ff4c68 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/granularity.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/planner/time_dimension/granularity.rs @@ -1,6 +1,7 @@ use super::{GranularityHelper, QueryDateTime, SqlInterval}; use crate::planner::sql_templates::PlanSqlTemplates; -use crate::planner::{MemberSymbol, SqlCall}; +use crate::planner::symbols::deps::symbol_deps; +use crate::planner::SqlCall; use chrono_tz::Tz; use cubenativeutils::CubeError; use std::rc::Rc; @@ -17,6 +18,18 @@ pub struct Granularity { calendar_sql: Option>, } +symbol_deps! { + Granularity { + calendar_sql: dep, + granularity: skip, + granularity_interval: skip, + granularity_offset: skip, + origin: skip, + is_predefined_granularity: skip, + is_natural_aligned: skip, + } +} + impl Granularity { pub fn try_new_predefined(timezone: Tz, granularity: String) -> Result { let granularity_interval = format!("1 {}", granularity).parse()?; @@ -83,17 +96,6 @@ impl Granularity { }) } - pub fn apply_to_deps) -> Result, CubeError>>( - &self, - f: &F, - ) -> Result { - let mut result = self.clone(); - if let Some(calendar_sql) = &self.calendar_sql { - result.calendar_sql = Some(calendar_sql.apply_recursive(f)?); - } - Ok(result) - } - pub fn is_natural_aligned(&self) -> bool { self.is_natural_aligned } diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/integration_multi_stage.yaml b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/integration_multi_stage.yaml index e5d19170fea62..0dad048dda9c7 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/integration_multi_stage.yaml +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/test_fixtures/schemas/yaml_files/common/integration_multi_stage.yaml @@ -91,6 +91,40 @@ cubes: - R3 - YTD + # Case-switch dimension over a regular string dimension: a filter + # on `status` statically prunes the branches. + - name: status_case_label + type: string + case: + switch: "{CUBE.status}" + when: + - value: completed + sql: "'done'" + - value: pending + sql: "'waiting'" + else: + sql: "'other'" + + # Calc group: an abstract enumeration with no backing column. + - name: group_mode + type: switch + values: + - by_status + - by_category + + # Case-switch dimension driven by the calc group. + - name: group_mode_label + type: string + case: + switch: "{CUBE.group_mode}" + when: + - value: by_status + sql: "{CUBE.status}" + - value: by_category + sql: "{CUBE.category}" + else: + sql: "{CUBE.status}" + - name: customer_name type: string sql: "{customers.name}" @@ -180,6 +214,44 @@ cubes: add_group_by: - orders.id + # `include` grain refs below carry case-switch / calc-group + # dimensions: the planner selects them inside the multi-stage CTE, + # so static filter pruning must reach them there. + - name: amount_by_status_case + type: number + sql: "{CUBE.total_amount}" + multi_stage: true + add_group_by: + - orders.status_case_label + + - name: amount_by_group_mode_label + type: number + sql: "{CUBE.total_amount}" + multi_stage: true + add_group_by: + - orders.group_mode_label + + - name: amount_by_group_mode + type: number + sql: "{CUBE.total_amount}" + multi_stage: true + add_group_by: + - orders.group_mode + + # Case pruning must follow the leaf's own filters: the `exclude` + # directive drops the status filter inside this measure's CTEs, so + # the case dimension from `include` must stay unpruned there. + - name: amount_by_status_case_unfiltered + type: sum + sql: "{CUBE.total_amount}" + multi_stage: true + grain: + include: + - orders.status_case_label + filter: + exclude: + - orders.status + - name: amount_by_category type: number sql: "{CUBE.total_amount}" diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/include_switch.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/include_switch.rs new file mode 100644 index 0000000000000..c96da49402e9e --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/include_switch.rs @@ -0,0 +1,118 @@ +//! Multi-stage `include` grain refs carrying switch / case-switch +//! dimensions. The planner selects these inside the multi-stage CTEs, +//! and case pruning / calc-group value resolution must follow each +//! leaf's own filters — not the outer query's. + +use crate::test_fixtures::cube_bridge::MockSchema; +use crate::test_fixtures::test_utils::TestContext; +use indoc::indoc; + +fn create_context() -> TestContext { + let schema = MockSchema::from_yaml_file("common/integration_multi_stage.yaml"); + TestContext::new(schema).unwrap() +} + +const SEED: &str = "integration_multi_stage_tables.sql"; + +#[tokio::test(flavor = "multi_thread")] +async fn test_include_case_switch_dim() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.amount_by_status_case + dimensions: + - orders.status_case_label + filters: + - member: orders.status + operator: equals + values: + - completed + order: + - id: orders.status_case_label + "#}; + + let sql = ctx.build_sql(query).unwrap(); + assert!( + !sql.contains("CASE"), + "a filter restricting the switch to one value prunes the case everywhere, including the include copy inside the CTE" + ); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_include_case_switch_dim_with_filter_excluded_from_leaf() { + let ctx = create_context(); + + // The `exclude` filter directive drops the status filter inside the + // measure's CTEs, so the include copy of the case dimension must stay + // unpruned there: every row keeps its real label. + let query = indoc! {r#" + measures: + - orders.amount_by_status_case_unfiltered + filters: + - member: orders.status + operator: equals + values: + - completed + "#}; + + let sql = ctx.build_sql(query).unwrap(); + assert!( + sql.contains("CASE"), + "the leaf runs without the status filter, so its case dimension must keep all branches" + ); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_include_case_over_calc_group() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.amount_by_group_mode_label + dimensions: + - orders.group_mode_label + filters: + - member: orders.group_mode + operator: equals + values: + - by_status + order: + - id: orders.group_mode_label + "#}; + + ctx.build_sql(query).unwrap(); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} + +#[tokio::test(flavor = "multi_thread")] +async fn test_include_calc_group_dim() { + let ctx = create_context(); + + let query = indoc! {r#" + measures: + - orders.amount_by_group_mode + filters: + - member: orders.group_mode + operator: equals + values: + - by_status + "#}; + + ctx.build_sql(query).unwrap(); + + if let Some(result) = ctx.try_execute_pg(query, SEED).await { + insta::assert_snapshot!(result); + } +} diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/mod.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/mod.rs index de8f5ba041c99..97ca662f5aff8 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/mod.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/mod.rs @@ -8,6 +8,7 @@ mod filter_directive; mod filters; mod granularities; mod group_by; +mod include_switch; mod joins; mod multi_fact; mod multiple_measures; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__include_switch__include_calc_group_dim.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__include_switch__include_calc_group_dim.snap new file mode 100644 index 0000000000000..3fe362bc6c011 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__include_switch__include_calc_group_dim.snap @@ -0,0 +1,8 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/include_switch.rs +assertion_line: 116 +expression: result +--- +orders__amount_by_group_mode +---------------------------- +2250.00 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__include_switch__include_case_over_calc_group.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__include_switch__include_case_over_calc_group.snap new file mode 100644 index 0000000000000..83551c6d8c056 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__include_switch__include_case_over_calc_group.snap @@ -0,0 +1,10 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/include_switch.rs +assertion_line: 95 +expression: result +--- +orders__group_mode_label | orders__amount_by_group_mode_label +-------------------------+----------------------------------- +cancelled | 200.00 +completed | 1400.00 +pending | 650.00 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__include_switch__include_case_switch_dim.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__include_switch__include_case_switch_dim.snap new file mode 100644 index 0000000000000..5b739186cda14 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__include_switch__include_case_switch_dim.snap @@ -0,0 +1,8 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/include_switch.rs +assertion_line: 42 +expression: result +--- +orders__status_case_label | orders__amount_by_status_case +--------------------------+------------------------------ +done | 1400.00 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__include_switch__include_case_switch_dim_with_filter_excluded_from_leaf.snap b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__include_switch__include_case_switch_dim_with_filter_excluded_from_leaf.snap new file mode 100644 index 0000000000000..4d6ef86ecb35d --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/snapshots/cubesqlplanner__tests__integration__multi_stage__include_switch__include_case_switch_dim_with_filter_excluded_from_leaf.snap @@ -0,0 +1,8 @@ +--- +source: cubesqlplanner/cubesqlplanner/src/tests/integration/multi_stage/include_switch.rs +assertion_line: 70 +expression: result +--- +orders__amount_by_status_case_unfiltered +---------------------------------------- +2250.00 diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs index 61fcbbf3a235b..b1e1b0bc5055a 100644 --- a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/mod.rs @@ -13,6 +13,7 @@ mod no_query_tools_leak; mod positional_params; mod string_measures; mod subquery_dimensions; +mod time_dimension_symbol; mod utils; mod view_default_filters; diff --git a/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/time_dimension_symbol.rs b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/time_dimension_symbol.rs new file mode 100644 index 0000000000000..72676ae6485f5 --- /dev/null +++ b/rust/cube/cubesqlplanner/cubesqlplanner/src/tests/time_dimension_symbol.rs @@ -0,0 +1,164 @@ +//! Tests for the TimeDimensionSymbol dependency contract. +//! +//! A time dimension is a granularity view of its base dimension, not a +//! consumer of it: the read side looks through the base (emits the +//! base's dependencies, never the base itself), while the transform +//! side receives the base as a replaceable slot. + +use crate::planner::MemberSymbol; +use crate::test_fixtures::cube_bridge::MockSchema; +use crate::test_fixtures::test_utils::TestContext; +use itertools::Itertools; +use std::cell::RefCell; +use std::rc::Rc; + +fn ctx() -> TestContext { + let schema = MockSchema::from_yaml_file("common/visitors.yaml"); + TestContext::new(schema).unwrap() +} + +#[test] +fn deps_of_plain_base_are_empty() { + let ctx = ctx(); + let td = ctx + .create_time_dimension("visitors.created_at", Some("day")) + .unwrap(); + + assert_eq!(td.full_name(), "visitors.created_at_day"); + assert!( + td.get_dependencies().is_empty(), + "the base dimension is not a dependency of its time dimension" + ); + + assert!( + td.get_cube_refs().is_empty(), + "a bare-column base carries no cube refs, so neither does its time dimension" + ); +} + +#[test] +fn cube_refs_look_through_base() { + let ctx = ctx(); + let td = ctx + .create_time_dimension("visitors.visitor_id", Some("day")) + .unwrap(); + + let cube_refs = td.get_cube_refs(); + assert_eq!(cube_refs.len(), 1); + assert_eq!(cube_refs[0].cube_name(), "visitors"); +} + +#[test] +fn deps_look_through_base() { + let ctx = ctx(); + let td = ctx + .create_time_dimension("visitors.minVisitorCheckinDate", Some("day")) + .unwrap(); + + let dep_names = td + .get_dependencies() + .iter() + .map(|d| d.full_name()) + .collect_vec(); + assert_eq!( + dep_names, + vec!["visitor_checkins.minDate".to_string()], + "deps are the base's dependencies; the base itself is not emitted" + ); +} + +#[test] +fn deps_as_time_dimensions_wrap_time_deps() { + let ctx = ctx(); + let td = ctx + .create_time_dimension("visitors.minVisitorCheckinDate", Some("day")) + .unwrap(); + + let wrapped = td + .as_time_dimension() + .unwrap() + .get_dependencies_as_time_dimensions(); + assert_eq!(wrapped.len(), 1); + let dep = wrapped[0].as_time_dimension().unwrap(); + assert_eq!(wrapped[0].full_name(), "visitor_checkins.minDate_day"); + assert_eq!(dep.granularity(), &Some("day".to_string())); + assert_eq!(dep.base_symbol().full_name(), "visitor_checkins.minDate"); +} + +#[test] +fn transform_visits_base_node_once() { + let ctx = ctx(); + let td = ctx + .create_time_dimension("visitors.minVisitorCheckinDate", Some("day")) + .unwrap(); + + let visited = RefCell::new(Vec::new()); + td.apply_recursive(&|node| { + visited.borrow_mut().push(node.full_name()); + Ok(node.clone()) + }) + .unwrap(); + + let visited = visited.into_inner(); + assert_eq!( + visited, + vec![ + "visitors.minVisitorCheckinDate_day".to_string(), + "visitors.minVisitorCheckinDate".to_string(), + "visitor_checkins.minDate".to_string(), + ], + "the transform sees the base as a node even though deps look through it" + ); +} + +#[test] +fn transform_replaces_base_slot_and_keeps_wrapper_identity() { + let ctx = ctx(); + let td = ctx + .create_time_dimension("visitors.minVisitorCheckinDate", Some("day")) + .unwrap(); + let replacement = ctx.create_dimension("visitors.created_at").unwrap(); + + let result = td + .apply_recursive(&|node: &Rc| { + if node.full_name() == "visitors.minVisitorCheckinDate" { + Ok(replacement.clone()) + } else { + Ok(node.clone()) + } + }) + .unwrap(); + + let result_td = result.as_time_dimension().unwrap(); + assert_eq!(result_td.base_symbol().full_name(), "visitors.created_at"); + assert_eq!( + result.full_name(), + "visitors.minVisitorCheckinDate_day", + "replacing the base redirects rendering; the wrapper keeps its own identity" + ); + assert!(result.get_dependencies().is_empty()); +} + +#[test] +fn read_and_transform_agree_on_slots() { + let ctx = ctx(); + let td = ctx + .create_time_dimension("visitors.minVisitorCheckinDate", Some("day")) + .unwrap(); + + let rebuilt = td.apply_recursive(&|node| Ok(node.clone())).unwrap(); + + assert_eq!(rebuilt.full_name(), td.full_name()); + assert_eq!( + rebuilt + .get_dependencies() + .iter() + .map(|d| d.full_name()) + .collect_vec(), + td.get_dependencies() + .iter() + .map(|d| d.full_name()) + .collect_vec(), + "an identity transform preserves the dependency list and its order" + ); +}