DEV-1703: typed-pipeline branch → dev-1450 (latest: merge origin/main) - #263
Open
ZmeiGorynych wants to merge 625 commits into
Conversation
…ne only _settings_holder walked all descendant SELECTs (find_all), so a SETTINGS clause on a subquery nested in a UNION branch's FROM would capture our statement-level flag, leaving the outer correlated EXISTS without it. Follow only the set operation's own right-spine branch (where trailing UNION SETTINGS actually attach) or fall back to the root; never a nested FROM subquery. Adds a regression test. (Codex) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Opt-in per-call via execute(query, cache=True); per-engine, in-memory. TTL (lazy on read) + Cube-style refresh keys scanned by engine.refresh(). Adds evict/clear_cache/cache_size and sync wrappers. Refactors execute into _normalize_input + DB-free _prepare_pipeline shared by execute, evict, and refresh re-execution. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…er-query-cache-in-slayer # Conflicts: # CLAUDE.md # mkdocs.yml # slayer/engine/query_engine.py
Runnable, self-contained notebook demonstrating miss/hit, staleness, refresh keys, TTL, and eviction on a temp SQLite database. Wire it into the Tutorials nav; cross-link with the concept doc. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ig, scope-based table detection - Apply the forced-filter policy to refresh-key scans so the baseline is computed over the same tenant-scoped rows as the cached data query (Codex): thread datasource through _scan_refresh_keys/_scan_one_table. - Freeze CacheConfig + coerce refresh_keys to a tuple so in-place mutation can't silently bypass the cache-clearing setter (Codex). - Detect physical tables via sqlglot traverse_scope instead of excluding by bare name, so a table sharing a CTE alias name is still matched (CodeRabbit). - Correct the _prepare_pipeline / evict 'DB-free' docstrings: under a policy, column-presence introspection runs (CodeRabbit). - Tests: pytest.approx for float sums (Sonar S1244), narrow + hoist the baseline-failure exception assertion (Sonar S5778/S5958), NOSONAR S3776 on _build_response_metadata; add RLS-scoped-scan and frozen-config tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A schema/catalog-qualified table can never be a CTE (CTE names are unqualified), so only bare names need the scope-shadow check. Fixes a physical table sharing a CTE's bare name (e.g. public.cte) being skipped (Codex follow-up). Add a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A cache entry is bound to the datasource it originally resolved against (part of the key). refresh() already scans that datasource's refresh keys, so re-execution must pin to it via entry.resolved_data_source rather than re-resolving through a possibly-changed priority list and migrating the entry to a different datasource (Codex follow-up). Add a priority-drift regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
JoinFilterRule.join_path is now a tuple of hop strings
("from_table.from_column = to_table.to_column") instead of structured
JoinHop objects. Strings are the sole public/serialized form and
round-trip symmetrically; JoinHop survives only as an internal,
un-exported parse product.
- _parse_hop() naive-splits each hop (one '=', last-dot table/column
split, whitespace-tolerant, schema/catalog qualifiers preserved).
- JoinFilterRule.parsed_hops @Property derives the internal JoinHops
fresh from join_path on each access (no cache -> a model_copy that
swaps join_path can't go stale). Chain validators run on parsed_hops;
a bare-string join_path is rejected.
- _build_exists reads rule.parsed_hops (only SQL-layer change).
- JoinHop dropped from __all__; dotted column names are out of scope.
Docs (CLAUDE.md, row-level-security.md, RLS notebook) moved to the
string form. Full non-integration suite green; ruff clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review feedback on the string-hop refactor: - Codex: model_copy(update=...) bypasses Pydantic validation, so a copied JoinFilterRule could feed a non-chaining join_path to SQL generation. Move the chain check into a module-level _validate_hop_chain() run by the parsed_hops accessor (not just at construction), so a broken copy fails closed on access. The construction validator now delegates to parsed_hops. - Sonar S5778: hoist the _hop() call out of the pytest.raises block in test_join_rule_kind_literal_enforced (only one throwing call per raises). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add `slayer import-osi` to convert OSI semantic-model configs (YAML/JSON, file or directory) into SLayer models, mirroring import-dbt's CLI-only shape (parse -> convert -> save_model per model -> printed report). OSI maps directly to SLayer, not via the dbt intermediate. - slayer/osi/: models (schema port), source (source parsing + routing stub), expression (sqlglot -> colon formula, derived-column materialization), converter (OsiToSlayerConverter) - Live table + query-source introspection for real column types; OSI overlays labels/descriptions/is_time/ai_context/primary keys; unique_keys + custom_extensions -> meta - Relationships -> LEFT ModelJoin (composite-safe); metrics -> ModelMeasure anchored via shared join_graph.min_hops_root (also now used by recommend_root_model); orphan COUNT(*) with no unique fact table errors - ModelJoin gains optional description/meta (additive, no version bump) - Extract ConversionResult/ConversionWarning to slayer/ingest_report.py, re-exported from slayer/dbt/converter.py Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The inline comment `# target_table="orders"` looked like commented-out code to Sonar (S125). Rephrase as prose — no behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- converter: qualified metric refs now verify the column exists on the qualified model (Codex) — SUM(orders.no_such_col) clean-fails instead of importing a query-time-broken measure; test added - converter: reuse core.refs.IDENTIFIER_RE (S6353); drop dead sm_of_dataset dict; extract _build_measures_for and _overlay_one_field to cut cognitive complexity (S3776) - expression: math.isclose for the median 0.5 check (S1244); extract _residual_violation (S3776); list-comprehension instead of list(gen) (S7504) - parser: resolve() the caller-supplied path before filesystem access (S8707) - test: hoist arg construction out of pytest.raises (S5778) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- converter: a derived OSI field whose name matches a physical column now overlays its expression onto that column instead of being silently dropped - expression/converter: materialized hidden-column names are reserved against existing columns (name_taken callback) so a metric never aggregates a colliding pre-existing column - converter: enforce SLayer namespace invariants before the post-construction measure append — duplicate metric names and metric-vs-column collisions clean-fail instead of persisting a model that fails to load - converter: relationship join columns are validated to exist on both models; a typo clean-fails instead of emitting a query-time-broken join - tests for all four Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- converter: bare-alias fields that shadow a physical column now REPLACE it (my prior fix only covered the derived-expression branch) - converter: materialized hidden-column names are reserved against columns AND measures (SLayer shares one namespace), not columns alone - converter: building a ModelMeasure is guarded — a metric named after a reserved transform (cumsum, ...) clean-fails instead of crashing the import, and leaves no orphan hidden columns - converter: an unqualified metric column present on multiple datasets is ambiguous and clean-fails instead of binding by dataset order - tests for all four Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tence) - converter: derived field expressions now validate their (unqualified) column references exist on the table (_missing_expr_columns), consistent with the bare-field / metric / relationship checks - expression: a materialized aggregate operand with any unresolved column now clean-fails instead of discarding the None owner and materializing SQL that references the missing column - tests for both Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dation) - converter: derived field expressions that fail to parse now clean-fail (_missing_expr_columns returns None on ParseError) instead of importing a column with invalid SQL - converter: self-qualified column refs (<dataset>.col) in derived field expressions are validated for existence too; genuinely cross-model refs stay deferred to query-time join resolution - tests for both Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A derived field expression may reference a joined model via <alias>.<col> / <a>__<b>.<col>. SLayer's Column.sql resolver does NOT clean-fail a bad such ref (it leaves an unknown alias untouched, or rewrites a known-model/unknown-column ref into SQL that errors at the DB at query time). So the importer now runs a post-join validation pass (_validate_cross_model_field_refs) that walks each cross-model ref through the join graph and drops+reports any column whose ref names a model with no join path or a nonexistent target column — clean-failing at import instead of failing opaquely at query time. Unqualified and self-qualified refs are still validated at field-overlay time. Tests added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two valid-on-SQLite / invalid-on-Postgres compiler defects, both surfacing as UndefinedColumn on real execution (SLayer's dry_run compiles without executing, which hid them until submit_query's dry-run gate). Flavor A - ORDER BY on an unprojected/renamed column. The sort key was rendered as one composite-quoted identifier "<model>.<col>", which resolves only when the column is a projected output alias. _resolve_order_column now returns a discriminated _OrderColRef: projected aliases stay whole-quoted; non-projected/renamed keys emit a split table.column reference (two identifiers) that resolves against the FROM-scope column. Applied at all three ORDER BY emission sites. Flavor B - mixed-case identifiers emitted unquoted, so case-folding dialects (Postgres/Redshift to lower; Snowflake/Oracle to upper) fold them to a non-existent name. A context-aware quoting pass quotes real DB identifiers - column-name leaves and physical table-name parts - while leaving SLayer-internal aliases/qualifiers alone (they fold consistently within a query). Wired into both parse paths (_parse and _parse_predicate) and every direct AST-construction site: _resolve_sql, _build_from_clause, cross-model measure tables/join keys, resolved-join targets, and sql=None measures. Universal across dialects. Tests: new tests/test_dev1645_invalid_postgres_sql.py (unit: both flavors, helper-level, multi-dialect); real-Postgres integration tests reproducing the exact UndefinedColumn failures; a Snowflake upper-fold test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- converter: cross-model field validation now runs to a FIXED POINT — dropping a column can invalidate another column that referenced it, so re-run until a pass drops nothing (transitive chains resolve correctly) - converter: the validator is now scope-aware (reuses column_expansion's _root_scope_column_ids) so nested subquery/CTE aliases are not mistaken for join refs, and catalog/db-qualified physical refs are skipped — no false drops - converter: a second relationship from a model to the same target is reported and skipped (SLayer joins key only on target_model and can't disambiguate multiple joins to one model) - tests for all three Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
_resolve_expression returned the requested --dialect even when it was non-SQL (MDX/MAQL/TABLEAU), feeding non-SQL syntax into the SQL conversion path. Now the requested dialect is used only when it is in SQL_DIALECTS; a non-SQL request falls back to an available SQL dialect (or clean-fails if none). Test added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… queries _looks_like_query ran the SELECT regex over the whole source string, so a valid quoted identifier segment like "My Select" routed the source to sql-mode. Run the SELECT check on the text outside double-quoted spans (the space check already respects quotes). Test added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…EV-1645) Codex review of PR #224: the first/last aggregation path referenced group-by dimensions by bare name against the ranked subquery's model.* output via unquoted exp.to_identifier(dim.name). A mixed-case dimension folded to lower on Postgres (upper on Snowflake) and failed to match the subquery output column. Route both ranked-path dimension-reference sites through _to_ident so mixed-case names are quoted, matching the rest of the DEV-1645 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Only COUNT(DISTINCT) maps to a SLayer aggregation (count_distinct). For the other simple aggregates, sqlglot makes the operand an exp.Distinct, which the materialize path would turn into a hidden column with invalid SQL (`DISTINCT amount`) that fails at query time. Detect exp.Distinct in the simple-agg branch and clean-fail. Test added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A field expression that is a single double-quoted identifier (e.g. "legalEntityType", common for case-sensitive columns) was misclassified as a derived expression and rebuilt as a DOUBLE column, corrupting the introspected type. _as_bare_column now parses the expression and recognizes a single unqualified column reference (bare or quoted), so it overlays the introspected column instead. Test added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- converter: when an OSI dataset sets primary_key it now fully REPLACES the introspected primary key (clearing physical PK flags OSI omits), matching the "authoritative" contract; when unset, the introspected PK is kept - converter: fold join-pruning into the cross-model fixed-point loop so a join whose key column is dropped during validation is itself dropped+reported, and the column<->join cascade converges (no join referencing a removed column) - tests for both Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- converter: an aliased quoted bare column keeps its ORIGINAL quoted SQL (the unquoted name is used only for lookup/type) so it doesn't break on case-folding dialects like Snowflake - converter: an OSI primary_key with an unknown column no longer silently clears the physical PK — it reports and keeps the introspected PK - tests for both Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- cli: import-osi catches FileNotFoundError from parse_osi_path and exits with a clean message instead of a raw traceback (CodeRabbit) - converter: the cross-model column/join drop passes now collect-then-remove instead of iterating over list(...) snapshots, clearing 4 Sonar S7504 issues while keeping mutation-safe iteration - test: import-osi on a nonexistent path exits cleanly Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A derived field that redefines an existing physical column (e.g. status: LOWER(status)) was hard-typed DOUBLE, clobbering the introspected TEXT type. It now inherits the shadowed column's known type; a genuinely new derived column still defaults to DOUBLE (is_time still wins as TIMESTAMP). Test tightened. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…(DEV-1645) Codex review of PR #224: a plain-column ORDER BY on an unprojected multi-hop joined path (e.g. order by customers.regions.name) never pulled its join into scope — filters resolve such joins, ORDER BY did not — so the split fallback emitted a reference to a phantom alias, i.e. invalid SQL, defeating the point of the PR. _resolve_order_column now resolves the fallback qualifier to an in-scope table: the base model, or a join already pulled into scope (dimension / measure / filter), mapping a multi-hop dotted qualifier (customers.regions) to its canonical __ alias (customers__regions). When neither is in scope it raises the new UnresolvableOrderColumnError (SlayerError, ValueError) with an actionable message, rather than emitting SQL that fails at the database. Base-column unprojected ORDER BY (the reported flavor-A cases) still emits the split reference; the measure-CTE _base combined path stays a documented limitation (base-model qualifier is treated as in-scope, so it is not rejected). Adds tests: reject-when-unresolvable, resolve-via-__-alias-when-in- scope. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… column A derived field replacing an introspected column produced a fresh Column with primary_key=False, silently dropping the physical PK when OSI didn't restate primary_key. The replace path now carries the shadowed column's primary_key across the redefinition (OSI's explicit primary_key still overrides later). Test added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two new exception tests called self._gen() inside the pytest.raises block, so Sonar flagged two potentially-throwing invocations. Hoist gen + bundle out so the only call that can raise inside the block is the one under test — matches the existing pattern in test_reroot_aggregate_key.py.
…t mangling) Grow slayer/sql/naming.py into the single owner of every alias/result-key decision and close the DEV-1495/DEV-1692 defects it gates. - naming module: result_key() (dotted final keys), result_key_from_alias() (canonical dotted aliases), flat_name() (inner-stage __ binds); relocate encode_alias/decode_alias (delete dialects/_alias_mangle.py) and the DEV-1645 mixed-case quoting policy here; add assert_unique_cte_names() (per-WITH belt). - D3 (DEV-1495 bug 1): joined DERIVED dimensions project + return under the dotted key (orders.customers.rev_x2), not the flat orders.customers__revenue. generator._full_alias_for_slot and response_meta._slot_result_keys both route the three ROW key shapes through result_key so they can't drift; ORDER BY on a projected joined dim follows the same dotted alias. - Bare named-measure aliasing: a bare saved-measure ref surfaces under the measure NAME, not the formula-derived canonical. - DEV-1692: unique CTE names AND unique hidden time_shift value aliases in the typed pipeline (fixes both the duplicate-WITH error and the value collapse). - Remove the BigQuery scope-validator TypeError carve-out (verified zero residual); BigQuery output is now validated like every dialect. - Legacy flatteners delegate to flat_name (byte-identical). - Promote the 5 Stage-9 pins (2 named-measure, 2 DEV-1692, DEV-1495 bug 1) and un-pin the 2 OSI notebooks + DEV-1692 integration test. Full non-integration suite green (8197 passed); ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…cross-modelisolation-cte-renderer-on DEV-1708 Stage 4: cross-model/isolation CTE renderer on ScopeFrame + null-safe grain join-back
…ng-joins-on-the' of https://github.com/MotleyAI/slayer into egor/dev-1710-dev-1703-stage-6-firstlast-explicit-time-completion
…eRabbit fixes Address the PR #269 review round (Codex + CodeRabbit + Sonar): - CodeRabbit (Major): apply the DEV-1692 hidden-alias de-collision to _emit_consecutive_periods_ctes_for_planned too — two arithmetic-wrapped consecutive_periods slots shared the _consecutive_periods_inner placeholder and collided the same way time_shift did. Add a 2-consecutive_periods test. - CodeRabbit (nitpick): keyword args on the two maybe_validate_scopes calls. - Sonar S5778: hoist the ModelMeasure build out of the pytest.raises block so only one invocation can throw. - Sonar S3776: NOSONAR the CC-46 _emit_time_shift_ctes_for_planned (one cohesive per-slot emission; matches this file's 22 other S3776 suppressions). - Codex (case-sensitive CTE collision): deferred to DEV-1726 (dialect-aware fold); pre-existing + edge case, recorded in DECISIONS.md. - CodeRabbit (integration marker): replied invalid — file-backed SQLite tests are not marked integration by convention (siblings confirm). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…firstlast-explicit-time-completion DEV-1710 Stage 6: first/last explicit-time completion
- Sonar S7632: rewrite the NOSONAR(S3776) reason on _emit_time_shift_ctes_for_planned to be parenthesis-free — SonarQube greedily matches parens after NOSONAR, so a second (...) in the prose was parsed as malformed rule-keys. - Sonar S3776: NOSONAR _emit_consecutive_periods_ctes_for_planned (CC 26; modified by the round-1 consecutive_periods de-collision) with a paren-free reason. - Codex: seed the typed transform-chain allocator with the bare forms of every already-projected column alias, so a hidden transform alias (_time_shift_inner / _consecutive_periods_inner) can never shadow a real user column of that name (mirrors the legacy path seeding base_aliases). Add a regression test. Full non-integration suite green (8199 passed); ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…oach-expressions-crossing-joins-on-the' into egor/dev-1726-dialect-aware-case-folding-for-cte-name-collision-detection # Conflicts: # DECISIONS.md
Brings in Stage 4 (DEV-1708) + Stage 6 (DEV-1710). Conflict resolution: - DECISIONS.md: kept both sides, ordered chronologically (Stage 4/6 then Stage 9). - tests/test_dev1713_naming.py: two F6 tests combined a joined derived dim with a cross-model measure, which DEV-1708 now deliberately gates (NotImplementedError — a derived dim as cross-model shared grain is deferred to DEV-1495-b1). Adjusted both to use local measures only; the derived-dim dotted-key coverage is unaffected, cross-model dotted keys stay covered by test_cross_model_measure_key_still_dotted. Full support tracked in DEV-1728. Full non-integration suite green (8286 passed); ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… + composite lowering A LOCAL aggregate now isolates into a host-rooted _cm_* CTE when ANY explicit input crosses a join (D1/D2): source Column.sql (dotted / __ / sibling derived chains), Column.filter (pre-existing DEV-1503), positional args incl. the explicit first/last time arg, and kwargs (column refs, user template fragments, non-overridden model-default AggregationParam fragments). Closes DEV-1531 and DEV-1702-B1. Plan-time: new slayer/engine/aggregate_input_paths.py (compute_aggregate_input_join_paths — same parse/expand/scan pipeline as the filter scan; unparseable fragments contribute nothing, parity carve-out); trigger widened in stage_planner.py; disable_dev1503_isolation renamed disable_host_rooted_isolation (gates only the host-rooted half); _dispatch_filtered_local precondition widened; _local_agg_formula round-trip pinned for every input shape. Render-time: _build_first_last_base_select gains Law-2 materialization — every crossing source AND kwarg expression in the ranked outer scope projects as a _val_<n> whose body is the RESOLVED value (qualified + Column.type inner CAST), keyed by resolved text end-to-end (host path, Stage-4 CTE path, HAVING/composite consumers), closing the DEV-1527/DEV-1476 first/last kwarg deferral; same-sql-different-type aggregates keep distinct _vals. _validate_aggregate_kwarg_paths relaxed for local sources (structurally-crossing kwargs are now supported inputs). Composite crossing leaves isolate individually per interned AggregateKey slot; aggregate filters on isolated aggregates route to the combined-SELECT outer WHERE; host ROW filters (local + pathed) propagate into the host-rooted sub-plan (F4); F1 multiply-per-match pinned with executed 1:N DuckDB values, plus the headline sibling-protection value pins (a crossing measure's 1:N join no longer multiplies sibling aggregates). DEV-1502-era inline-shape tests, DEV-1527 F1 base-pull pins, and the 8 DEV-1531 strict-xfails rewritten/promoted to the isolated-CTE shape; DEV-1501 crossing-time-arg shapes updated. Implicit crossing default time column deferred to DEV-1729 (strict-xfail pin). Docs: cross-model-aggregates.md Strategy 3 widened, sql-generation.md, DECISIONS.md entry. Full non-integration suite: 8292 passed / 42 xfailed. Local integration (SQLite/DuckDB/Postgres): 220 passed / 2 xfailed. ruff clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Migrate `_emit_time_shift_ctes_for_planned` onto a per-slot `ScopeFrame`: every partition key and the shift-axis time expression enters through `scope.resolve()` (Law 1), and the shifted CTE's FROM is built from the scope's registered `join_paths`. Closes DEV-1474 and, as one uniform rule, three adjacent silent-broadcast defects the old joinless shifted CTE could not express. - ScopeFrame._anchor: a path'd ColumnSqlKey (derived column ON a joined model) expands at the `__`-path alias with is_root=False (DEV-1701 shape), instead of mis-anchoring at the host relation. - Remove the `7b.12` cross-model-partition raise; widen the auto-partition walk from ColumnKey-only to ColumnKey | ColumnSqlKey | TimeTruncKey ROW slots (the shift axis is excluded by slot id). The sjoin grain is now uniformly every projected dimension: joined columns (stores.name), derived columns (local upper(status) or joined stores.tier), and any secondary time dimension. - Joined time axis (stores.opened_at) resolves through the scope, pulling its join into the shifted CTE. - Lift the joined-filter guard: _build_shifted_cte_where_parts returns (parts, crossed_join_paths) and _guard_no_joined_refs is deleted; a joined-column ROW filter (query or Mode-A model filter) now pulls its join and re-aggregates over the same filtered population as _base. - Null-safe sjoin grain join-back (Codex F2): time axis + every partition pair use _null_safe_join_pair_sql, so a NULL dimension value or NULL time bucket keeps its prior-period value instead of dropping under plain `=`. Tests: new tests/test_dev1474_time_shift_cross_model_partition.py (SQL-shape + hand-computed DuckDB execution ground-truth); ScopeFrame path'd-ColumnSqlKey unit tests in test_scope.py; the DEV-1474 carrier_scope_matrix pin promoted from strict-xfail; 04_time QoQ notebook un-skipped (09_lightning_talk stays skipped for the unrelated DEV-1692/Stage-9 duplicate-CTE-name gap). Docs: formulas.md null-safe/every-dim grain wording + DECISIONS.md entry. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codex (major, confirmed by repro): a crossing template-fragment kwarg (user str value or model-default AggregationParam.sql fragment) triggered host-rooted isolation, but the CTE sub-render never registered the fragment's joins — _resolve_agg_inputs_via_scope only resolved column-valued kwargs, so the CTE emitted customers__regions.weight with no LEFT JOIN (ScopeLeakError / invalid SQL). Added sub-pass 3b: scan str kwargs + non-overridden model-default param fragments with the same _filter_join_paths pipeline as Column.filter and register the paths into the scope. Pinned with two new end-to-end generator tests (user fragment + model-default fragment) that previously failed with ScopeLeakError. Sonar S5778 (test_agg_render_spec.py): hoist _orders_model() out of the pytest.raises block so only _invoke() can throw. Sonar S9073 + CodeRabbit nitpick (test_dev1527_agg_kwargs.py): split the composite outer-WHERE assertion. Full non-integration suite: 8294 passed / 42 xfailed. Local integration (SQLite/DuckDB/Postgres): 220 passed / 2 xfailed. ruff clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…flat_name
Codex: two cross-model downstream-name paths in enrichment.py still called
`.replace(".", "__")` directly (lines 1152, 1485), bypassing the naming module.
Route both through `flat_name()` (byte-identical) so the single-owner flatten
contract holds across every legacy delegation site in this file.
Full non-integration suite green (8286 passed); ruff clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
AliasAllocator gains folds_case (comparison-only str.lower() folding; allocated names keep original case) and assert_unique_cte_names folds per dialect, so case-differing user aliases that both generate CTEs (shifted_Foo/shifted_foo) dedup instead of colliding after the backend folds. Policy lives in naming.py (CASE_FOLDING_SQLGLOT_DIALECTS + KNOWN_CASE_SENSITIVE_SQLGLOT_DIALECTS + dialect_folds_case); the single SQLGenerator._new_allocator factory threads it to all former construction sites (test-pinned as the only one). Fold set = every registry dialect except ClickHouse; unknown strings stay exact. Issue-text corrections (GoogleSQL docs + sqlglot + empirics): BigQuery FOLDS (CTE names are query aliases, case-insensitive); SQLite/DuckDB fold even quoted names. MySQL/T-SQL fold deliberately despite config-dependence (rename-only-safe). Belt folds regardless of quoting — over-strict by design on allocator-sanitized output. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…widen-law-3-isolation-trigger-to-any DEV-1709 Stage 5: widen Law-3 isolation trigger to any crossing input + composite lowering
- S9073: split the composite belt-message assertion into two asserts. - S7632: the time_shift emitter's suppression comment ended with the literal word NOSONAR, which Sonar parsed as a second malformed suppression tag — reworded to "sibling emitters' suppressions". - S3776: _emit_consecutive_periods_ctes_for_planned gains the same cohesive-emitter suppression its siblings carry (per the documented sibling-emitter rationale), instead of a scattering refactor. - CodeRabbit nitpick: the three identical _assert_valid_sql copies consolidate onto tests/_engine_helpers.py; tests/dialects/conftest.py and tests/test_sql_generator.py now import it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Brings in Stage 5 (DEV-1709, widened Law-3 isolation trigger). Only conflict was DECISIONS.md (append-vs-append) — kept both sides, Stage 5 entry ordered before the Stage 9 entries. Code auto-merged cleanly. Full non-integration suite green (8360 passed); ruff clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…oach-expressions-crossing-joins-on-the' into egor/dev-1711-dev-1703-stage-7-time_shift-ctes-on-scopeframe-cross-model # Conflicts: # DECISIONS.md
…/S9073 Address the CodeRabbit nitpick + SonarCloud new-code issues on the Stage-7 diff (quality gate was already green; these clear the attributed issues): - CodeRabbit + Sonar python:S3776 on _build_shifted_cte_where_parts: extract the per-filter rendering into _shifted_where_part. For a TYPED filter the join paths are now scanned STRUCTURALLY on the already-rendered AST via _joined_paths_in_sql (no text round-trip that could silently swallow a parse failure and drop a needed LEFT JOIN); a Mode-A text filter keeps the _filter_join_paths dual raw+expanded scan (DEV-1494). The extraction also drops the function's cognitive complexity back under 15. - Sonar python:S3776 on _emit_time_shift_ctes_for_planned: NOSONAR(S3776) with a justification matching the sibling _render_cross_model_cte — one cohesive per-slot CTE-emission unit whose tightly-coupled state would only scatter through many-argument helpers if split. - Sonar python:S1192: extract the duplicated "SELECT\n " CTE-head literal to the _SQL_SELECT_HEAD module constant (shifted + consecutive-periods sites). - Sonar python:S9073: split the two composite `assert X and Y` filter- preservation assertions into separate assertions. Full non-integration suite green (8317 passed); ruff clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…full-naming-module-result-keys-flat-names DEV-1713 Stage 9: full naming module (result keys, flat names, dialect alias mangling)
…oach-expressions-crossing-joins-on-the' into egor/dev-1726-dialect-aware-case-folding-for-cte-name-collision-detection # Conflicts: # DECISIONS.md
…itHub errors CI failed because the dbt MetricFlow demo's shallow clone hit a transient GitHub 503. The existing skip-guard only skipped when github.com:443 was unreachable at the socket level, so a 503 over a reachable socket hard-failed the integration run. - setup_metricflow.py: classify transient git stderr (5xx/429, DNS, resets) and retry the shallow fetch up to 3x with linear backoff; deterministic failures (bad pin SHA, missing git) still fail on the first attempt. - test_notebooks.py: skip a MetricFlowDemoError when GitHub is unreachable OR the failure is transient, reusing the same classifier; deterministic bootstrap errors still fail loudly. - tests/test_metricflow_setup.py: unit coverage for the classifier and the retry loop (success, retry-then-succeed, exhaust-and-raise, no-retry on deterministic errors). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Combine Stage 9's collision-safe CTE naming (cte_allocator threaded through _emit_time_shift_ctes_for_planned) with Stage 7's ScopeFrame-based cross-model partition resolution + null-safe sjoin. Conflict resolution: - _emit_time_shift_ctes_for_planned: keep Stage 9's cte_allocator param + collision-safe shifted_/sjoin_ CTE naming AND Stage 7's shifted ScopeFrame, partition-via-resolve, guard lift, and null-safe grain join-back; merged NOSONAR(S3776) justification covers both. - Merge interaction fix (BigQuery): Stage 9 dropped the BigQuery scope-validator carve-out and added .→___ alias mangling, exposing that Stage 7's null-safe _grain_eq round-tripped the dotted public alias through _null_safe_join_pair_sql's string parse — which re-splits `base.\`orders. created_at\`` into `\`base___orders\`.\`created_at\`` on BigQuery. Build the null-safe predicate from AST nodes directly (alias as one quoted=True identifier), matching the SELECT parts byte-for-byte on every dialect. - DECISIONS.md: keep both the Stage-7 entry and the Stage-9 entries. Full non-integration suite green (8383 passed); ruff clean.
- docs/concepts/formulas.md: align the inline self-join example with the null-safe grain — `ON base.month IS NOT DISTINCT FROM shifted.month AND ...` (the surrounding text already documents null-safe matching). - DECISIONS.md: compress the DEV-1711 Stage-7 entry to the decision + rationale per the file's "1–3 lines, not implementation detail" format; the mechanics live in the PR description, commits, and code comments. Also addresses (already fixed in an earlier commit) the CodeRabbit nitpick that _build_shifted_cte_where_parts rescanned rendered SQL: the typed-filter branch now collects join paths structurally via _joined_paths_in_sql in _shifted_where_part, with _filter_join_paths kept only for the Mode-A text branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…oach-expressions-crossing-joins-on-the' into egor/dev-1703-comprehensive-approach-expressions-crossing-joins-on-the
…time_shift-ctes-on-scopeframe-cross-model DEV-1711 Stage 7: time_shift CTEs on ScopeFrame (cross-model partitions)
…oach-expressions-crossing-joins-on-the' into egor/dev-1726-dialect-aware-case-folding-for-cte-name-collision-detection # Conflicts: # DECISIONS.md # slayer/sql/generator.py # tests/test_dev1713_naming.py
…der site CodeRabbit: placeholder_allocator now uses the `self._gen_allocator or self._new_allocator()` form like the other four comparable sites, so a future materializing resolve through the placeholder scope cannot restart the _val_<n> counter and collide with names minted elsewhere in the same generation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e-folding-for-cte-name-collision-detection DEV-1726: dialect-aware case folding for CTE name collision detection
|
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
4 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.
Integration PR for the DEV-1703 typed-pipeline branch into
egor/dev-1450-principled-redesign-of-syntax. The stage/parity sub-work (DEV-1704/1715/1716/1717) landed via its own PRs into this branch; the latest change is the origin/main merge below.Latest change — merge origin/main
Integrated 18
origin/maincommits into the typed pipeline:JoinFilterRulesetadaptation inquery_engine._policy_has_join_rules(incl. theand ruleset.joinsguard); a full-tree sweep found no stale old-shape (data_filters/ColumnFilterRule) usages.run_query→query— no branch references, no-op.stage_planner._declared_measures_from_query): rejectsGROUP BYonDataType.UNKNOWNdims, firing on the declared type beforebind_expr(so an opaque derived column is caught by type rather than tripping DEV-1410 cycle detection first).DECISIONS.md(typed-pipeline convention bullets remain documented underdocs/architecture/*).Conflicts resolved (2)
CLAUDE.md→ main's decluttered version.tests/test_sql_generator.py→ import union (AggRenderSpec+_wrap_cast_for_type).Post-merge test triage — real fixes, no new xfails
test_slayer_help_package_is_deleted→ removed the stale untrackedslayer/help/bytecode dir.test_opaque_column_rejected_as_dimension→ implemented the guard above.test_opaque_column_emits_no_cast_when_projected→ aligned the over-broad"CAST(" not inassertion to"AS UNKNOWN" not in(typed pipeline intentionally casts the INT count per DEV-1361).Verification: full non-integration suite green (7891 passed, 8 skipped, 98 xfailed — unchanged);
ruffclean.🤖 Generated with Claude Code