From 77590c69ff50ed87e7db9207abe51a6d5332ff9b Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Mon, 2 Nov 2026 05:15:14 +0000 Subject: [PATCH] fix: Snowflake time and timestamp precision no longer fails source contracts Metadata reads report Snowflake time and timestamp types without their fractional-seconds precision, so an enforced contract declaring TIME(3), TIME(9) or TIMESTAMP_TZ(9) was compared with a bare TIME or TIMESTAMP_TZ and failed with S404. Snowflake type equality now treats an unstated temporal precision as matching any precision of the same type; two stated precisions must still agree and different temporal types still differ. --- .../src/type_system/constants.rs | 9 ++ .../src/type_system/main/mod.rs | 1 + .../src/type_system/main/same_type.rs | 41 +++++++++ .../src/type_system/tests.rs | 2 + .../src/type_system/tests/test_same_type.rs | 84 ++++++++++++++++++ .../src/type_system/tests/test_types.rs | 7 ++ .../bindings/_helpers/type_system/equality.rs | 3 +- .../build/snowflake_metadata/_test_types.py | 11 +++ .../build/snowflake_metadata/helpers.py | 51 +++++++++++ .../test_temporal_source_contracts.py | 87 +++++++++++++++++++ 10 files changed, 295 insertions(+), 1 deletion(-) create mode 100644 crates/sqlbuild-analysis/src/type_system/main/same_type.rs create mode 100644 crates/sqlbuild-analysis/src/type_system/tests/test_same_type.rs create mode 100644 tests/integration/src/sqlbuild/executor/build/snowflake_metadata/test_temporal_source_contracts.py diff --git a/crates/sqlbuild-analysis/src/type_system/constants.rs b/crates/sqlbuild-analysis/src/type_system/constants.rs index ef06dd32cb..e5d79cc893 100644 --- a/crates/sqlbuild-analysis/src/type_system/constants.rs +++ b/crates/sqlbuild-analysis/src/type_system/constants.rs @@ -43,3 +43,12 @@ pub(crate) const POLYGLOT_TYPE_NAME_ALIASES: &[(&str, &str)] = &[ pub(crate) const MAX_FUNCTION_CALL_DEPTH: usize = 512; /// Python's `str(0)`, the only integer text Python treats as false. pub(crate) const ZERO_TEXT: &str = "0"; +/// Snowflake time and timestamp types, word separators removed, whose precision metadata omits. +pub(crate) const SNOWFLAKE_PRECISION_TEMPORAL_TYPES: &[&str] = + &["TIME", "TIMESTAMPLTZ", "TIMESTAMPNTZ", "TIMESTAMPTZ"]; +/// The character that opens a type's parameter list. +pub(crate) const TYPE_PARAMETERS_OPEN: char = '('; +/// The word separator in Snowflake type names such as `TIMESTAMP_TZ`. +pub(crate) const TYPE_WORD_SEPARATOR: char = '_'; +/// The character that closes a type's parameter list. +pub(crate) const TYPE_PARAMETERS_CLOSE: char = ')'; diff --git a/crates/sqlbuild-analysis/src/type_system/main/mod.rs b/crates/sqlbuild-analysis/src/type_system/main/mod.rs index 85cc669961..7434eded53 100644 --- a/crates/sqlbuild-analysis/src/type_system/main/mod.rs +++ b/crates/sqlbuild-analysis/src/type_system/main/mod.rs @@ -1,2 +1,3 @@ pub mod normalize_type; pub mod polyglot_type_args; +pub mod same_type; diff --git a/crates/sqlbuild-analysis/src/type_system/main/same_type.rs b/crates/sqlbuild-analysis/src/type_system/main/same_type.rs new file mode 100644 index 0000000000..f4995d687c --- /dev/null +++ b/crates/sqlbuild-analysis/src/type_system/main/same_type.rs @@ -0,0 +1,41 @@ +//! `same_type`: equal normalized types, a Snowflake temporal precision left unstated matching any. + +use crate::type_system::_helpers::dialects::coerce_type_dialect; +use crate::type_system::constants::{ + SNOWFLAKE_PRECISION_TEMPORAL_TYPES, TYPE_PARAMETERS_CLOSE, TYPE_PARAMETERS_OPEN, + TYPE_WORD_SEPARATOR, +}; +use crate::type_system::models::{NormalizedType, TypeDialect}; + +/// Whether `left` and `right` name the same type under `dialect`, up to an unstated precision. +#[must_use] +pub fn same_type(left: &NormalizedType, right: &NormalizedType, dialect: &str) -> bool { + if left == right { + return true; + } + if coerce_type_dialect(dialect) != Some(TypeDialect::Snowflake) { + return false; + } + let (Some((left_base, left_precision)), Some((right_base, right_precision))) = ( + temporal_parts(&left.normalized_name), + temporal_parts(&right.normalized_name), + ) else { + return false; + }; + left_base == right_base + && (left_precision.is_none() + || right_precision.is_none() + || left_precision == right_precision) +} + +/// A Snowflake time or timestamp type's compact base name and its stated precision text. +fn temporal_parts(name: &str) -> Option<(String, Option<&str>)> { + let (base, parameters): (&str, Option<&str>) = match name.split_once(TYPE_PARAMETERS_OPEN) { + Some((base, rest)) => (base, rest.strip_suffix(TYPE_PARAMETERS_CLOSE)), + None => (name, None), + }; + let compact: String = base.trim().replace(TYPE_WORD_SEPARATOR, ""); + SNOWFLAKE_PRECISION_TEMPORAL_TYPES + .contains(&compact.as_str()) + .then(|| (compact, parameters.map(str::trim))) +} diff --git a/crates/sqlbuild-analysis/src/type_system/tests.rs b/crates/sqlbuild-analysis/src/type_system/tests.rs index e27bb932fd..c423d513fb 100644 --- a/crates/sqlbuild-analysis/src/type_system/tests.rs +++ b/crates/sqlbuild-analysis/src/type_system/tests.rs @@ -4,5 +4,7 @@ mod helpers; mod normalize_type; #[path = "tests/test_python_text.rs"] mod python_text; +#[path = "tests/test_same_type.rs"] +mod same_type; #[path = "tests/test_types.rs"] mod test_types; diff --git a/crates/sqlbuild-analysis/src/type_system/tests/test_same_type.rs b/crates/sqlbuild-analysis/src/type_system/tests/test_same_type.rs new file mode 100644 index 0000000000..c97bf25c73 --- /dev/null +++ b/crates/sqlbuild-analysis/src/type_system/tests/test_same_type.rs @@ -0,0 +1,84 @@ +use crate::type_system::main::normalize_type::normalize_type; +use crate::type_system::main::same_type::same_type; +use crate::type_system::tests::test_types::SameTypeTestCase; + +#[test] +fn given_two_types_when_comparing_then_an_unstated_snowflake_temporal_precision_matches() { + let test_cases = [ + SameTypeTestCase { + description: "a declared millisecond time matches metadata's precision-less time", + dialect: "snowflake", + types: ("TIME(3)", "TIME"), + expected_same: true, + }, + SameTypeTestCase { + description: "a declared nanosecond time matches metadata's precision-less time", + dialect: "snowflake", + types: ("TIME", "TIME(9)"), + expected_same: true, + }, + SameTypeTestCase { + description: "two stated time precisions must agree", + dialect: "snowflake", + types: ("TIME(3)", "TIME(9)"), + expected_same: false, + }, + SameTypeTestCase { + description: "a declared offset timestamp precision matches metadata's spelling", + dialect: "snowflake", + types: ("TIMESTAMP_TZ(9)", "TIMESTAMP_TZ"), + expected_same: true, + }, + SameTypeTestCase { + description: "two stated offset timestamp precisions must agree", + dialect: "snowflake", + types: ("TIMESTAMP_TZ(3)", "TIMESTAMP_TZ(9)"), + expected_same: false, + }, + SameTypeTestCase { + description: "local time zone timestamps with and without precision", + dialect: "snowflake", + types: ("TIMESTAMP_LTZ(6)", "TIMESTAMP_LTZ"), + expected_same: true, + }, + SameTypeTestCase { + description: "an offset timestamp is not a local time zone timestamp", + dialect: "snowflake", + types: ("TIMESTAMP_TZ", "TIMESTAMP_LTZ(9)"), + expected_same: false, + }, + SameTypeTestCase { + description: "time is not a timestamp", + dialect: "snowflake", + types: ("TIME(9)", "TIMESTAMP_NTZ"), + expected_same: false, + }, + SameTypeTestCase { + description: "other dialects keep exact comparison", + dialect: "duckdb", + types: ("TIME(3)", "TIME"), + expected_same: false, + }, + SameTypeTestCase { + description: "an unstated text length is still a different type", + dialect: "snowflake", + types: ("VARCHAR(10)", "VARCHAR(20)"), + expected_same: false, + }, + ]; + for test_case in test_cases { + let (left, right) = test_case.types; + let left = normalize_type(left, test_case.dialect) + .expect("the type normalizes") + .normalized; + let right = normalize_type(right, test_case.dialect) + .expect("the type normalizes") + .normalized; + assert_eq!( + same_type(&left, &right, test_case.dialect), + test_case.expected_same, + "{}", + test_case.description + ); + } +} diff --git a/crates/sqlbuild-analysis/src/type_system/tests/test_types.rs b/crates/sqlbuild-analysis/src/type_system/tests/test_types.rs index f731088453..2b9aeec95d 100644 --- a/crates/sqlbuild-analysis/src/type_system/tests/test_types.rs +++ b/crates/sqlbuild-analysis/src/type_system/tests/test_types.rs @@ -23,3 +23,10 @@ pub(super) struct BracketDepthTestCase { pub(super) depth: usize, pub(super) expected_native: bool, } + +pub(super) struct SameTypeTestCase { + pub(super) description: &'static str, + pub(super) dialect: &'static str, + pub(super) types: (&'static str, &'static str), + pub(super) expected_same: bool, +} diff --git a/crates/sqlbuild-python/src/bindings/_helpers/type_system/equality.rs b/crates/sqlbuild-python/src/bindings/_helpers/type_system/equality.rs index cc781383a5..125c688b3c 100644 --- a/crates/sqlbuild-python/src/bindings/_helpers/type_system/equality.rs +++ b/crates/sqlbuild-python/src/bindings/_helpers/type_system/equality.rs @@ -6,6 +6,7 @@ use std::sync::{Mutex, OnceLock, PoisonError}; use pyo3::prelude::{Bound, PyAny, PyAnyMethods, PyResult, Python}; use pyo3::types::PyString; use sqlbuild_analysis::type_system::main::normalize_type::normalize_type; +use sqlbuild_analysis::type_system::main::same_type::same_type; use sqlbuild_analysis::type_system::models::{NormalizedType, TypeNormalization}; use sqlbuild_host::entries::models::ExecutorHost; @@ -32,7 +33,7 @@ pub(crate) fn types_equal(dialect: &Bound<'_, PyAny>, left: &str, right: &str) - }; let left_type: NormalizedType = normalized(dialect, &name, left)?; let right_type: NormalizedType = normalized(dialect, &name, right)?; - Ok(left_type == right_type) + Ok(same_type(&left_type, &right_type, &name)) } fn normalized(dialect: &Bound<'_, PyAny>, name: &str, type_sql: &str) -> PyResult { diff --git a/tests/integration/src/sqlbuild/executor/build/snowflake_metadata/_test_types.py b/tests/integration/src/sqlbuild/executor/build/snowflake_metadata/_test_types.py index 136eec98e4..9f1aa4e71f 100644 --- a/tests/integration/src/sqlbuild/executor/build/snowflake_metadata/_test_types.py +++ b/tests/integration/src/sqlbuild/executor/build/snowflake_metadata/_test_types.py @@ -132,3 +132,14 @@ class QuotedConfiguredColumnTestCase: expected_exit_code: int expected_fragments: tuple[str, ...] = () expected_errors: tuple[str, ...] = () + + +@dataclass(frozen=True) +class TemporalSourceContractTestCase: + """An enforced source contract's temporal type and the column type the warehouse holds.""" + + description: str + declared_type: str + warehouse_type: str + expected_models: tuple[str, ...] + expected_error: str = "" diff --git a/tests/integration/src/sqlbuild/executor/build/snowflake_metadata/helpers.py b/tests/integration/src/sqlbuild/executor/build/snowflake_metadata/helpers.py index cd508ad9ed..5a402a41aa 100644 --- a/tests/integration/src/sqlbuild/executor/build/snowflake_metadata/helpers.py +++ b/tests/integration/src/sqlbuild/executor/build/snowflake_metadata/helpers.py @@ -98,6 +98,15 @@ _RETENTION_PATTERN: re.Pattern[str] = re.compile(r"DATA_RETENTION_TIME_IN_DAYS = (?P\d+)") _DEFAULT_COLUMNS: tuple[FakeColumn, ...] = (FakeColumn(name="ID", data_type="NUMBER"),) _STATUS_DESCRIPTION: tuple[tuple[str], ...] = (("status",),) +_SHIFTS_PROJECT: str = ( + 'name = "catalog"\nadapter = "snowflake"\n\n[connection]\n' + 'account = "example"\nuser = "example"\ndatabase = "analytics"\nschema = "marts"\n\n' + '[defaults]\ndatabase = "analytics"\nschema = "marts"\nmaterialized = "table"\n' +) +_SHIFTS_MODEL: str = ( + "MODEL (description 'Shift report.');\n" + 'SELECT shift_id, starts_at FROM __source("raw_shifts")\n' +) type _Identity = tuple[str, str, str] @@ -840,3 +849,45 @@ def quoted_configured_column_files( 'SELECT "listingId", "eventAt" FROM __source("raw_listing_events")\n' ), } + + +def shifts_warehouse(warehouse_type: str) -> SimulatedSnowflakeWarehouse: + """Return a simulated warehouse holding a shifts source whose start column has a type.""" + + return SimulatedSnowflakeWarehouse( + relations=( + FakeRelation( + database="ANALYTICS", + schema="RAW", + name="SHIFTS", + columns=( + FakeColumn( + name="SHIFT_ID", data_type="NUMBER", numeric_precision=38, numeric_scale=0 + ), + FakeColumn(name="STARTS_AT", data_type=warehouse_type), + ), + ), + ) + ) + + +def write_shifts_project(*, project_dir: Path, declared_type: str) -> None: + """Write one model reading a shifts source whose enforced contract declares a start type.""" + + files: dict[str, str] = { + "sqlbuild_project.toml": _SHIFTS_PROJECT, + "sources/raw.yml": ( + "sources:\n - name: raw_shifts\n description: Shift rows.\n" + " database: analytics\n schema: raw\n table: shifts\n" + " contract: enforced\n columns:\n" + " - name: shift_id\n type: NUMBER(38,0)\n" + f" - name: starts_at\n type: {declared_type}\n" + ), + "models/shift_report.sql": _SHIFTS_MODEL, + } + relative: str + content: str + for relative, content in files.items(): + path: Path = project_dir / relative + path.parent.mkdir(parents=True, exist_ok=True) + _ = path.write_text(content, encoding="utf-8") diff --git a/tests/integration/src/sqlbuild/executor/build/snowflake_metadata/test_temporal_source_contracts.py b/tests/integration/src/sqlbuild/executor/build/snowflake_metadata/test_temporal_source_contracts.py new file mode 100644 index 0000000000..d48bcd1a5d --- /dev/null +++ b/tests/integration/src/sqlbuild/executor/build/snowflake_metadata/test_temporal_source_contracts.py @@ -0,0 +1,87 @@ +"""Enforced Snowflake source contracts on time and timestamp columns read through metadata.""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from sqlbuild._native import CompilePipelineResult +from tests.integration.src.sqlbuild.executor.build.snowflake_metadata._test_types import ( + TemporalSourceContractTestCase, +) +from tests.integration.src.sqlbuild.executor.build.snowflake_metadata.helpers import ( + plan_offline_snowflake_project, + shifts_warehouse, + write_shifts_project, +) + +_PLANNED: tuple[str, ...] = ("shift_report",) + + +@pytest.mark.parametrize( + "test_case", + [ + TemporalSourceContractTestCase("millisecond time", "TIME(3)", "TIME", _PLANNED), + TemporalSourceContractTestCase("nanosecond time", "TIME(9)", "TIME", _PLANNED), + TemporalSourceContractTestCase("time without precision", "TIME", "TIME", _PLANNED), + TemporalSourceContractTestCase( + "nanosecond offset timestamp", "TIMESTAMP_TZ(9)", "TIMESTAMP_TZ", _PLANNED + ), + TemporalSourceContractTestCase( + "millisecond local timestamp", "TIMESTAMP_LTZ(3)", "TIMESTAMP_LTZ", _PLANNED + ), + TemporalSourceContractTestCase( + "microsecond timestamp", "TIMESTAMP_NTZ(6)", "TIMESTAMP_NTZ", _PLANNED + ), + ], + ids=lambda case: case.description, +) +def test_given_enforced_temporal_source_contract_when_planning_then_precision_is_no_mismatch( + test_case: TemporalSourceContractTestCase, tmp_path: Path +) -> None: + write_shifts_project(project_dir=tmp_path, declared_type=test_case.declared_type) + + planned: CompilePipelineResult = plan_offline_snowflake_project( + project_dir=tmp_path, warehouse=shifts_warehouse(test_case.warehouse_type) + ) + + assert ( + tuple(entry.name for entry in planned.plan_output.model_entries) + == test_case.expected_models + ) + + +@pytest.mark.parametrize( + "test_case", + [ + TemporalSourceContractTestCase( + "a time contract on a timestamp column", + "TIME(3)", + "TIMESTAMP_NTZ", + (), + r"has type TIMESTAMP_NTZ but contract declares TIME\(3\)", + ), + TemporalSourceContractTestCase( + "an offset contract on a local timestamp column", + "TIMESTAMP_TZ(9)", + "TIMESTAMP_LTZ", + (), + r"has type TIMESTAMP_LTZ but contract declares TIMESTAMP_TZ\(9\)", + ), + ], + ids=lambda case: case.description, +) +def test_given_enforced_temporal_source_contract_when_planning_then_another_type_mismatches( + test_case: TemporalSourceContractTestCase, tmp_path: Path +) -> None: + write_shifts_project(project_dir=tmp_path, declared_type=test_case.declared_type) + + with pytest.raises(Exception, match=test_case.expected_error): + _ = plan_offline_snowflake_project( + project_dir=tmp_path, warehouse=shifts_warehouse(test_case.warehouse_type) + ) + + +if __name__ == "__main__": + pytest.main([__file__, "-vv"])