feat(tesseract): native domain model representation behind env flag#10986
feat(tesseract): native domain model representation behind env flag#10986waralexrom wants to merge 5 commits into
Conversation
github-actions
Bot
added
rust
|
Claude finished @waralexrom's task in 3m 22s —— View job Code review — verdict: approve with notes (0 high, 0 medium, 4 low)The PR cleanly stages the native model behind DetailsInline findings
Notes on what looks right
Security / perf / docs
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| fn build_case_label(label: StringOrSql) -> CaseLabel { | ||
| match label { | ||
| StringOrSql::String(s) => CaseLabel::String(s), | ||
| StringOrSql::MemberSql(member) => { | ||
| // StructWithSqlMember holds a `sql` callable behind another | ||
| // trait — surface it as Expression. This swallows the | ||
| // `sql()` Result; we'd rather fail at build time, but the | ||
| // bridge surface returns it eagerly. | ||
| match member.sql() { | ||
| Ok(sql) => CaseLabel::Sql(Expression::new(sql)), | ||
| Err(_) => CaseLabel::String(String::new()), | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
build_case_label silently swallows a MemberSql::sql() error and substitutes an empty CaseLabel::String(""). That converts a real bridge failure (e.g. a malformed JS callable returned for a case label) into a built model with a broken/empty label that won't surface until a query actually hits this case. The comment acknowledges this is a shortcut, but since this is the model-build phase (one-shot, errors are already meant to be reported via tryPrepareTesseractModel's warning channel), the natural fix is to make build_case_label return Result<CaseLabel, CubeError> and propagate the error up through build_case_variant. That way a broken label fails fast at build time rather than producing wrong SQL later.
| pub fn add_cube(&mut self, cube: Rc<Cube>) { | ||
| let name = cube.name.clone(); | ||
| self.cubes.insert(name, cube); |
There was a problem hiding this comment.
add_cube silently overwrites on duplicate CubeName. If two cube definitions ever share a name (e.g. a bug in schema-compiler, a stale view) the second one wins with no diagnostic — the model loses a cube and nothing tells you. Consider returning an error when the key is already present, since SchemaModelBuilder::build already returns Result<Model, CubeError> and could surface a clear "duplicate cube" message at model-build time.
| /// Parses a `Cube.member` reference. Returns an error for paths | ||
| /// that do not split into exactly two segments (we'll grow this | ||
| /// to support view-style join chains later). | ||
| pub fn parse(path: &str) -> Result<Self, cubenativeutils::CubeError> { | ||
| match path.split_once('.') { | ||
| Some((cube, name)) if !cube.is_empty() && !name.is_empty() => { | ||
| Ok(MemberPath::new(CubeName::new(cube), name.to_string())) | ||
| } | ||
| _ => Err(cubenativeutils::CubeError::user(format!( | ||
| "Invalid member path: {path}" | ||
| ))), | ||
| } | ||
| } |
There was a problem hiding this comment.
MemberPath::parse is restricted to exactly two segments, but it's already being called on inputs that the JS layer may hand over with a join-hint prefix (e.g. evaluatePreAggregationReferences collects with { collectJoinHints: true }, producing View.Cube.member). In the current PR none of the call sites in builder.rs go through those join-hinted paths (access-policy resolves to cube.member, alias members come from pathFromArray of a 2-element array, view includedMember.memberPath is cube.member, etc.), so this is fine for now — but the restriction is load-bearing and easy to violate as soon as something starts feeding multi-segment paths through. Worth either: (a) leaving a clearer doc-comment that this is intentionally 2-segment-only and any join-hinted input needs different handling, or (b) growing the type now to model Vec<JoinHint> + member so callers can't accidentally bypass it.
| const wrapped = Object.create(dim); | ||
| wrapped.granularities = Object.entries(dim.granularities).map(([name, gran]: [string, any]) => { | ||
| if (gran.name === undefined) { | ||
| gran.name = name; | ||
| } | ||
| return gran; | ||
| }); |
There was a problem hiding this comment.
wrapDimension / wrapPreAggregation mutate the underlying gran / idx objects in place (gran.name = name). These objects live on cube.dimensions[*].granularities / cube.preAggregations[*].indexes in the evaluator, so the mutation leaks back into the cached EvaluatedCube state. It's idempotent on repeated calls, but it also means any downstream code that introspects the evaluator will start seeing a name field stamped on every granularity / index — surprising for a wrapper that's meant to be read-only against the source of truth. Cleaner: spread { name, ...gran } into a fresh object rather than mutating the original.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10986 +/- ##
==========================================
+ Coverage 78.78% 83.33% +4.54%
==========================================
Files 471 255 -216
Lines 94509 77261 -17248
Branches 3520 0 -3520
==========================================
- Hits 74457 64382 -10075
+ Misses 19544 12879 -6665
+ Partials 508 0 -508
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Claude finished @waralexrom's task in 1m 7s —— View job Code review — re-review after push: no new findingsLatest commit ( The 4 prior low-severity inline threads still apply to the current head and remain unresolved — none of the referenced code was touched. Not re-posting to avoid duplicates:
Verdict unchanged: approve with notes (0 high, 0 medium, 4 low — all carried over). The off-by-default Tasks
|
…ehind CUBEJS_TESSERACT_NATIVE_MODEL Introduces the Tesseract domain Model (cubesqlplanner/src/model/*) and the cube_bridge traits to populate it from the JS schema, plus the native endpoints (prepareModel / modelBuildSqlAndParams) and the TesseractModel JS wrapper. Population is gated behind the new CUBEJS_TESSERACT_NATIVE_MODEL flag (off by default): CubeEvaluator builds the model at the end of compile() only when the flag is on. The planner is NOT routed through the model yet — BaseQuery stays on the per-request nativeBuildSqlAndParams path, so the model is built and held but not consumed for SQL. MeasureType supports the multi-stage-only rank / numberAgg types so cubes with rank measures build; build_multi_stage_spec maps rank to a filtering stage on the parsed enum. Tests: native bridge model-roundtrip + extended object-bridges-coverage for the new cube/dimension/measure/segment/granularity/pre-agg getters; Rust YAML-fixture model build incl. rank/numberAgg.
Hierarchies are presentation-only metadata (BI drill-down, exposed via /meta) and never participate in SQL generation, so they don't need to be collected into the Tesseract domain model that the planner will read. Removes model/hierarchy.rs and cube_bridge/hierarchy_definition.rs, Cube.hierarchies, ViewSpec.evaluated_hierarchies, IncludedMemberKind:: Hierarchy, the CubeDefinition.hierarchies() getter, and the harness / fixture references. View included members of kind "hierarchies" are now skipped during model build (they contribute no SQL member).
RollingWindowKind matched the non-existent "time"/"row" types and rejected the real Cube schema values, so a to_date rolling window failed model build. Map the actual schema kinds: fixed, to_date, year_to_date, quarter_to_date, month_to_date.
waralexrom
force-pushed
the
tesseract-native-model-data
branch
from
97f3e9f to
b119529
Compare
|
Claude finished @waralexrom's task in 1m 12s —— View job Code review — re-review after push: no new findingsLatest commit ( The 4 prior low-severity inline threads still apply to the current head and remain unresolved — none of the referenced code was touched by this commit. Not re-posting to avoid duplicates:
Verdict unchanged: approve with notes (0 high, 0 medium, 4 low — all carried over). Off-by-default Tasks
· · Branch |
- build_case_label propagates MemberSql::sql() errors instead of swallowing them into an empty label, so a malformed case label fails at model-build time - SchemaSource wrappers build fresh granularity/index objects instead of mutating the underlying evaluated cube, keeping the wrapper read-only against the evaluator
|
Claude finished @waralexrom's task in 48s —— View job Code review — re-review after push: 2 of 4 prior notes addressedLatest commit (
Two prior threads still apply at the same paths/lines — not re-posting to avoid duplicates:
Verdict: approve with notes (0 high, 0 medium, 2 low — down from 4). Off-by-default Tasks
|
2 participants
Summary
Introduces the Tesseract native domain model — a Rust-side representation of the schema and the bridge to populate it from JS — staged behind the off-by-default
CUBEJS_TESSERACT_NATIVE_MODELflag. The model is built and held but not yet consumed for SQL: the planner stays on the existing per-request path, so this PR is a no-op in production and a foundation for the follow-up that routes planning through the model.Changes
cubesqlplanner/src/model/*): cubes, measures, dimensions, segments, joins, pre-aggregations, access policies, and view resolution, plus thecube_bridgetraits andSchemaModelBuilderthat populate it from the JS schema.prepareModelbuilds the model and hands JS aJsBoxhandle wrapped in aTesseractModel;CubeEvaluatorbuilds it at the end ofcompile()only whenCUBEJS_TESSERACT_NATIVE_MODELis enabled.BaseQueryis unchanged (stillnativeBuildSqlAndParams).MeasureTypenow supports the multi-stage-onlyrank/numberAggtypes so cubes with rank measures build;build_multi_stage_specmapsrankto a filtering stage./meta) and never participate in SQL generation. Pre-aggregation build/refresh metadata (refresh_key, indexes, build_range, …) is collected but not yet read, kept on purpose for the upcoming index/refresh-key SQL work.Testing
cargo check+cargo clippy --testsclean; 975cubesqlplannerlib tests pass (incl. YAML-fixture model build with rank/numberAgg).yarn test:bridge→ 205/205 native bridge tests, including the newmodel-roundtripsuite and the extendedobject-bridges-coveragefor the new cube/measure/dimension/segment/granularity/pre-agg getters.tscpasses on@cubejs-backend/shared,@cubejs-backend/native,@cubejs-backend/schema-compiler.