Skip to content

feat(tesseract): default value filters for views (CORE-357)#10892

Open
waralexrom wants to merge 4 commits into
masterfrom
tesseract-default-value-filter
Open

feat(tesseract): default value filters for views (CORE-357)#10892
waralexrom wants to merge 4 commits into
masterfrom
tesseract-default-value-filter

Conversation

@waralexrom
Copy link
Copy Markdown
Member

Summary

Adds support for view-level default value filters (CORE-357). A view can now declare:

filters:
  - member: currency
    operator: equals
    values: [USD]
    unless: [currency]

Whenever the view is touched by a query, the default filter is added; an explicit user filter on the same member can release it via unless. The motivating customer case is a switch dimension where the planner otherwise rolls up across all switch values.

Adds a view-only top-level `filters` field accepting entries with
`member`, `operator`, `values`, and optional `unless`. Wires the new
member/values/unless paths into the JS and YAML transpilers, and
rejects `filters` on regular cubes via Joi's unknown-key validation.
Wires the view-only `filters` field (added to the Joi schema in a703f81)
end-to-end:

- CubeEvaluator.prepareViewFilters resolves `member`/`unless` against the
  view's includedMembers (short, real-cube and view-prefixed paths all
  normalize to the real cube path) and stringifies `values` to match the
  existing FilterItem.values contract.
- New ViewFilterDefinition bridge in cubesqlplanner (static-only, no
  trait fields) plus an optional `filters()` getter on CubeDefinition.
- MockViewFilterDefinition and `filters` field on MockCubeDefinition for
  Rust-side test fixtures.
- backend-native bridge_registry registration with fixture, expected
  field set and round-trip coverage for the new struct.
…ompiler

Default-value filters declared on a view (CORE-357) are now materialized
into the query during `QueryPropertiesCompiler::compile_filters`:

- Active views are detected from compiled MemberSymbols
  (`compiled_path().cube_name()` for every dimension / time-dim / measure
  / filter-member). Each view's `filters()` becomes a candidate.
- A candidate is dispatched through the existing `FilterCompiler::add_item`
  so default filters share the same operator/typing path as explicit ones,
  and get routed into the right bucket (dimension / measure / time-dim).
- `unless` is filter-only: a member referenced only by projection does not
  release the default, since changing what columns the user selects must
  not silently change the row set. Only an explicit filter on the same
  member overrides the default.

JS evaluator (CubeEvaluator.prepareViewFilters) resolves `member` / `unless`
to view-scoped paths (`<view>.<member>`) so they line up with
`MemberSymbol::full_name` on the Rust side.

Test coverage:

- Unit (cube_evaluator-level): default filter applies when view is active;
  projection alone does not trigger `unless`; explicit filter on the
  unless-member releases the default; filter applies when unless-member is
  absent; filter applies even when member is in dimensions if `unless` is
  not declared.
- Postgres integration: virtual `type: switch` currency dim plus a
  `case`-switch measure on `country`; covers union-collapse via default
  filter, projection-not-triggering-unless, explicit-filter-overrides,
  default-filter-with-switch-measure, default-filter-when-unless-member-
  is-absent.

The Postgres seed (`view_default_filters_tables.sql`) deliberately ships
without a `currency` column — currency is purely virtual; `country` drives
the switch-case measure.
New file scaffolded on calendars.test.ts: a single inline YAML model with
both a real string dimension (`country`) and a virtual `type: switch`
dimension (`currency`), each exposed through two views — `_unconditional`
and `_with_unless`. Tests run through the full JS pipeline (Joi schema,
YAML transpiler, CubeEvaluator.prepareViewFilters, BaseQuery, Tesseract
planner) against testcontainers Postgres, gated on `nativeSqlPlanner`.

Six cases — three per filter flavour: default applies without `unless`,
projection alone does not release the default, an explicit filter on the
unless-member overrides the default.

YamlCompiler fix in the same commit: `filters.[N].member` (string) and
`filters.[N].unless` (array) are now wrapped as f-string literals, like
`values`. They are member references in the view's own namespace, not
Python expressions — and the view's `includedMembers` are not resolvable
at transpile time, so the previous default path treated names like
`country` as undefined identifiers and blew up at runtime with
`country is not defined`.
@waralexrom waralexrom marked this pull request as ready for review May 15, 2026 14:24
@waralexrom waralexrom requested review from a team as code owners May 15, 2026 14:24
@github-actions github-actions Bot added rust Pull requests that update Rust code javascript Pull requests that update Javascript code labels May 15, 2026
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 15, 2026

