[pull] master from cube-js:master - #653
Merged
Merged
Conversation
) * test(tesseract): cover FILTER_PARAMS column callbacks referencing members A `FILTER_PARAMS.….filter(cb)` column callback is invoked at render time, and the member references inside it have to resolve to that member's SQL — the same as when the column is handed over as a template string. The two `native planner` cases fail with `Placeholder {arg:0} out of bounds` and stay red until the planner resolves those references; the `legacy planner` cases pin the SQL that the fix has to produce. Also adds FILTER_PARAMS support to the Rust mock member templates — `{FILTER_PARAMS:<cube>.<member>:<column>}`, where `[path]` inside the column is a member reference and `%N` is the Nth filter value — plus a planner-level test that a callback column's placeholder is resolved against the dependency list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tesseract): compile a FILTER_PARAMS column callback into a call of its own A column callback is a SQL function in its own right: it has its own body, its own member references and its own parameters. It was handed to the planner as an opaque JS function and invoked at render time, at which point the references it touched were recorded into a dependency list the planner had already read — so the placeholders it emitted indexed nothing and rendering failed with `Placeholder {arg:0} out of bounds`. The callback is now compiled like any other member sql, with its declared parameters bound to `{fpv:N}` value placeholders, and becomes a `SqlCall` with its own template, dependencies and parenthesisation contexts. Rendering substitutes the filter values into `{fpv:N}` and its own dependencies into `{arg:N}`, so a reference resolves the same whichever way it is spelled, and a compound member is parenthesised according to the context it lands in. Two places keep the callback and render it as-is: a cube's own `sql`, which is the innermost FROM and has no member in scope to resolve against, and a callback taking its values through a rest parameter, whose count only the query knows. A column renders only when its filter reaches the query, so the members it reads are not dependencies of the enclosing member and cannot pull a cube into the join. Reading outside the owning cube is therefore reported when the column renders — leaving queries that never use that filter unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tesseract): reject member references in a cube's own sql A cube's `sql` builds the table the query reads from, so nothing a member reference could resolve against is in scope inside it. Resolving one anyway recursed between the cube table and the member until the stack ran out; the legacy planner does not recurse but emits a qualifier for a table that is not in the query, which the database rejects. All three spellings — a direct `${CUBE.dimension}`, a string `FILTER_PARAMS` column and a `FILTER_PARAMS` column callback — now report the reference instead. The path is classified without building any symbol, so the report replaces the recursion rather than following it. A column callback itself stays allowed, and so does everything in it that needs no member in scope: the filter values it takes and any security context value, which becomes a query param. That keeps the row-level-security shape working, as well as a reference to another cube's `sql`, which resolves to that cube's own table expression. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tesseract): bind a compiled FILTER_PARAMS column only to values it can take A compiled column carries one value placeholder per parameter its callback declares, while a filter supplies as many values as its operator has. The two were never compared, so an operator carrying fewer values left trailing placeholders unbound and the query failed. A filter carrying no values now applies nothing, which is what `set` or `notSet` on the filtered member amounts to and what the legacy planner does. Fewer values than the column takes is reported: the legacy planner fills the missing bound in with the current time, quietly widening the predicate to a range the filter never asked for, which is worse than saying so. Three further holes the same reading turned up: - A cube named directly in a callback passed the own-cube check, since only member dependencies were examined and cube references were dropped. It now renders no qualifier for a table the query does not read. A cube's table expression stays exempt — it inlines the whole expression and needs no join. - The parameter list is read out of the callback's source, which comes up short for a bound or native function and for a `)` inside a comment or a string default. Too few placeholders would have rendered the missing values as `undefined`, so a count that cannot account for every parameter now leaves the callback to render time instead. - A compiled column no longer silently ignores the time shift the surrounding query applies, which would restrict a shifted CTE to the current period and empty it. The cube reported by the own-cube check is picked deterministically; the names arrive unordered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(tesseract): let an active FILTER_PARAMS column contribute its dependencies A column renders only where its filter reaches the query, so the members it reads belong to the enclosing member's dependencies exactly there. Marking each binding by whether the query filters the member it names lets an active column's dependencies flow into join hints, which is what puts the cube it reads into the join — and keeps them out everywhere else, so a query that never uses the filter plans as though the column were not there. That removes the restriction to the owning cube: a column may read whatever its filter's arrival justifies joining. When the join cannot be built the join graph says so on its own, in the same terms as for any other reference — row multiplication for a hasMany, no join path when there is none — so the restriction needed no replacement. Activity is settled per query and again per multi-stage subquery, since `filter: include` lets a subquery filter a member the query around it does not. It starts off, so a symbol compiled for comparison rather than for a query carries no dependency the query would not — a pre-aggregation is matched against the same shape either way. Rewrites reach into an inactive column all the same, so it never keeps a symbol every other reference to it has replaced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tesseract): settle FILTER_PARAMS activity for every symbol that renders one A column renders wherever the symbol carrying it does, and a query reaches symbols through more than its selected members. Activity was settled for dimensions, time dimensions and measures only, so a column inside a segment, a dimension named only in a filter, or a measure named only in a having filter rendered while contributing nothing — and the cube it reads went unjoined, leaving a qualifier with no table behind it. Filters, segments and order items are now settled too, the way case pruning already covered them. A `FILTER_GROUP` renders as one predicate, matched against every member it names, while activity was decided per member. Since partial matching applies to AND groups only, an OR group could match as a whole while a single member of it did not, so the group rendered and none of its members contributed. Activity for a group is now decided from the group's whole member list, which is the same question the render asks — the two can no longer disagree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tesseract): trust a column callback's parameter count only when something vouches for it `Function.length` was the witness that the parameter list had been read in full, but it drops to zero at the first defaulted parameter and can vouch for nothing after that — so a `)` inside a string default of the first parameter closed the list early, the count came back short, and the trailing value rendered as the literal `undefined`. Where `Function.length` cannot speak, the text itself does: a list holding neither a string nor a comment has nothing for the scan to trip over. Also builds the filter set matched against once per pass instead of once per symbol, and records on the pass itself where a multi-stage stage settles activity against the wider set it may filter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tesseract): stop trusting a parameter count past the first default `Function.length` counts the parameters before the first defaulted one, so it catches a scan that stopped short of that point and nothing after it. Reading it as a witness for the whole list left a `)` inside any later default closing the scan early, the count coming back short, and the trailing value rendering as the literal `undefined`. Past the first default only the text can vouch, so the check now applies whatever `Function.length` says: a parameter list holding a string or a comment holds the callback back to the render-time path, where every list that cannot be read in full already goes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tesseract): apply a FILTER_PARAMS column only as far as its filter reaches A column applies what its filter supplies, and nothing when the filter cannot supply what the column takes. That already covered an operator carrying no values; it now covers one carrying fewer than the column declares — a one-sided date operator against a column taking both bounds — instead of reporting it. The filter still reaches the query on its own, so only its restatement inside the member's SQL is dropped. That is narrower than binding a bound the filter never gave: the legacy planner fills the missing one in with the current time, which for `beforeDate` lands the given value on the opposite side of the range and leaves the measure empty for every row the query keeps, and makes the SQL for one query change from day to day. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ctive drops a filter (#11483) * fix(tesseract): keep the query row set when a multi-stage filter directive drops a filter A `filter:` directive frees the inner aggregation stage from a query filter, which is its purpose. On the window path it freed the reported rows along with it, because a window has a single row set serving both roles: the rows it aggregates over and the rows it reports. The measure's CTE then contributed dimension keys the query had filtered out, and the FullKeyAggregate brought them back as result rows carrying NULL for every other measure. Revoke the window path for a member whose directive drops a filter the query restricts the grid by, so the JOIN-model assembles it instead: there the keys side carries the query's rows while the value side spans the widened set. Detection is anchored to the query's own filters — a filter an upstream multi-stage member introduced through its own `filter: include` narrows that member's view, not the result grid, so a descendant dropping it cannot widen anything. The comparison is member-aware because `FilterItem` equality ignores the member a filter targets, and a dropped filter would otherwise be cancelled out by an unrelated look-alike that survived. Also make the explicit-keys join null-safe. A NULL dimension key kept its place in the grid but lost the measure, since `NULL = NULL` is unknown; the shape that derives its keys from the measure refs was already null-safe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tesseract): build the keys side when a multi-stage filter directive drops a query filter Revoking the window path restores the reported row set only for inodes whose grain reshape already demands a keys side. An inode that drops a query filter while keeping the parent grain has nothing to shrink, so it got no keys side and still reported the widened rows: the dimension values the filter excluded came back with NULL for every other measure. Gate the keys side on a dropped query filter as well. The condition it joins is the same one the grain case already satisfies — the measure side no longer enumerates the rows the inode has to report — the difference being that here the grid keeps every dimension and only the row count within it grows. A rollup that does not store the dropped filter's dimension can no longer serve such a query. That is the price of asking the right question: before, the filter was absent from the plan entirely, so the rollup answered a query the user had not made. Exempt a parent state with no dimensions. Its grid is a single row that no filter change can widen, and the keys assembly does not render a keys side without key dimensions. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tesseract): exempt Dimension and Rank inodes from the query-grid keys side Gating the keys side on a dropped query filter reached two inode kinds the grain-shrink gate never could, and neither can use it. `plan_for_cte_dimension_query` builds its FullKeyAggregate without a keys input at all, so a multi-stage dimension carrying a `filter:` directive planned a duplicate child subtree whose CTEs nothing then referenced. A Rank inode ranks within whatever its source carries. Feeding it the keys side shrinks the ranked population to the query grid, which leaves one row per partition and collapses every rank to 1 — trading a widened row set for wrong values. Rank needs its rows restricted after the window, which this assembly cannot express, so it keeps reporting the wider set. Move the member-aware filter-tree comparison to `planner/filter/tree_ops`, next to the other tree walkers and to the equality it compensates for, rather than keeping a private copy in the planner. Order the null-key test by every selected dimension: all of its rows tie on the one key it had, so the snapshot pinned whatever order the plan happened to emit. Cover the value side of a `keep_only` segment drop. With the dropped filter's dimension inside the query grid, the measure agrees with the plain one whichever way the directive resolves, so the existing snapshot cannot detect the directive regressing; grouping by status instead puts the two apart at 600 and 1400. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(tesseract): cover a rolling window over a dropped date filter A rolling window rewrites the date range it hands to its base state, extending it backwards by the trailing interval. The filter an inner multi-stage measure drops therefore no longer carries the query's own bounds, which raised the question of whether the row set could widen the way it does elsewhere. It cannot, and the reason is structural rather than incidental: a rolling window draws its rows from the time series built out of the query's `dateRange`, and its values through the frame condition derived from that same range. Neither depends on the leaf keeping a date filter, so dropping it leaves both the reported rows and the values bounded. Two snapshots pin that — one at the grain of the time dimension alone, one with a dimension and a range narrow enough that the trailing window reaches months the leaf can see and the query did not ask for. Record the reasoning behind the unconditional null-safe keys join as well: the data model carries no nullability information, and a dimension read through an outer join in the join graph arrives as NULL even from a column declared NOT NULL, so the comparison cannot be narrowed to the cases that need it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(tesseract): make the multi-stage CTE dedup key member-aware `eq_as_state` decides whether an already-planned CTE can serve a member, and it compared filter vectors with `FilterItem`'s own equality — which looks at a filter's operator and values but not at the member it restricts. Two states filtering different dimensions to the same value therefore counted as one. That let the keys side dedup onto the very CTE it exists to bound. A directive combining `exclude` with an `include` whose operator and values coincide on another member plans the widened measure child first; the keys child is then matched against it, compares equal, and comes back as the same CTE — so the keys grid is the widened grid and the row set is not held after all. Compare filters with the member-aware equality instead. Two tests come with it: the shape above, which returns the query's six rows rather than nine, and a rank measure with a filter directive, which pins the row set Rank keeps reporting and would fail loudly if Rank were dropped from the keys-side exemption. State the two remaining couplings where they are decided rather than only in tests: the exemptions now sit next to the flag revocation they belong with, and the query-membership check records that a filter whose values were rewritten on the way down reads as one the query never asked for — a rewriting path has to bound the row set by other means, as the rolling-window one does. Drop the `assertion_line` header from the snapshots this branch touches; most of the suite does without it, and it churns them on unrelated line shifts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
1 participant
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 : )