Repository navigation
Mode B as a SQL-compatible grammar: SQL forms in every position, casts, FILTER, quoted identifiers, NULL ordering - #509
Open
ZmeiGorynych wants to merge 18 commits into
Conversation
Proposal, design, tasks and delta specs: one pyparsing grammar for every Mode-B position, SQL forms, quoted identifiers and SQL string literals, CAST as the database's cast plus portable TRY_CAST, FILTER (WHERE) row semantics, literal-only {var} binding, NULLS FIRST/LAST, stored-text migration, and the importers' legacy parser retired.
New tests (fail until implementation): grammar parse, SQL forms in every position (SQLite/DuckDB executed, Tier-1 emission), CAST/TRY_CAST incl. Postgres 15/16/unknown emissions, cast typing, FILTER (WHERE c), variables as literals, interval/typed literals and clock keywords, NULLS FIRST/LAST, stored-text migration, importer dependency checks, parser benchmark, and Postgres / MySQL / SQL Server integration cases. Retargeted off deleted helpers / flipped to the new string rules: - test_dev1833_keyword_lexing: _rewrite_sql_like checks -> parse_expr; SQL string escapes; "p%" is an identifier; fused-Unicode LIKE operands parse whole - test_dev1740_case_rewrite: _rewrite_case_when -> parse_expr equivalences - test_dev1450fix_group2_correctness: double-quoted/backslash literals -> '' - test_dev1743_validators: stale _preprocess_colons comments trimmed - test_formula: _rewrite_funcstyle_aggregations / parse_formula -> parse_expr - test_functional_remedies: remedy asserted through the dbt dependency check - test_dev1576_heals: colon healing and arity at binding; ndigits rejection and function-style alias healing at binding (ruling 2026-10-21) - test_dev1737_scalar_specs, test_rank_direction: parse_formula -> parse_expr / binder - test_syntax: "hello" is an identifier - test_variables_planner: SQL quoting of substituted text (ruling 2026-10-21) - test_mode_a_variable_substitution: escape="python" tests deleted - parse_filter_expr -> parse_expr in test_translator, test_dev1475, test_dev1737_date_functions_parse, test_heal_date_parts, test_heal_functional_aggregations, test_mode_b_placeholder_parse, test_schema_drift_typed, test_sql_boolean_literal_filters - test_osi_converter: emitted formula validated by the new grammar
…ble-grammar-custom-parser-sql-forms
…les to core/enums.py Deterministic move (dr-refactor) of slayer/engine/syntax.py into core as the declared child core.syntax; ALL_TRANSFORMS / TIME_TRANSFORMS / TIMELESS_TRANSFORMS move from core/formula.py to core/enums.py. Approved harness edits H1-H7: model.c4 child + arrows, legacy baseline 8 -> 7, system P2 / P10, core and engine arc42 text, regenerated diagrams. Adds the pyparsing dependency.
… FILTER, quoted identifiers Replaces the ast.parse pipeline and its text pre-passes with one pyparsing grammar (parse_filter_expr deleted). Adds the Cast and TemporalLiteral nodes, CastKey (database cast of the author's type, TRY_CAST lowered per dialect, Postgres 16+ via the server profile), FILTER (WHERE c) (inputs masked at binding; star counts and first/last carry the filter on the key), ILIKE, IS [NOT] DISTINCT FROM, the quoted-identifier hint, the round() ndigits check, hidden row-level ORDER BY expressions, and canonical names for unnamed computed dimensions. Tests respelled for the SQL string rule (backslash escapes and double-quoted strings no longer literals): test_dev1901_param_binding, test_dev1934_formula_agg, test_quoted_aggregation_placeholders. Totality registries extended with CastKey / ilike: test_dev1747_reroot_visitor, test_dev1842_keys_prepend, test_dev1825_substitute_keys, test_dev1827_value_key_traversal, test_dev2013_expression_attribution, test_dev1753_last_four_scalars, test_dev1744_value_expr.
…rter parser
- OrderItem.nulls feeds OrderEntry.nulls; dialects emit a list of order terms
(build_order_terms) and T-SQL builds its NULL key over the resolved
projection expression.
- {var} placeholders bind as literals (bind_variables): strings, bare
number/boolean slots, whole IN lists; structure-changing values rejected;
date_range bounds keep their own literal substitution. ComputedDimension
.substituted removed.
- Stored-only migration SlayerModel 15->16, SlayerQuery 6->7, Memory 4->5
re-spells Python string literals as SQL literals and | as ||; stored
queries are migrated before the legacy time-literal repair.
- Importers use parse_expr and one namespace-aware dependency check;
slayer/core/formula.py (parse_formula, FieldSpec) deleted.
- Postgres TRY_CAST reads the server profile only when a plan casts with it;
the statement timeout probes the profile only where it depends on it.
- Literal dimensions render without a GROUP BY entry.
Test changes (mechanical): version pins bumped to 16/7/5 in
test_stored_upgrade_rank_orders, test_stored_upgrade_backing_sql,
test_dev1934_save_time_formula, test_model_access_caches,
test_model_access_tags, test_rank_direction_migration, test_memory_string_ids,
test_memories_storage, test_sqlite_memories_pk_migration; type narrowing in
test_syntax, test_dev1737_scalar_specs, test_dev1838_kernels,
test_dev1576_heals, test_rank_direction and the dev2101 suites; composite
asserts split and pytest.raises blocks reduced to the call under test in the
dev2101 suites, test_dev1576_heals, test_dev1737_scalar_specs,
test_osi_converter and test_quoted_aggregation_placeholders.
…, casts, literals, variables, NULL ordering
Changed tests: test_syntax::test_scalar_function_with_kwarg_rejected -> test_equals_inside_scalar_call_is_comparison; test_dev1737_date_functions_parse::TestKeywordArgsRejected -> TestEqualsInDateCallsIsComparison (asserts the error each spelling now raises).
Keyword-slot rejections suggest '==' or parentheses; scalar-call failures over a 'name = value' argument say scalar functions take positional arguments. Cmp records a single '=' spelling outside its identity. One help sentence in help.aggregations.
…MIN/MAX Extends the existing column wrap to aggregate-free row expressions whose row refs are not grouping values (it crashed with MaterialisationStageError). Docs: order-target table row and explicit nulls placement.
Changed tests: test_dev1733::test_composite_over_declared_aliases_still_rejected -> ..._is_an_expression (+ a sort test); test_dev1712::test_composite_order_string_now_accepted_at_construction and test_sql_generator::test_hidden_composite_order_accepted_at_input_validation assert alias / row composites are expressions.
Changed tests: test_cube_converter::test_calculated_measure_with_case_when_is_reported_not_emitted -> ..._is_emitted; test_distinct_dimension_values::test_filter_post_variable_substitution asserts the literal-values error; test_dev2101_agg_filter::_filtered_equals_restricted drops the row-count assertion (FILTER keeps empty buckets); test_dev2101_sql_forms::test_substring_and_position_equal_their_calls orders by id; test_dev2101_null_ordering::test_no_policy_emits_no_modifier -> test_no_policy_keeps_nulls_last; test_dev2101_null_ordering::test_tsql_orders_over_the_resolved_expression asserts the subquery column / inlined expression.
Two infix_notation tiers (arithmetic / concat, then predicates) replace the hand-built precedence ladder, and a keyword lookahead guards the keyword-led atoms; packrat was measured slower. Cold parses ~2.6x faster than the packrat ladder. Design D2 updated accordingly; basedpyright baseline shrinks.
…teger column Changed tests: _dev2101_fixtures % predicate/value use quantity (float % deferred to DEV-2120); test_parameterised_cast_is_postgres_numeric asserts the DECIMAL(10, 2) cast and value; test_cast_to_json_executes runs raw rows. Example models re-saved at SlayerModel version 16.
Contributor
|
Important Review skippedToo many files! This PR contains 219 files, which is 69 over the limit of 150. To get a review, reduce the PR to 150 files or fewer by splitting it into smaller PRs or changing its base branch. Upgrade to Team to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configuration
⛔ Files ignored due to path filters: 1
📒 Files selected for processing: 219
You can disable this status message by setting the
Comment |
…ty and regex cleanups
…NULL rows on T-SQL, Oracle, Redshift
Member
Author
|
@coderabbitai review |
Contributor
|
|
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
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.
Closes DEV-2101.
What this does
Mode B (measures, dimensions, filters, order targets, saved measure formulas) used to be parsed as Python with a stack of text pre-passes that rewrote SQL spellings into Python. It is now one SQL-compatible grammar (
slayer/core/syntax.py, built on pyparsing'sinfix_notation), shared by every position, so whatever is valid in a filter is valid in a measure, a dimension or an order target, and only typing decides whether it makes sense there. The importers (dbt, Cube, OSI) use the same parser; the oldparse_formula/FieldSpecparser is gone.New in every Mode-B position
CAST(x AS type)/x::type: the database's own cast of the type you write (DECIMAL(10,2)staysDECIMAL(10, 2)).TRY_CAST(x AS type): NULL for input the cast cannot convert, on every dialect (Postgres < 16 or unknown version gets a plainCAST).FILTER (WHERE c)on any aggregation:count(*) FILTER (WHERE status = 'paid'),last(amount) FILTER (WHERE amount > 0).BETWEEN,IS [NOT] DISTINCT FROM,IS [NOT] TRUE / FALSE,ILIKE,||,EXTRACT,POSITION … IN …,SUBSTRING … FROM … FOR …,CASEanywhere.INTERVAL '7 days'/INTERVAL '7' DAY/INTERVAL 7 DAY,DATE '…'/TIMESTAMP '…',CURRENT_DATE/CURRENT_TIMESTAMP.sum("order total"),customers."Region"."nulls": "first" | "last", honoured on every dialect including SQL Server.{ "source_model": "orders", "dimensions": ["status"], "measures": [ {"formula": "sum(amount)", "name": "rev"}, {"formula": "count(*) FILTER (WHERE amount BETWEEN 10 AND 50)", "name": "mid"}, {"formula": "sum(TRY_CAST(code AS INT))", "name": "codes"} ], "filters": ["created_at >= DATE '2024-01-01' AND customer_name <> 'O''Brien'"], "order": [{"column": "rev / mid", "direction": "desc", "nulls": "last"}] }Breaking changes
status = 'paid'. Stored models, saved queries and memories are upgraded on load (SlayerModel 16, SlayerQuery 7, Memory 5); queries sent by clients are not.''for an embedded quote, backslash is an ordinary character.a | b(use||) anda // b(usediv(a, b)/floor(a / b)) are rejected with a hint.{variable}placeholders bind literal values only: a placeholder in a name position (sum({col}),customers.{c},partition_by={x}) is rejected, so no value can change an expression's structure.slayer.core.formulais removed; useslayer.core.syntax.parse_expr.name = valueinside a scalar call is a comparison (iif(status = 'paid', 1, 0)); in aggregations and transforms it stays a keyword argument. Errors on the wrong spelling say which reading applied.Notes for review
main(packrat was measured slower and is off).%/modover a non-integer still fails on Postgres and SQL Server, as onmain; that is deferred to DEV-2120.(a = b AND a IS NOT NULL AND b IS NOT NULL) OR (a IS NULL AND b IS NULL)), soIS DISTINCT FROMkeeps rows where one side is NULL; the grain join-backs use the same form, hence the T-SQL / Oracle / Redshift golden SQL churn (results unchanged).openspec/changes/dev-2101-mode-b-as-a-sql-compatible-grammar-custom-parser-sql-forms/.