Claude finished @waralexrom's task in 3m 32s —— View job


Code Review: feat(tesseract): default value filters for views (CORE-357)

  • Gather context and understand the full diff
  • Review schema compiler changes (TypeScript)
  • Review Rust bridge and planner changes
  • Review test coverage
  • Post comprehensive review feedback

Overall Assessment

This is a well-structured feature addition with clean layer separation — schema validation (Joi), reference resolution (CubeEvaluator), bridge definition (Rust ViewFilterDefinition), and query-time application (QueryPropertiesCompiler). The unless semantics (filter-only, projection doesn't release the guard) are a good design choice and well-documented throughout.

What Looks Good

  • Layered validation: Joi catches structural errors early, CubeEvaluator.prepareViewFilters validates membership against includedMembers, and the Rust planner receives only resolved references. Each layer has clear responsibility.
  • unless is filter-only: The decision that pulling a member into the projection doesn't silently change the row set is the right call. It's documented inline and tested explicitly in both Rust unit tests and integration tests.
  • Comprehensive test coverage: Validator tests (valid/invalid operator, missing member, regular cube rejection), schema compiler tests (short/real-path/view-path resolution, unincluded members), Rust unit and integration tests, Postgres e2e tests, and bridge coverage tests. Hard to find a scenario that isn't covered.
  • String coercion: Values are coerced to strings via String(v) in CubeEvaluator to match the Option<Vec<Option<String>>> contract used by regular query filters — nulls are preserved correctly.
  • Bridge wiring: The new viewFilterDefinition is properly exported in bridge_test_exports.rs with a fixture and coverage assertion.

Issues Found

1. Segments not included in view activation (filed as inline comment)

apply_view_default_filters iterates dimensions, time dimensions, measures, and filter members — but not segments. If a query touches a view only through a segment, the view's default filters won't activate. segments is compiled at line 50 of query_properties_compiler.rs but never passed into compile_filters. This may be intentional if segments-only view queries are rare, but it should be documented or fixed.

2. Missing isView guard in prepareViewFilters (filed as inline comment)

The method relies on !cube.filters to skip non-views, which works because Joi strips filters from regular cubes. But adding an explicit cube.isView check would make the contract more robust.

Minor Observations

  • YamlCompiler duplication (inline comment): The isViewFilterMember branch duplicates the sql/sqlTable f-string path — could be merged for readability.
  • unless between default filters: If view A declares two default filters where filter-2's unless references the same member as filter-1, the unless check uses only user-supplied filters (the mentioned_in_filters set is built before default filters are added). This means default filters can't suppress each other, which seems intentionally correct — unless is a user-override mechanism, not an inter-filter dependency system.
  • Validator uses Joi.any().valid(...) for operator: Consistent with existing patterns in the file, but Joi.string().valid(...) would be slightly more precise. Non-blocking.

Test Coverage Summary

Layer Tests Scenarios
Validator (TS) 7 tests Valid filters, set without values, missing values/member, invalid operator, regular cube rejection
Schema (TS) 5 tests Reference resolution (3 forms), member not included, unless not included, non-included cube path
Rust unit 5 tests Default applies, unless filter-only, explicit override, unless member absent, unconditional
Rust integration 5 tests Switch union collapse, unless projection-only, explicit override, unless absent, switch-case measure
Postgres e2e (TS) 6 tests Real dimension + virtual switch × unconditional/with-unless/explicit-override
Bridge 3 additions Fixture, coverage assertion, invoke function

Solid work overall. The segments gap is the one functional concern worth addressing before merge.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 15, 2026

Claude finished @waralexrom's task in 2m 59s —— View job


Code Review: feat(tesseract): default value filters for views (CORE-357)

  • Gather context and understand PR changes
  • Review schema compiler changes (TypeScript)
  • Review Rust bridge and planner changes
  • Review test coverage
  • Review security and edge cases
  • Post final review

This is a well-structured PR that adds view-level default value filters across the full stack (schema validation → JS evaluator → Rust bridge → Tesseract planner). The implementation is clean, the design decisions are well-reasoned, and the test coverage is thorough. Below are my findings.


Architecture & Design

The overall design is sound:

  1. Schema validation (CubeValidator.ts) — new ViewFilterSchema with Joi, correctly restricts values requirement based on operator (set/notSet don't need values).
  2. Evaluation (CubeEvaluator.ts) — prepareViewFilters() resolves member references against includedMembers, validates membership, coerces values to strings.
  3. Rust bridge (view_filter_definition.rs) — clean NativeBridgeStatic struct matching the JS output shape.
  4. Planner (query_properties_compiler.rs) — apply_view_default_filters() collects active views from query members, applies filters unless released by unless.

The unless being filter-only (not projection-based) is a deliberate and well-documented design choice that prevents surprising row-count changes when columns are added to the SELECT. Good call.


Issues & Suggestions

1. resolveViewMember silently drops resolution on dotted paths (Low Severity)

In CubeEvaluator.ts:238-248, the resolveViewMember function handles three cases: bare name, view-prefixed name, and dotted (cube) path. For the dotted path case (lookupPath), it searches includedMembers by memberPath:

const match = lookupPath
  ? included.find((m) => m.memberPath === lookupPath)
  : included.find((m) => m.name === lookupName);

If a user writes orders.currency (cube-qualified) and the included member's memberPath is also orders.currency, this works fine. But if memberPath uses a different format (e.g., with join paths), the lookup silently fails and falls through to the error reporter. This seems intentional based on the test at schema.test.ts:620 (other.currency → error), but worth confirming that the memberPath format is stable across join paths.

2. Missing negative test for filters on non-view cubes in Rust tests (Nitpick)

The TypeScript validator test at cube-validator.test.ts:329 correctly verifies that a regular (non-view) cube with filters is rejected. However, there's no corresponding test on the Rust side verifying that apply_view_default_filters skips non-view cubes. The code does check is_view at query_properties_compiler.rs:465, but a unit test would improve confidence.

3. ViewFilterSchema operator list doesn't match BaseFilter operators (Observation)

The operator allowlist in CubeValidator.ts:1133-1157 includes all standard operators. This is a superset of what makes semantic sense for a "default value" filter (e.g., inDateRange as a default filter is unusual but technically valid). This seems intentional — no restriction beyond what the filter engine already supports — but worth flagging for documentation purposes.

4. Potential for stale filter_members if add_item modifies existing filters (Low Risk)

In query_properties_compiler.rs:433-436:

let filter_members: Vec<Rc<MemberSymbol>> = filter_compiler
    .iter_all_items()
    .flat_map(|f| f.all_member_evaluators())
    .collect();

The comment correctly notes this is materialized once to avoid borrow conflicts. The mentioned_in_filters set is built before default filters are added. This means default filter A cannot be "unless'd" by default filter B from the same view. This is the correct behavior (only explicit user filters release the guard), and it's well-documented in the comments. Just confirming this is intentional.

5. YamlCompiler special-casing is a bit fragile (Low Severity)

In YamlCompiler.ts:195-200, the isViewFilterMember and isViewFilterUnless regex checks are added inline:

const isViewFilterMember = /^filters\.\d+\.member$/.test(fullPath);
const isViewFilterUnless = /^filters\.\d+\.unless$/.test(fullPath);

These regexes run on every property path for every transpiled field. The performance impact is negligible, but the approach of adding more special-case regexes to this already-complex transpilation loop could become harder to maintain. Not actionable for this PR, but worth noting for future contributors.


Test Coverage Assessment

Excellent coverage across all layers:

Layer Tests Verdict
Schema validation (Joi) 6 tests: valid filter, valid with unless, set operator, missing values, missing member, invalid operator, non-view cube rejection Thorough
JS evaluator (CubeEvaluator) 5 tests: reference resolution (short/real-path/view-path), member-not-included error, unless-not-included error, fully-qualified non-included path error, mixed-type value coercion Thorough
Rust unit tests 5 tests: default applies, unless + projection keeps filter, unless + explicit filter releases, unless member absent, no-unless + projection Thorough
Rust integration (snapshot) 5 tests with Postgres execution: switch union collapse, unless projection, explicit override, unless member untouched, switch-case measure Thorough
E2E Postgres 6 tests: real dimension + switch dimension, each with unconditional / unless+projection / unless+explicit filter Thorough
Bridge coverage Fixture + fields meta for viewFilterDefinition added Complete

Edge cases covered:

  • Null values in filter values array (coerced correctly)
  • Mixed types (numbers, booleans, strings, null) in values
  • set/notSet operators without values
  • Members referenced by short name, cube-qualified path, and view-qualified path
  • Members not included in the view (error)
  • Unless references not included in the view (error)

One scenario not explicitly tested: Multiple default filters from the same view where one filter's unless member is also another filter's member. The current code handles this correctly (unless only checks explicit user filters, not other defaults), but an explicit test would document this behavior.


Security

No security concerns. The feature:

  • Only operates on server-side schema definitions, not user input
  • Values are coerced to strings matching the existing FilterItem.values contract
  • Member references are validated against includedMembers before reaching the planner
  • No SQL injection risk — filters go through the same FilterCompiler path as regular filters

Summary

This is a high-quality PR. The implementation is well-layered, the design decisions (especially filter-only unless) are clearly documented with inline comments explaining the why, and the test coverage is comprehensive across all layers. The code is clean and follows existing patterns in both the TypeScript and Rust codebases.

Verdict: Looks good to merge. The issues noted above are low-severity observations rather than blockers.

Comment on lines +454 to +458
for sym in dimensions
.iter()
.chain(time_dimensions)
.chain(measures)
.chain(filter_members.iter())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug (minor): Segments are excluded from the view activation scan. If a query touches a view only through a segment (e.g., segments: [orders_view.some_segment] with no dimensions/measures from that view), the view's default filters won't fire.

segments is compiled at line 50 but never passed into compile_filtersapply_view_default_filters. Consider whether segments should also participate in view activation. The fix would be to thread segments through compile_filters and add it to this chain — segment FilterItems contain MemberSymbols accessible via all_member_evaluators() just like the other buckets.

This may be an intentional omission if segments-only queries are rare, but it's worth documenting either way.

Comment on lines +217 to +220
private prepareViewFilters(cube: any, errorReporter: ErrorReporter) {
if (!cube.filters) {
return;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit (defensive): prepareViewFilters relies on !cube.filters to skip regular cubes. The Joi schema already prevents filters from appearing on non-view cubes, so this works in practice. But since prepareViewFilters accesses cube.includedMembers (which is undefined on regular cubes), an explicit isView guard here would make the contract clearer and more robust against future schema changes:

Suggested change
private prepareViewFilters(cube: any, errorReporter: ErrorReporter) {
if (!cube.filters) {
return;
}
private prepareViewFilters(cube: any, errorReporter: ErrorReporter) {
if (!cube.isView || !cube.filters) {
return;
}

Comment on lines +196 to +201
const isViewFilterMember = /^filters\.\d+\.member$/.test(fullPath);
const isViewFilterUnless = /^filters\.\d+\.unless$/.test(fullPath);
if (typeof obj === 'string' && ['sql', 'sqlTable'].includes(propertyPath[propertyPath.length - 1])) {
return this.parsePythonIntoArrowFunction(`f"${this.escapeDoubleQuotes(obj)}"`, cubeName, obj, errorsReport);
} else if (typeof obj === 'string' && isViewFilterMember) {
return this.parsePythonIntoArrowFunction(`f"${this.escapeDoubleQuotes(obj)}"`, cubeName, obj, errorsReport);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: The isViewFilterMember branch (line 199–200) duplicates the sql/sqlTable branch above it — both produce parsePythonIntoArrowFunction(f"...", ...). Consider merging them:

} else if (typeof obj === 'string' && (
  ['sql', 'sqlTable'].includes(propertyPath[propertyPath.length - 1]) ||
  isViewFilterMember
)) {
  return this.parsePythonIntoArrowFunction(`f"${this.escapeDoubleQuotes(obj)}"`, cubeName, obj, errorsReport);
}

Minor readability win — not blocking.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 15, 2026

Codecov Report

❌ Patch coverage is 80.00000% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.87%. Comparing base (22031fb) to head (3671fb6).
⚠️ Report is 12 commits behind head on master.

Files with missing lines Patch % Lines
...ubejs-schema-compiler/src/compiler/YamlCompiler.ts 0.00% 6 Missing ⚠️
...bejs-schema-compiler/src/compiler/CubeEvaluator.ts 92.10% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master   #10892    +/-   ##
========================================
  Coverage   78.86%   78.87%            
========================================
  Files         470      470            
  Lines       92289    92478   +189     
  Branches     3435     3448    +13     
========================================
+ Hits        72788    72944   +156     
- Misses      18999    19030    +31     
- Partials      502      504     +2     
Flag Coverage Δ
cube-backend 58.42% <80.00%> (+0.06%) ⬆️
cubesql 83.47% <ø> (+<0.01%) ⬆️

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

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

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

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

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant