Repository navigation
test(compliance): add portability rungs M7 (window ORDER BY) and M8 (ratio-to-total) - #700
Draft
MrBlack1995 wants to merge 16 commits into
Draft
MrBlack1995 wants to merge 16 commits into
MrBlack1995 wants to merge 16 commits into
Conversation
One dataset with YEAR and DATE_TRUNC fields and an AVG(DATEDIFF(...)) metric, using the part-first DATEDIFF(part, start, end) order from expression_language.md. Adds one construct on top of the baseline. Co-authored-by: Isaac <no-reply@databricks.com>
expression_language.md makes the aggregate FILTER (WHERE ...) clause REQUIRED. Two variants (qualified and bare) express the same filtered aggregate as the existing CASE WHEN fixtures, so a difference between the two forms isolates handling of the clause itself. Co-authored-by: Isaac <no-reply@databricks.com>
Two datasets joined by a declared relationship and one model-scoped metric that aggregates across the join (SUM over one dataset divided by a COUNT DISTINCT over the other), referencing fields as dataset.field. Co-authored-by: Isaac <no-reply@databricks.com>
A model-scoped metric (avg_order_value) composed from two other model-scoped metrics referenced by name, per Metric Scoping in core-spec/spec.md. Co-authored-by: Isaac <no-reply@databricks.com>
Lists the M3 FILTER (WHERE ...) form and rungs M4-M6 in the README table, notes that M7 and M8 wait on the window ORDER BY NULL-placement default, and records that the fixtures live in this repository. Co-authored-by: Isaac <no-reply@databricks.com>
…t by kind and name Follow-ups from the review of apache#631: - The identity regression test looped over M0-M3 only; it now covers M0-M6, and the grader README says so. - M5's reference_form_primary said "qualified", but its only variant (the composite under test) references metric names; it is now "metric-name". - M6 named "fields and metric" with a comma-joined name, and M2 named "metrics" without a name, so a lookup by kind and name found neither. Every rung now lists elements_under_test, each with one kind (metric or field) and one name. Two new tests keep the manifest honest: every listed element is found by kind and name in each variant of its rung, and each primary variant uses the rung's primary reference form. Both fail on the previous manifest. Co-authored-by: Isaac <no-reply@databricks.com>
A metric expression cannot refer to other metrics (core-spec/spec.md, Metric Scoping; expression_language.md), and since apache#608 validate.py warns when one does. M5's composite, avg_order_value = total_amount / order_count, referenced the two metrics by name. The "Referenced as metric_name" row in the Metric Scoping table is how a query addresses a metric, not what an expression may contain. M5 now carries the composition the way a converter has to emit it when the source tool composes measures: the parts inlined, over qualified dataset.field references, SUM(sales.amount) / NULLIF(COUNT(sales.order_id), 0), NULL when there are no orders. The two base metrics stay alongside it. The fixture validates with no warnings on main's validator, and its primary reference form is qualified again. Co-authored-by: Isaac <no-reply@databricks.com>
…form means expression_language.md (Conditional Aggregations) states that FILTER (WHERE ...) is equivalent to a CASE with no ELSE for an aggregate that ignores NULL, and that an ELSE 0 "would count as a real row". M3's CASE fixtures wrote ELSE 0, so on rows that exist but do not match they give 0 where the FILTER variants added here give NULL; they were not the same aggregate the rung describes. Found by idk-arsh's execution check on apache/ossie-converters#24. Both CASE fixtures now read SUM(CASE WHEN ... THEN amount END), which also matches DAX (CALCULATE over no rows is BLANK). All four M3 fixtures validate on main. Co-authored-by: Isaac <no-reply@databricks.com>
…grade them Since apache#486 a model-scoped metric must reference fields as dataset.field, and since apache#608 validate.py warns on the bare variants of M0, M2 and M3 ("is not qualified"). Metric Scoping in core-spec/spec.md gives the bare form its place: a dataset-scoped metric (datasets[].metrics) references its own dataset's fields by single-part name. The four bare fixtures now carry their metrics there, and validate without warnings on main. The grader read only the model-root metrics, so a dataset-scoped metric was not graded at all, and the identity test still passed. It now collects datasets[].metrics too, keyed dataset.metric as the spec references them, and the identity test asserts that every metric in a fixture is graded. New tests: a dataset-scoped metric graded PORTABLE, dropped, and moved to the model (REWRITTEN, identity not preserved). All four fail without the grader change. Co-authored-by: Isaac <no-reply@databricks.com>
…pelling in M6 Follow-ups from the second review of apache#631: - The manifest named fields bare (amount_with_tax, order_year, order_month_start), but the grader reports a field as dataset.field, so matching manifest elements to findings found no result for M1 or M6. Fields are now named sales.<field>, and a variant that carries the rung's metrics as dataset-scoped metrics says so with metrics_in: sales (the grader reports those as sales.<metric>). The manifest test now uses the grader's own _collect keys instead of building a name set of its own, so the two cannot drift apart; it fails on the previous manifest. - M6 wrote DATEDIFF('day', ...), the Tableau spelling in the mapping table; the spec writes the part as a keyword, DATEDIFF(day, ...). Co-authored-by: Isaac <no-reply@databricks.com>
…ters#24 Issue #576 was transferred to apache/ossie-converters#24 with the converters; the ladder, grader and manifest still cited the old number, which now resolves to nothing. Co-authored-by: Isaac <no-reply@databricks.com>
…ored dialect means it sqlglot's base dialect sorts NULL as the smallest value, the opposite of the OSSIE_SQL_2026 default (apache#587). The grader therefore marked RANK() OVER (ORDER BY x DESC NULLS LAST), which changes the order, as a cosmetic rewrite of RANK() OVER (ORDER BY x DESC), and the equivalent DESC NULLS FIRST as needing review. OSSIE_SQL_2026 expressions are now compared with NULL as the largest value; ANSI_SQL ones, whose default is the engine's, only count as cosmetic when equal under both orderings. Co-authored-by: Isaac <no-reply@databricks.com>
…ratio-to-total) M7 covers window functions with an ORDER BY: LAG, a running SUM with a ROWS frame, and RANK() OVER (ORDER BY ... DESC) without NULLS FIRST/LAST, which relies on the OSSIE_SQL_2026 default from apache#587. That default is defined for OSSIE_SQL_2026 only, so M7 is authored in it. M8 covers ratio-to-total, x / SUM(x) OVER () and OVER (PARTITION BY ...), with no ORDER BY. Both are field expressions over sibling columns, as M1. The manifest, the identity test and both READMEs list M0-M8; the hold note on apache#587 is gone. All 15 fixtures validate on main (f28692a) with sqlglot 30.21.0. Co-authored-by: Isaac <no-reply@databricks.com>
…eclare M4 is the first rung with two datasets, and customer_id is declared by both sales and customers. The grader stripped every table qualifier before comparing trees, so a converter that re-pointed COUNT(DISTINCT customers.customer_id) to sales.customer_id, or dropped the qualifiers, was graded as a cosmetic rewrite, and _repair_by_expression paired such a renamed metric as expression-equivalent. _ambiguous_fields() reads the field names that more than one input dataset declares; for those the qualifier is kept, for all others it is stripped as before (M0-M3 have one dataset each, so their grades do not change). Three regression tests; two fail on the previous grader. Suggested-by: Jean-Baptiste Onofré <jbonofre@apache.org> Co-authored-by: Isaac <no-reply@databricks.com>
Co-authored-by: Isaac <no-reply@databricks.com>
This was referenced Nov 8, 2026
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
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.
Summary
Follow-up to #631: the two rungs held for the default NULL placement in window
ORDER BY, now that #587 has settled it. One commit fixes the grader so it reads that default the way the spec defines it, and one adds the rungs.ORDER BY:LAG(amount) OVER (ORDER BY order_date), a runningSUMwithROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW, andRANK() OVER (ORDER BY amount DESC)withoutNULLS FIRST/NULLS LAST, which relies on the #587 default. Authored inOSSIE_SQL_2026, the dialect that default applies tom7/m7_window_order.ossie.yamlORDER BY:amount / SUM(amount) OVER ()andamount / SUM(amount) OVER (PARTITION BY region), the "Percent of total" form inexpression_language.mdm8/m8_ratio_to_total.ossie.yamlBoth rungs test field expressions over sibling columns of their dataset, like M1, so they carry one fixture each.
manifest.yaml, the identity test and both READMEs now list M0-M8, and the note that M7 and M8 wait for #587 is gone.The grader read the omitted placement the opposite way from the spec. It normalizes expressions with sqlglot's base dialect, which sorts NULL as the smallest value. So for
RANK() OVER (ORDER BY amount DESC)it graded a converter returning... DESC NULLS LAST, which moves the NULL rows from first to last, as a cosmetic rewrite, and the equivalent... DESC NULLS FIRSTas needing review. It is the same reading #587 describes for the SQLGlot-based converters. The grader now picks the NULL ordering from the authored tag:OSSIE_SQL_2026: NULL is the largest value, as feat(core-spec): default NULL placement in window ORDER BY #587 defines.ANSI_SQL: the default is the engine's, so an omitted and an explicit placement count as cosmetic only when they agree under both orderings.Nothing else changes in how expressions are compared. The three new tests fail without the fix.
Validation
validation/validate.pyon main (f28692a) with sqlglot 30.21.0, no warnings. The validation suite passes (531 passed, 1 skipped).OSSIE_SQL_2026, and that inANSI_SQLneither explicit placement is proven equal to an omitted one.Related Issues
Refs apache/ossie-converters#24 (portability measurement). Follows #631; uses the default from #587 (closes #498).
Why field expressions.
expression_language.mdleaves where window functions are allowed to the query interface. That interface (#653,wide_table_semantics.mdSection 5.2) defers window functions in model-scoped metrics (E_DEFERRED_WINDOWED_METRIC) and puts no such limit on field expressions, so both rungs place their windows in fields.