From f971f4a2e9132472bc257613eeef3eba1318fb21 Mon Sep 17 00:00:00 2001 From: Kevin Longe <273747162+kvlonge@users.noreply.github.com> Date: Fri, 18 Sep 2026 09:23:15 +0100 Subject: [PATCH] feat: configure authoritative contract name types --- .../src/engine/tests/test_evaluation.rs | 75 +++++++++++++++ .../src/engine/tests/test_types.rs | 6 ++ .../src/rules/_helpers/contract_name_types.rs | 91 +++++++++++++++++++ .../src/rules/_helpers/evaluation.rs | 51 ++--------- .../src/rules/_helpers/mod.rs | 1 + .../compile/_helpers/analysis/cte_facts.py | 10 +- .../rule_engine/_helpers/engine/catalogue.py | 71 ++++++++++++++- src/sqlbuild/rule_engine/constants.py | 3 + .../_helpers/test_sql_analysis_columns.py | 32 ++++++- 9 files changed, 286 insertions(+), 54 deletions(-) create mode 100644 crates/sqlbuild-rules-native/src/rules/_helpers/contract_name_types.rs diff --git a/crates/sqlbuild-rules-native/src/engine/tests/test_evaluation.rs b/crates/sqlbuild-rules-native/src/engine/tests/test_evaluation.rs index 6235af48d1..6a34a2b1bd 100644 --- a/crates/sqlbuild-rules-native/src/engine/tests/test_evaluation.rs +++ b/crates/sqlbuild-rules-native/src/engine/tests/test_evaluation.rs @@ -544,6 +544,81 @@ fn given_enforced_contract_outputs_when_evaluating_explicit_type_rule_then_retur Ok(()) } +#[test] +fn given_contract_name_type_options_when_evaluating_then_accepts_configured_interfaces() +-> Result<(), String> { + let project_dir = TempDir::new().map_err(|error| error.to_string())?; + let test_cases = [ + test_types::ContractNameTypeOptionsTestCase { + description: "configured compatibility types are accepted", + config: json!({ + "select": ["SQBRCONTRACT102", "SQBRCONTRACT103", "SQBRCONTRACT104"], + "rule_options": { + "SQBRCONTRACT102": {"allow_numeric_indicators": true}, + "SQBRCONTRACT103": { + "allow_date_for_at": true, + "allow_numeric_epoch": true, + "allow_encoded_values": true + }, + "SQBRCONTRACT104": { + "allow_timestamps": true, + "allow_encoded_values": true + } + }, + "cache": {"enabled": false} + }), + expected_fault_codes: &[], + }, + test_types::ContractNameTypeOptionsTestCase { + description: "compatibility types remain findings by default", + config: json!({ + "select": ["SQBRCONTRACT102", "SQBRCONTRACT103", "SQBRCONTRACT104"], + "cache": {"enabled": false} + }), + expected_fault_codes: &[ + "SQBRCONTRACT102", + "SQBRCONTRACT103", + "SQBRCONTRACT103", + "SQBRCONTRACT103", + "SQBRCONTRACT104", + "SQBRCONTRACT104", + ], + }, + ]; + + for test_case in test_cases { + let mut request: Value = + serde_json::from_str(&helpers::request(&project_dir, &test_case.config)) + .map_err(|error| error.to_string())?; + request["models"][0]["query_sql"] = json!( + "SELECT is_ready, first_seen_at, source_timestamp, opaque_ts, raw_date, observed_date FROM orders" + ); + request["models"][0]["columns"] = json!([ + {"name": "is_ready", "type": "INTEGER"}, + {"name": "first_seen_at", "type": "DATE"}, + {"name": "source_timestamp", "type": "NUMBER(38,0)"}, + {"name": "opaque_ts", "type": "VARIANT"}, + {"name": "raw_date", "type": "VARCHAR"}, + {"name": "observed_date", "type": "TIMESTAMP_NTZ"} + ]); + + let result: Value = serde_json::from_str(&evaluate_json(&request.to_string())?) + .map_err(|error| error.to_string())?; + let fault_codes: Vec<&str> = result["faults"] + .as_array() + .into_iter() + .flatten() + .filter_map(|fault| fault["code"].as_str()) + .collect(); + assert_eq!( + fault_codes, test_case.expected_fault_codes, + "{}", + test_case.description + ); + } + Ok(()) +} + #[test] fn given_snowflake_parser_extensions_when_normalizing_then_preserves_source_positions() { let test_cases = [test_types::NormalizationTestCase { diff --git a/crates/sqlbuild-rules-native/src/engine/tests/test_types.rs b/crates/sqlbuild-rules-native/src/engine/tests/test_types.rs index ecc1ba3289..481290dac8 100644 --- a/crates/sqlbuild-rules-native/src/engine/tests/test_types.rs +++ b/crates/sqlbuild-rules-native/src/engine/tests/test_types.rs @@ -29,6 +29,12 @@ pub(crate) struct ExplicitOutputTypeTestCase { pub(crate) expected_message_fragment: &'static str, } +pub(crate) struct ContractNameTypeOptionsTestCase { + pub(crate) description: &'static str, + pub(crate) config: Value, + pub(crate) expected_fault_codes: &'static [&'static str], +} + pub(crate) struct TypedContractColumnTestCase { pub(crate) description: &'static str, pub(crate) contract: &'static str, diff --git a/crates/sqlbuild-rules-native/src/rules/_helpers/contract_name_types.rs b/crates/sqlbuild-rules-native/src/rules/_helpers/contract_name_types.rs new file mode 100644 index 0000000000..16f4a90c1d --- /dev/null +++ b/crates/sqlbuild-rules-native/src/rules/_helpers/contract_name_types.rs @@ -0,0 +1,91 @@ +use crate::constants::{BOOLEAN_TYPE, DATE_TYPE, TIMESTAMP_TYPE}; +use crate::models::{Column, RulesConfig}; + +const BOOLEAN_COLUMN_RULE: &str = "SQBRCONTRACT102"; +const TIMESTAMP_COLUMN_RULE: &str = "SQBRCONTRACT103"; +const DATE_COLUMN_RULE: &str = "SQBRCONTRACT104"; + +pub(crate) fn finding(column: &Column, config: &RulesConfig) -> Option<(&'static str, String)> { + let data_type = column.data_type.to_ascii_uppercase(); + if column.name.starts_with("is_") + || column.name.starts_with("has_") + || column.name.starts_with("can_") + { + let allowed_numeric = + rule_option_enabled(config, BOOLEAN_COLUMN_RULE, "allow_numeric_indicators") + && numeric_type(&data_type); + return (data_type != BOOLEAN_TYPE && !allowed_numeric).then(|| { + ( + BOOLEAN_COLUMN_RULE, + format!( + "column {:?} implies BOOLEAN but is typed {data_type}", + column.name + ), + ) + }); + } + if column.name.ends_with("_at") + || column.name.ends_with("_ts") + || column.name.ends_with("_timestamp") + { + let allowed_date = column.name.ends_with("_at") + && data_type == DATE_TYPE + && rule_option_enabled(config, TIMESTAMP_COLUMN_RULE, "allow_date_for_at"); + let allowed_epoch = numeric_type(&data_type) + && rule_option_enabled(config, TIMESTAMP_COLUMN_RULE, "allow_numeric_epoch"); + let allowed_encoded = encoded_temporal_type(&data_type) + && rule_option_enabled(config, TIMESTAMP_COLUMN_RULE, "allow_encoded_values"); + return (!data_type.contains(TIMESTAMP_TYPE) + && !allowed_date + && !allowed_epoch + && !allowed_encoded) + .then(|| { + ( + TIMESTAMP_COLUMN_RULE, + format!( + "column {:?} implies a timestamp but is typed {data_type}", + column.name + ), + ) + }); + } + if column.name.ends_with("_date") { + let allowed_timestamp = data_type.contains(TIMESTAMP_TYPE) + && rule_option_enabled(config, DATE_COLUMN_RULE, "allow_timestamps"); + let allowed_encoded = encoded_temporal_type(&data_type) + && rule_option_enabled(config, DATE_COLUMN_RULE, "allow_encoded_values"); + return (data_type != DATE_TYPE && !allowed_timestamp && !allowed_encoded).then(|| { + ( + DATE_COLUMN_RULE, + format!( + "column {:?} implies DATE but is typed {data_type}", + column.name + ), + ) + }); + } + None +} + +fn rule_option_enabled(config: &RulesConfig, code: &str, option: &str) -> bool { + config + .rule_options + .get(code) + .and_then(|options| options.get(option)) + .and_then(serde_json::Value::as_bool) + .unwrap_or(false) +} + +fn numeric_type(data_type: &str) -> bool { + [ + "BIGINT", "DECIMAL", "DOUBLE", "FLOAT", "INT", "NUMBER", "NUMERIC", "REAL", "SMALLINT", + ] + .iter() + .any(|prefix| data_type.starts_with(prefix)) +} + +fn encoded_temporal_type(data_type: &str) -> bool { + ["CHAR", "STRING", "TEXT", "VARCHAR", "VARIANT"] + .iter() + .any(|prefix| data_type.starts_with(prefix)) +} diff --git a/crates/sqlbuild-rules-native/src/rules/_helpers/evaluation.rs b/crates/sqlbuild-rules-native/src/rules/_helpers/evaluation.rs index f36161f5d7..7fa0866da2 100644 --- a/crates/sqlbuild-rules-native/src/rules/_helpers/evaluation.rs +++ b/crates/sqlbuild-rules-native/src/rules/_helpers/evaluation.rs @@ -1,11 +1,11 @@ use crate::constants::{ - BOOLEAN_TYPE, DATE_TYPE, DECLARATION_DOMAIN_COMPONENTS, ENFORCED_CONTRACT, NEGATION_OPERATOR, - REFERENCE_KIND, SOURCE_REFERENCE_KIND, TIMESTAMP_TYPE, VIEW_MATERIALIZATION, + DECLARATION_DOMAIN_COMPONENTS, ENFORCED_CONTRACT, NEGATION_OPERATOR, REFERENCE_KIND, + SOURCE_REFERENCE_KIND, VIEW_MATERIALIZATION, }; use crate::models::{Declaration, EvaluateRequest, Fault, Model, RuleMetadata, RulesConfig}; use crate::rules::_helpers::authored_literals::numeric_literal_tokens; use crate::rules::_helpers::domain_layout::folder_layer_details; -use crate::rules::_helpers::{explicit_output_types, typed_contract_columns}; +use crate::rules::_helpers::{contract_name_types, explicit_output_types, typed_contract_columns}; use crate::rules::models::{ FaultCollector, ModelEvaluationRequest, ProjectEvaluationRequest, ResolvedThresholdOverride, }; @@ -182,7 +182,7 @@ fn evaluate_model_inner(request: ModelEvaluationRequest<'_>) -> Result, rule: &RuleMetadata, faults: &Fau fn evaluate_naming_rules( parsed: &ParsedModel<'_>, + config: &RulesConfig, selected: &BTreeMap, faults: &FaultCollector, ) { for column in &parsed.model.columns { - let data_type = column.data_type.to_ascii_uppercase(); - let rule_and_message = if column.name.starts_with("is_") - || column.name.starts_with("has_") - || column.name.starts_with("can_") - { - (data_type != BOOLEAN_TYPE).then(|| { - ( - "SQBRCONTRACT102", - format!( - "column {:?} implies BOOLEAN but is typed {data_type}", - column.name - ), - ) - }) - } else if column.name.ends_with("_at") - || column.name.ends_with("_ts") - || column.name.ends_with("_timestamp") - { - (!data_type.contains(TIMESTAMP_TYPE)).then(|| { - ( - "SQBRCONTRACT103", - format!( - "column {:?} implies a timestamp but is typed {data_type}", - column.name - ), - ) - }) - } else if column.name.ends_with("_date") { - (data_type != DATE_TYPE).then(|| { - ( - "SQBRCONTRACT104", - format!( - "column {:?} implies DATE but is typed {data_type}", - column.name - ), - ) - }) - } else { - None - }; - if let Some((code, message)) = rule_and_message + if let Some((code, message)) = contract_name_types::finding(column, config) && let Some(rule) = selected.get(code) { faults.push(custom_fault!(parsed.model, rule, None, message, None)); diff --git a/crates/sqlbuild-rules-native/src/rules/_helpers/mod.rs b/crates/sqlbuild-rules-native/src/rules/_helpers/mod.rs index 964c38fe85..e67dc1a4bb 100644 --- a/crates/sqlbuild-rules-native/src/rules/_helpers/mod.rs +++ b/crates/sqlbuild-rules-native/src/rules/_helpers/mod.rs @@ -1,5 +1,6 @@ pub(crate) mod authored_literals; pub(crate) mod catalogue; +pub(crate) mod contract_name_types; pub(crate) mod domain_layout; pub(crate) mod evaluation; pub(crate) mod explicit_output_types; diff --git a/src/sqlbuild/compiler/compile/_helpers/analysis/cte_facts.py b/src/sqlbuild/compiler/compile/_helpers/analysis/cte_facts.py index d9b892311d..76726a3d3a 100644 --- a/src/sqlbuild/compiler/compile/_helpers/analysis/cte_facts.py +++ b/src/sqlbuild/compiler/compile/_helpers/analysis/cte_facts.py @@ -624,14 +624,10 @@ def _polyglot_top_level_ctes(root: Any) -> tuple[tuple[str, Any, bool], ...]: ) if not isinstance(raw_ctes, list): return () - cte_bodies: tuple[Any, ...] = tuple( - child - for child in root.children() - if str(getattr(child, "kind", "")) - in {_POLYGLOT_KIND_SELECT, *_POLYGLOT_SET_OPERATION_KINDS} - ) - if len(cte_bodies) != len(raw_ctes): + children: tuple[Any, ...] = tuple(root.children()) + if len(children) < len(raw_ctes): return () + cte_bodies: tuple[Any, ...] = children[-len(raw_ctes) :] ctes: list[tuple[str, Any, bool]] = [] for raw_cte, body in zip(raw_ctes, cte_bodies, strict=True): if not isinstance(raw_cte, dict): diff --git a/src/sqlbuild/rule_engine/_helpers/engine/catalogue.py b/src/sqlbuild/rule_engine/_helpers/engine/catalogue.py index 07b08759c4..9fc23e3aa6 100644 --- a/src/sqlbuild/rule_engine/_helpers/engine/catalogue.py +++ b/src/sqlbuild/rule_engine/_helpers/engine/catalogue.py @@ -9,13 +9,20 @@ from importlib.machinery import ModuleSpec from pathlib import Path from types import ModuleType +from typing import cast from sqlbuild.rule_engine._helpers.engine.definition import ( resolve_rule_signature, rule_from_value, ) from sqlbuild.rule_engine._helpers.engine.native import native_catalogue, native_selected_codes -from sqlbuild.rule_engine.constants import INIT_MODULE_NAME, RULE_DECORATOR_TOKEN +from sqlbuild.rule_engine.constants import ( + CONTRACT_BOOLEAN_NAME_CODE, + CONTRACT_DATE_NAME_CODE, + CONTRACT_TIMESTAMP_NAME_CODE, + INIT_MODULE_NAME, + RULE_DECORATOR_TOKEN, +) from sqlbuild.rule_engine.exceptions import RulesError from sqlbuild.rule_engine.models import ( Finding, @@ -65,6 +72,7 @@ def _builtins() -> tuple[Rule, ...]: message=str(item["message"]), remediation=str(item["remediation"]), check=_native_builtin, + options=_builtin_options(str(item["code"])), enabled_by_default=bool(item["enabled_by_default"]), subject=(RuleSubject.PROJECT if bool(item["project_wide"]) else RuleSubject.MODEL), guidance=_guidance(item.get("guidance")), @@ -73,6 +81,67 @@ def _builtins() -> tuple[Rule, ...]: ) +def _builtin_options(code: str) -> tuple[RuleOption[object], ...]: + if code == CONTRACT_BOOLEAN_NAME_CODE: + return ( + cast( + RuleOption[object], + RuleOption.boolean( + name="allow_numeric_indicators", + default=False, + description="Accept numeric is_/has_/can_ indicator columns.", + ), + ), + ) + if code == CONTRACT_TIMESTAMP_NAME_CODE: + return ( + cast( + RuleOption[object], + RuleOption.boolean( + name="allow_date_for_at", + default=False, + description="Accept DATE for columns ending in _at.", + ), + ), + cast( + RuleOption[object], + RuleOption.boolean( + name="allow_numeric_epoch", + default=False, + description="Accept numeric epoch values for timestamp-named columns.", + ), + ), + cast( + RuleOption[object], + RuleOption.boolean( + name="allow_encoded_values", + default=False, + description="Accept string or variant encoded timestamp values.", + ), + ), + ) + if code == CONTRACT_DATE_NAME_CODE: + return ( + cast( + RuleOption[object], + RuleOption.boolean( + name="allow_timestamps", + default=False, + description="Accept timestamp types for columns ending in _date.", + ), + ), + cast( + RuleOption[object], + RuleOption.boolean( + name="allow_encoded_values", + default=False, + description="Accept string or variant encoded date values.", + ), + ), + ) + return () + + def _guidance(value: object) -> RuleGuidance | None: if not isinstance(value, dict): return None diff --git a/src/sqlbuild/rule_engine/constants.py b/src/sqlbuild/rule_engine/constants.py index e57b80e2f4..7442e15643 100644 --- a/src/sqlbuild/rule_engine/constants.py +++ b/src/sqlbuild/rule_engine/constants.py @@ -26,6 +26,9 @@ DEPENDENCY_FUNCTIONS: frozenset[str] = frozenset({"__ref", "__source"}) CROSS_JOIN_KIND: str = "CROSS" CONTRACT_ENFORCED: str = "enforced" +CONTRACT_BOOLEAN_NAME_CODE: str = "SQBRCONTRACT102" +CONTRACT_TIMESTAMP_NAME_CODE: str = "SQBRCONTRACT103" +CONTRACT_DATE_NAME_CODE: str = "SQBRCONTRACT104" MATERIALIZED_VIEW: str = "view" REFERENCE_KIND_REF: str = "ref" LAYER_INT_CLEAN: str = "int_clean" diff --git a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_analysis_columns.py b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_analysis_columns.py index 2ff2d134bd..4cc09a9b9e 100644 --- a/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_analysis_columns.py +++ b/tests/unit/src/sqlbuild/compiler/compile/_helpers/test_sql_analysis_columns.py @@ -702,7 +702,34 @@ def test_given_ref_query_when_analyzing_columns_and_lineage_then_returns_compact ), ), expected_has_star=False, - ) + ), + PolyglotAnalysisTestCase( + description="preserves a final CTE cast after a values CTE on the AST fallback", + query_sql=( + "WITH labels(label) AS (VALUES ('web')), transformed AS (" + 'SELECT CAST(amount AS INTEGER) AS amount FROM __ref("orders")' + "), final AS (SELECT amount FROM transformed) SELECT amount FROM final" + ), + references=(CompileSqlReference(SqlReferenceKind.REF, "orders"),), + column_nullability_by_table={"orders": {"amount": InferredNullability.UNKNOWN}}, + column_types_by_table={"orders": {"amount": "VARCHAR"}}, + expected_columns=(InferredColumn(name="amount", type="INT"),), + expected_lineage_columns=( + CompiledLineageColumnFact( + output_column="amount", + upstream_columns=( + CompiledLineageSourceFact( + resource_type=CompiledResourceType.MODEL, + resource_name="orders", + column_name="amount", + ), + ), + transform_kind=ColumnTransformKind.DIRECT, + confidence=ColumnLineageConfidence.MEDIUM, + ), + ), + expected_has_star=False, + ), ], ids=lambda case: case.description, ) @@ -712,7 +739,10 @@ def test_given_annotated_projection_when_using_ast_fallback_then_lineage_is_pres result: PolyglotAnalysisResult = analyze_columns_and_lineage_with_polyglot( query_sql=test_case.query_sql, references=test_case.references, + column_nullability_by_table=test_case.column_nullability_by_table, + column_types_by_table=test_case.column_types_by_table, allow_compact_analysis=False, + recover_cte_facts=True, ) assert result.analysis_succeeded