Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions crates/sqlbuild-rules-native/src/engine/tests/test_evaluation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 6 additions & 0 deletions crates/sqlbuild-rules-native/src/engine/tests/test_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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))
}
51 changes: 6 additions & 45 deletions crates/sqlbuild-rules-native/src/rules/_helpers/evaluation.rs
Original file line number Diff line number Diff line change
@@ -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,
};
Expand Down Expand Up @@ -182,7 +182,7 @@ fn evaluate_model_inner(request: ModelEvaluationRequest<'_>) -> Result<Vec<Fault
if let Some(rule) = metadata("SQBRCONTRACT105") {
explicit_output_types::evaluate(&parsed.query, parsed.model, rule, &faults);
}
evaluate_naming_rules(&parsed, selected, &faults);
evaluate_naming_rules(&parsed, config, selected, &faults);
evaluate_literal_rules(&parsed, selected, &faults);
evaluate_test_rules(TestRuleEvaluation {
parsed: &parsed,
Expand Down Expand Up @@ -1317,51 +1317,12 @@ fn contract_required(parsed: &ParsedModel<'_>, rule: &RuleMetadata, faults: &Fau

fn evaluate_naming_rules(
parsed: &ParsedModel<'_>,
config: &RulesConfig,
selected: &BTreeMap<String, &RuleMetadata>,
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));
Expand Down
1 change: 1 addition & 0 deletions crates/sqlbuild-rules-native/src/rules/_helpers/mod.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down
10 changes: 3 additions & 7 deletions src/sqlbuild/compiler/compile/_helpers/analysis/cte_facts.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
71 changes: 70 additions & 1 deletion src/sqlbuild/rule_engine/_helpers/engine/catalogue.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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")),
Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions src/sqlbuild/rule_engine/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Loading
Loading