[pull] master from cube-js:master - #684
Merged
Merged
Conversation
* fix(tesseract): resolve pre-aggregation references interpolating the cube
A pre-aggregation reference written as (CUBE) => `${CUBE}.issued_date`
stringifies the cube, so the member arrives as literal text next to a cube
reference rather than as a member symbol, and the reference resolved to no
member at all: a time_dimension in that form failed every query on the cube,
while a measures / dimensions / segments entry was dropped, leaving a rollup
that is built and refreshed but never matched.
Read such an element back as a member path — the cube reference's path
followed by the literal segments — and resolve it against the data model.
Reference elements are now read in declaration order, and an element naming no
member is reported with the member and pre-aggregation names instead of being
dropped.
* fix(tesseract): read reference template indices safely
An element that names a member through one placeholder while carrying an index
the recorded dependencies don't cover — `{arg:0} || {arg:7}` — indexed the
dependency list unchecked and panicked instead of reading what it could. Every
index now goes through `get`, and one it doesn't cover names nothing.
Alongside it: a failure to reach the data model while resolving a recovered
path is passed through as it is rather than reported as a missing member, and
the placeholder needle is built once per element instead of once per match.
Pins two shapes that had no coverage: an out-of-range index next to a member,
and a granularity named inside the reference (`${CUBE}.created_at.day`), which
is rejected exactly as the equivalent symbol reference is. The integration
helper now states that its row assertions do run when Postgres execution is
enabled.
…ry (#11599) * fix(tesseract): widen pre-agg date range for time_shift behind a view A multi_stage measure with time_shift returned NULL for every row when queried through a view while a pre-aggregation was matched. The shifted leaf scanned a partition set that could not contain its rows. Time shifts are keyed by the fully resolved cube member: QueryProperties builds them from all_time_members(), which peels the TimeDimension wrapper and follows the reference chain. extract_date_range probed that map with BaseFilter::member_name(), which resolves neither, so a view-qualified filter never found its shift and the range was left un-widened. Add TimeShiftState::get_for_symbol, which normalizes the probe the same way the keys are built, and route the lookup sites through it. Preferred over a fallback second lookup so the key-normalization rule lives in one place instead of being re-derived per call site; member_name() is left alone because its other callers compare against query-level names, where view qualification is consistent. TimeShiftSqlNode keeps its own probe: it is guarded on a non-reference symbol, and resolving there would apply the shift twice. Covered by a view-level test sitting next to the existing cube-level one, asserting the shifted and unshifted usages carry different date ranges. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tesseract): apply time_shift to a derived time dimension read from a rollup A time_shift declared on a time dimension that wraps another cube's time dimension was lost entirely once a pre-aggregation served the query: the shifted leaf read the same rows as the unshifted one, so the shifted measure silently repeated the current period instead of the previous one. The same query without a pre-aggregation was correct. Dimension-specific shifts are keyed by the owned member the declared dimension wraps, because that is where the interval lands when the member's SQL is expanded. Two things then went wrong when the rollup materialized the derived dimension instead: - extract_date_range probed only the chain-resolved name, so the range was never widened. get_for_symbol now probes the owned child too, covering both ways a key is built. - The rollup column is substituted for the dimension, so its SQL is never expanded and the recursion that normally carries the shift down to the owned member never happens. TimeShiftSqlNode now applies the shift to the column itself, but only for dimensions it knows are substituted — an evaluated dimension must still wait for the recursion, or the interval would be added twice. Covered by a test asserting the widened range, a single shift on the rollup column, and — on a seed holding a period before the queried range — the executed values. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(tesseract): harden time_shift rendering and its tests after review Fall through instead of unwrapping a shift with no interval, so TimeShiftSqlNode treats it as "no shift" like the other two consumers of the same lookup rather than panicking. Assert the single-application invariant in the derived-dimension test by requiring every rendered interval to sit directly on the rollup column, instead of matching one exact textual form of a doubled shift. Record what the view test's executed rows do and do not cover: the widened range only selects rollup partitions, which the harness does not emulate — it loads each rollup whole — so only the assertions on the usages guard the widening. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tesseract): fail loudly on a time shift with no interval A shift entry reaching the renderer without an interval was rendered unshifted, turning a state the map calls shifted into silently wrong numbers. Return an error instead. Also record why the first probe is by exact name: a dimension that gets evaluated picks its shift up when the recursion reaches the owned member it wraps, so matching it at the outer level too would add the interval twice. Only a substituted dimension, never expanded, resolves through the chain. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tesseract): skip a rollup that cannot carry the query's time shift A time dimension built from several members, only some of which the shift covers, has no valid offset of its stored column: moving the column would carry along the rows the shift must leave in place. The rollup was matched anyway and the shift was dropped, so the shifted measure silently repeated the unshifted one. Reject such a pre-aggregation during matching. The unrewritten leaf then triggers the existing rollback of the whole multi-stage rewrite and the query falls back to base SQL, which computes the shift correctly. The gate is tied to the shift lookup rather than re-deriving reachability: reject exactly when a shift is involved but cannot be attributed to the stored column. Re-deriving the rule would add a second place obliged to stay in step with the lookup. The test's expected values were captured from the same query with pre-aggregations disabled, before the gate existed. They differ from what offsetting the stored column would produce, which is what rules that approach out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tesseract): gate every stored member on carrying the query's time shift The gate scanned only a pre-aggregation's time dimensions, but dimensions and segments are substituted by column just the same. A dimension built from a partially shifted member slipped through, and the shifted leaf then read it computed from unshifted values: both leaves rendered identically, so the shifted measure repeated the unshifted one. Check every member the pre-aggregation stores. The type is not what matters — any stored column computed from a shifted member is wrong when read unshifted — so dimensions and segments are checked whatever they hold. Tests cover all three ways such a member reaches a rollup: as its time dimension, under dimensions, and through a segment. Each was confirmed to fail with its own part of the gate removed. Their snapshots pin what makes the stored column unusable: the row the shift leaves in place lands on the same key in both stages, which no offset of a single column reproduces. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tesseract): decide the time-shift gate per member the query reads Three ways the gate reached the wrong verdict, each confirmed by comparing the rollup plan against the same query without one. A measure was not examined at all. One whose SQL reads the shifted dimension is stored aggregated from unshifted values, and no offset recovers it, because a shift changes which rows feed an aggregate rather than the value itself. Measures are therefore rejected outright whenever a shift reaches them, never merely attributed — attribution succeeds for a measure with a single dependency and would have admitted exactly the broken case. Only the measures the match consumes are examined. A stored reference to the shifted dimension was admitted although the renderer resolves a reference through to what it points at instead of offsetting the column, leaving it unshifted. The gate asked whether a shift could be attributed while the renderer asked whether it would apply one; the two are now the same question, asked through `shift_for_substituted_column`, so they cannot disagree again. A member the query never reads could reject the whole rollup. Nothing renders such a column, so it cannot make the stored data wrong; the gate now looks only at the members the node actually reads, matching how measures were already treated. Tests cover all five ways a shift reaches a rollup — time dimension, dimension, segment, measure, reference — plus the case that must stay matched. Each was confirmed to flip with its own part of the gate removed, and the kept-rollup values were checked against a base-SQL run rather than against the rollup that produced them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(tesseract): pin the pushed-down filter column under a time shift A FILTER_PARAMS column bound to a time dimension derived from another cube's is offset in the shifted stage and left bare in the unshifted one. Both forms are asserted, so losing the offset and applying a spurious one are equally caught. Without the shift lookup resolving through the derivation, the shifted stage filtered the source rows by unshifted bounds while grouping by shifted values. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…11628) * fix(cubestore): transmit the router's planning flags with the query A select worker plans its own half of a split plan from the logical plan it receives, and it read `group_by_limit_factor` and `topk_strategy` from its own configuration. When the values at the two ends of a hop disagree, the halves do not fit together and the query returns silently wrong rows instead of failing. Both flags now travel in `WorkerPlanningParams` as `PlanningFlags`, stamped by the router into `ClusterSendExec` and reused by the worker for both hops. The field is optional: a sender that omits it predates the flags and planned from its own configuration, so the receiver falls back to its own configuration rather than to a hardcoded default. * fix(cubestore): pin the planning-flags wire contract in tests Cover the line the whole mechanism turns on: `worker_planning_params()` emitting `Some(flags)`. A regression to `None` would have been papered over by the receiver's configuration fallback, with the silently-wrong-rows failure mode this is meant to close. Pin the strategy names on the wire with explicit `serde(rename)` (the same names `CUBESTORE_TOPK_STRATEGY` accepts), so renaming a variant cannot break a mixed-version cluster, and state the deployment constraint the fallback cannot cover: a value set on the router alone is not reproducible on a receiver that gets no flags. * docs(cubestore): tighten the planning-flag doc comments Keep the three facts that matter -- both halves must be planned from one value, the wire names are the env names, an unknown strategy fails the deserialize -- and drop the restatements.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )