All-datasource dialect groundwork: time/interval, percentile, typed timestamp literal, NaN filter#2787
Open
mivds wants to merge 18 commits into
Open
Conversation
…rcentile support flag Three new SqlDialect seams for the all-datasource profiling/MM groundwork (OBSL-1036), consumed by soda-extensions (OBSL-1037/1038): - literal_timestamp_typed(dt): typed timestamp literal for use inside timestamp arithmetic. Base TIMESTAMP 'YYYY-MM-DD HH:MM:SS' = v3 sql_time_filter_to_timestamp (data_source.py:1524-1529), incl. sub-second truncation — byte-parity with the MM bulk query builder's _timestamp_anchor_literal (bulk_query_builder.py:74-79). - sql_expr_is_not_nan(expr): NaN-exclusion predicate for float aggregates; base None = no filter needed (v3 spark_data_source.py:427-488 is the only engine family that filters NaN). - supports_percentile_within_group(): base True; consumers skip Q1/median/Q3 metrics when False (Synapse). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ct overrides SQL Server family overrides for the OBSL-1036 groundwork seams: - TIME_DELTA -> DATEDIFF(second, start, end) / seconds_per_bucket (v3 sqlserver_data_source.py:710-716 kept verbatim, incl. the truncating T-SQL int/int division). DATEDIFF counts crossed unit boundaries, so seconds math is required for correct bucketing. - ADD_INTERVAL -> DATEADD(UNIT, count, ts) with singular unit names (v3 :725-727, unit names v3 :706-708). - PERCENTILE_WITHIN_GROUP -> APPROX_PERCENTILE_DISC(p) WITHIN GROUP (ORDER BY expr) (v3 :336-337). T-SQL PERCENTILE_DISC is window-only; the aggregate form applies to SQL Server 2022+/Azure SQL/Fabric per https://learn.microsoft.com/en-us/sql/t-sql/functions/approx-percentile-disc-transact-sql - literal_timestamp_typed -> CAST('...' AS DATETIME2). Deviation from v3 (which rendered a bare string, :699-703): T-SQL has no TIMESTAMP '...' literal (TIMESTAMP = deprecated rowversion), and the explicit DATETIME2 cast keeps arithmetic operands typed. - Synapse: supports_percentile_within_group() -> False. The MS applies-to list excludes Synapse; v3 warned and returned a dummy 1 (synapse_data_source.py:56-58), v4 consumers skip the metric instead. - Fabric inherits everything from SqlServerSqlDialect - pinned in tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n percentile_disc Spark family (Databricks + inherited SparkDf/DatabricksHive) overrides for the OBSL-1036 groundwork seams: - TIME_DELTA -> FLOOR(DATEDIFF(SECOND, start, end) / seconds_per_bucket) (v3 spark_data_source.py:1158-1161 kept verbatim). 3-arg DATEDIFF is the documented TIMESTAMPDIFF synonym on Databricks DBR 10.4+ (https://docs.databricks.com/en/sql/language-manual/functions/datediff3.html) and OSS Spark 3.3+ (SPARK-38389). - ADD_INTERVAL -> TIMESTAMPADD(UNIT, count, ts). Deviation from v3 (which inherited the base interval-multiply form, data_source.py:1305-1307): Spark parses the plural-unit string literal INTERVAL '1 days' as a legacy CalendarInterval, so the explicit function form documented on Databricks DBR 10.4+ and OSS Spark 3.3+ (SPARK-38195) is used instead. - sql_expr_is_not_nan -> NOT ISNAN(expr) (v3 spark_data_source.py:427-488). - PERCENTILE_WITHIN_GROUP: deliberately NO override — Databricks supports the base percentile_disc(p) WITHIN GROUP (ORDER BY ...) aggregate since DBR 11.3 (https://docs.databricks.com/en/sql/language-manual/functions/percentile_disc.html) and OSS Spark since 3.4; pinned in tests. - SparkDf inheritance pinned in tests (incl. its to_utc_timestamp literal wrapping). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…overrides Trino + Athena overrides for the OBSL-1036 groundwork seams (Athena's dialect does not inherit Trino's, so the forms are implemented in both; Athena engine v3 functions are Trino's, https://docs.aws.amazon.com/athena/latest/ug/functions-env3.html): - TIME_DELTA -> cast(floor(date_diff('second', start, end) / secs.0) as int) (v3 trino_data_source.py:269-271 kept verbatim). Deviation for Athena: v3 athena (:269-273) lacked the int cast; it is required because floor() returns double on the Trino engine and date_add's value argument must be integer-typed (date_add(unit, bigint, ts) per https://trino.io/docs/current/functions/datetime.html). - ADD_INTERVAL -> date_add('<unit>', count, ts) (v3 trino :280-281). Deviations: lowercase singular unit names as documented (v3 trino passed uppercase enum names; v3 athena already used lowercase, :233-242), and Athena now gets the explicit date_add form instead of v3's inherited interval-multiply. - PERCENTILE_WITHIN_GROUP -> approx_percentile(expr, p) (v3 trino :245-246, athena :256-259) - the base WITHIN GROUP form is not Trino syntax. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Redshift changes for the OBSL-1036 groundwork seams: - PERCENTILE_WITHIN_GROUP -> APPROXIMATE PERCENTILE_DISC(p) WITHIN GROUP (ORDER BY expr) (v3 redshift_data_source.py:224-225). Redshift restricts exact PERCENTILE_DISC; the approximate aggregate is the documented form (https://docs.aws.amazon.com/redshift/latest/dg/r_APPROXIMATE_PERCENTILE_DISC.html). - TIME_DELTA / ADD_INTERVAL: deliberately NO overrides. The base epoch-floor TIME_DELTA is v3's redshift override byte-for-byte (v3 :287-290), and v3 redshift shipped the base interval-multiply ADD_INTERVAL unchanged. Redshift documents interval-literal arithmetic incl. plural units ('1 days', '52 weeks') and interval * numeric (https://docs.aws.amazon.com/redshift/latest/dg/r_interval_data_types.html, https://docs.aws.amazon.com/redshift/latest/dg/r_interval_literals.html). Both inherited renderings are pinned in unit tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
for more information, see https://pre-commit.ci
…dwork - sqlserver _build_time_delta_sql docstring: state the caller precondition behind the v3-verbatim truncating int/int division (deltas must be non-negative — the MM bulk query filters to ts >= anchor, so truncation == FLOOR; negative bucket indices would misbucket on T-SQL only), and note the DATEDIFF int overflow at ~68-year spans with DATEDIFF_BIG as the escape hatch (not switched: v3 parity). - athena: pin the count != 1 TIME_DELTA form and the weeks ADD_INTERVAL unit, matching the protection trino/sqlserver/databricks already have for their copied (not inherited) implementations. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Strip v3/soda-library file references, ticket numbers, and migration narration from the comments and docstrings this branch added; keep only the genuine engine constraints, concisely stated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This was referenced Jul 14, 2026
Contributor
|
Reviewed (aware it's draft). No blocking findings — this is the best-documented PR of the chain: every override cites its v3 source, every deviation carries a vendor-doc link, and the claimed pins all exist (checked: fabric-inherits-sqlserver, sparkdf-inherits-databricks, synapse Minor notes:
|
… T-SQL delta - literal_timestamp_typed: tz-aware datetimes are now converted to UTC before rendering (strftime alone dropped tzinfo and emitted local wall-clock against UTC-typed columns). Shared _typed_timestamp_str helper on the base dialect, reused by the SQL Server DATETIME2 override. Naive datetimes unchanged. - SQL Server _build_time_delta_sql: wrap the DATEDIFF/divide form in parens so it stays self-contained when embedded in larger arithmetic, like every other dialect's FLOOR/cast-wrapped form. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mivds
marked this pull request as ready for review
July 20, 2026 08:23
Contributor
Author
|
Addressed in cefd991:
Deferred as follow-ups: the ~6 |
… soda package) Mirrors the removal on #2777: soda-core neither generates nor consumes metric/ check identities. The helpers + pin test live in soda.observability.identity in the shared `soda` extensions package. Keeps this branch's tree consistent so the file never reaches main via the chain. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…me delta Fabric and Synapse inherit SqlServer._build_time_delta_sql, which now wraps the DATEDIFF/divide form in parens (#2787 review). Update the two inherited-form assertions to match; the literal_timestamp_typed inherited tests use naive datetimes and are unaffected. Co-Authored-By: Claude Opus 4.8 (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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
PR chain (soda-core)
metrics[]in scan-results payloadWhat & why (OBSL-1036)
Extends the v4 profiling and metric-monitoring engines to all data sources, porting the per-warehouse SQL forms from the v3 engine (soda-library) into the v4
SqlDialectlayer: time-bucket arithmetic (TIME_DELTA/ADD_INTERVAL), the percentile aggregate (PERCENTILE_WITHIN_GROUP), a typed timestamp literal, and a NaN-exclusion predicate. Every override cites its v3 source (file:line) in its docstring, per the existing convention.New base seams (soda-core
SqlDialect)literal_timestamp_typed(dt)— typed timestamp literal for use inside timestamp arithmetic. BaseTIMESTAMP 'YYYY-MM-DD HH:MM:SS'= v3sql_time_filter_to_timestamp(data_source.py:1524-1529), incl. sub-second truncation; byte-parity with the MM bulk builder's_timestamp_anchor_literal.sql_expr_is_not_nan(expr)— NaN-exclusion predicate for float aggregates; baseNone= no filter needed (v3 spark_data_source.py:427-488).supports_percentile_within_group()— baseTrue; consumers skip the Q1/median/Q3 metrics whenFalseinstead of rendering invalid SQL.These base seams are consumed by the downstream profiling and metric-monitoring engines.
Per-dialect forms
DATEDIFF(second, s, e) / n(v3 :710-716)DATEADD(UNIT, c, ts)(v3 :725-727)APPROX_PERCENTILE_DISC(p) WITHIN GROUP (...)(v3 :336-337)literal_timestamp_typed→CAST('...' AS DATETIME2)supports_percentile_within_group()→FalseFLOOR(DATEDIFF(SECOND, s, e) / n)(v3 spark :1158-1161)TIMESTAMPADD(UNIT, c, ts)PERCENTILE_DISC(p) WITHIN GROUPvalid since DBR 11.3 (pinned)sql_expr_is_not_nan→NOT ISNAN(expr)cast(floor(date_diff('second', s, e) / n.0) as int)(v3 :269-271)date_add('unit', c, ts)(v3 :280-281)approx_percentile(expr, p)(v3 :245-246)approx_percentile(expr, p)(v3 :256-259)APPROXIMATE PERCENTILE_DISC(p) WITHIN GROUP (...)(v3 :224-225)Deviations from v3 (⚠️ above), with doc citations
DATEDIFF(second,...)/nint division) while every other dialect FLOORs toward -inf. Equal only for non-negative deltas — guaranteed by the MM bulk query'sts >= anchorfilter; a consumer relying on negative bucket indices would misbucket on the T-SQL family only. Also noted:DATEDIFF(second,...)overflows at ~68-year spans (DATEDIFF_BIGis the escape hatch; not switched, v3 parity).literal_timestamp_typed: v3 rendered a bare string (:699-703). T-SQL has noTIMESTAMP '...'literal (TIMESTAMP= deprecated rowversion), so the seam rendersCAST('...' AS DATETIME2)(DATETIME2 docs).1(:56-58). v4 skips the metric via the support flag; Synapse is absent from APPROX_PERCENTILE_DISC's applies-to list.ts + INTERVAL '1 <unit>' * n; Spark parses the plural-unit string as a legacy CalendarInterval, so v4 usesTIMESTAMPADD, documented on Databricks DBR 10.4+ and OSS Spark 3.3+ (SPARK-38195). The 3-argDATEDIFFused for TIME_DELTA is the documented TIMESTAMPDIFF synonym (OSS: SPARK-38389).as intcast; it is required becausefloor()returnsdoubleon the Trino engine anddate_add's value argument is integer-typed. Athena engine v3 functions are Trino's.interval * numericare documented (interval types, interval literals);APPROXIMATE PERCENTILE_DISCper its docs.Tests
Per-package snapshot tests pin every rendered form (incl. inheritance-pinning tests for Synapse/Fabric/SparkDf and the deliberate no-override cases on Databricks/Redshift), plus base-seam unit tests in
soda-tests/tests/unit/test_sql_dialect_seams.py.soda-testsunit (1191 passed, 2 skipped), feature (13 passed) and integration (164 passed, 14 skipped) green, plus all touched-package unit suites.🤖 Generated with Claude Code