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
Original file line number Diff line number Diff line change
Expand Up @@ -726,15 +726,21 @@ def is_invalid_expr(self, column_expression: str | COLUMN | SqlExpressionStr) ->
AND([NOT(IN(column_expression, literal_values)), IS_NOT_NULL(column_expression)])
)
elif not self.valid_values:
invalid_clauses.append(AND([LITERAL(True)]))
# No value is valid -> every row is invalid. Use a portable always-true predicate
Comment thread
Niels-b marked this conversation as resolved.
# (1 = 1) rather than a bare boolean literal: a standalone TRUE/FALSE is not a valid
# condition on data sources without a native boolean type in predicate position
# (e.g. pre-23ai Oracle, where it would render as `1` -> ORA-00920).
invalid_clauses.append(EQ(LITERAL(1), LITERAL(1)))
else:
invalid_clauses.append(NOT(IN(column_expression, literal_values)))
if isinstance(self.invalid_values, list):
literal_values = [LITERAL(value) for value in self.invalid_values if value is not None]
if None in self.invalid_values:
invalid_clauses.append(AND([IN(column_expression, literal_values), IS_NULL(column_expression)]))
elif not self.invalid_values:
invalid_clauses.append(AND([LITERAL(False)]))
# No value is explicitly invalid -> always-false predicate (1 = 0); see the
Comment thread
Niels-b marked this conversation as resolved.
# valid_values note above on why a bare boolean literal is not portable.
invalid_clauses.append(EQ(LITERAL(1), LITERAL(0)))
else:
invalid_clauses.append(IN(column_expression, literal_values))
if isinstance(self.valid_format, RegexFormat) and isinstance(self.valid_format.regex, str):
Expand Down
30 changes: 29 additions & 1 deletion soda-tests/src/helpers/data_source_test_helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -1542,6 +1542,29 @@ def quote_column(self, column_name: str) -> str:
"""For shorter notation in the tests, we can just point it to the dialect."""
return self.data_source_impl.sql_dialect.quote_column(column_name)

@staticmethod
def build_select_literal_query(data_source_type: str, expression: object) -> str:
"""Single source of truth for a standalone SELECT of a literal/constant in tests.
Most data sources allow a FROM-less SELECT; Oracle requires a FROM clause (FROM DUAL is
valid on every Oracle version). Static so callers without a helper instance (e.g. the
connection-test helper) can reuse it."""
if data_source_type == "oracle":
return f"SELECT {expression} FROM DUAL"
return f"SELECT {expression}"

def select_literal_query(self, expression: object) -> str:
"""A standalone query selecting a literal/constant, for tests needing a small ad-hoc
query (e.g. a failed_rows rows_tested_query)."""
return self.build_select_literal_query(self.data_source_impl.type_name, expression)

def supports_native_boolean(self) -> bool:
"""Whether a canonical BOOLEAN column round-trips through this data source's type system.
True for most sources (native BOOLEAN, or a bit/bool that maps back). Oracle < 23ai has
no SQL BOOLEAN and stores it as NUMBER, which can't be distinguished from any other
number on read-back, so the Oracle test helper overrides this. Test-only: the production
dialect gates SQL by server version directly (OracleSqlDialect._is_pre_23ai)."""
return True

def get_qualified_name_from_test_table(self, test_table: TestTable) -> str:
return self.data_source_impl.sql_dialect.qualify_dataset_name(
dataset_prefix=self.dataset_prefix,
Expand Down Expand Up @@ -1680,7 +1703,7 @@ def create_fat_rows_test_table_spec(
)

def get_column_mappings(self) -> dict[str, SodaDataTypeName]:
return {
mappings: dict[str, SodaDataTypeName] = {
"char_default": SodaDataTypeName.CHAR,
"char_w_length": SodaDataTypeName.CHAR,
"varchar_default": SodaDataTypeName.VARCHAR,
Expand All @@ -1705,6 +1728,11 @@ def get_column_mappings(self) -> dict[str, SodaDataTypeName]:
"time_default": SodaDataTypeName.TIME,
"boolean_default": SodaDataTypeName.BOOLEAN,
}
# Data sources without a native, round-trippable BOOLEAN (e.g. Oracle < 23ai stores
# it as NUMBER) report the column back as NUMERIC on read-back.
if not self.supports_native_boolean():
mappings["boolean_default"] = SodaDataTypeName.NUMERIC
return mappings

def map_table_type_to_short_string(self, table_type: TableType) -> str:
return {
Expand Down
11 changes: 9 additions & 2 deletions soda-tests/src/helpers/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from typing import Any, Optional

import pytest
from helpers.data_source_test_helper import DataSourceTestHelper
from soda_core.common.data_source_impl import DataSourceImpl
from soda_core.common.logs import Logs
from soda_core.common.yaml import DataSourceYamlSource
Expand Down Expand Up @@ -44,6 +45,11 @@
def create_data_source_impl(self, data_source_yaml_source: DataSourceYamlSource) -> DataSourceImpl:
return DataSourceImpl.from_yaml_source(data_source_yaml_source)

def _connection_test_query(self, data_source_impl: DataSourceImpl) -> str:
# A trivial query to verify the live connection, using the shared single source of
# truth for a literal SELECT (Oracle needs FROM DUAL; other sources accept a bare SELECT).
return DataSourceTestHelper.build_select_literal_query(data_source_impl.type_name, 1)

def test(self, monkeypatch: Optional[pytest.MonkeyPatch] = None):
if self.monkeypatches:
for module, mock in self.monkeypatches.items():
Expand All @@ -58,7 +64,7 @@
data_source_impl = self.create_data_source_impl(data_source_yaml)
assert not logs.has_errors
else:
with pytest.raises(Exception) as exc_info:

Check warning on line 67 in soda-tests/src/helpers/test_connection.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Specify a more specific exception type here.

See more on https://sonarcloud.io/project/issues?id=sodadata_soda-sql&issues=AZ9mdDstX74zXxzvgeye&open=AZ9mdDstX74zXxzvgeye&pullRequest=2795
self.create_data_source_impl(data_source_yaml)

assert self.expected_yaml_error in str(exc_info.value)
Expand All @@ -77,14 +83,15 @@
# do not try to query if connection failed
return

test_query = self._connection_test_query(data_source_impl)
if self.query_should_succeed:
data_source_impl.execute_query("SELECT 1")
data_source_impl.execute_query(test_query)
if logs.has_errors:
error_msg = "Query failed unexpectedly with error: " + logs.get_errors_str()
raise RuntimeError(error_msg)
else:
with pytest.raises(Exception) as exc_info:

Check warning on line 93 in soda-tests/src/helpers/test_connection.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Specify a more specific exception type here.

See more on https://sonarcloud.io/project/issues?id=sodadata_soda-sql&issues=AZ9mdDstX74zXxzvgeyg&open=AZ9mdDstX74zXxzvgeyg&pullRequest=2795
data_source_impl.execute_query("SELECT 1")
data_source_impl.execute_query(test_query)
assert self.expected_query_error in str(exc_info.value)
return
finally:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ def test_failed_rows_rows_tested_query_returns_zero(data_source_test_helper: Dat
FROM {test_table.qualified_name}
WHERE ({end_quoted} - {start_quoted}) > 5
rows_tested_query: |
SELECT 0
{data_source_test_helper.select_literal_query(0)}
""",
)

Expand Down Expand Up @@ -270,7 +270,7 @@ def test_failed_rows_rows_tested_query_returns_null(data_source_test_helper: Dat
FROM {test_table.qualified_name}
WHERE ({end_quoted} - {start_quoted}) > 5
rows_tested_query: |
SELECT NULL
{data_source_test_helper.select_literal_query("NULL")}
""",
)

Expand Down Expand Up @@ -313,7 +313,7 @@ def test_failed_rows_rows_tested_query_does_not_leak_to_other_checks(
FROM {test_table.qualified_name}
WHERE ({end_quoted} - {start_quoted}) > 5
rows_tested_query: |
SELECT 999
{data_source_test_helper.select_literal_query(999)}
""",
)

Expand Down Expand Up @@ -365,7 +365,7 @@ def test_failed_rows_rows_tested_query_percent_does_not_leak_to_other_checks(
FROM {test_table.qualified_name}
WHERE ({end_quoted} - {start_quoted}) > 5
rows_tested_query: |
SELECT 4
{data_source_test_helper.select_literal_query(4)}
threshold:
metric: percent
must_be_less_than: 10
Expand Down Expand Up @@ -420,15 +420,15 @@ def test_two_failed_rows_checks_resolve_to_distinct_metrics(
FROM {test_table.qualified_name}
WHERE ({end_quoted} - {start_quoted}) > 5
rows_tested_query: |
SELECT 100
{data_source_test_helper.select_literal_query(100)}
- failed_rows:
qualifier: gap_over_8
query: |
SELECT *
FROM {test_table.qualified_name}
WHERE ({end_quoted} - {start_quoted}) > 8
rows_tested_query: |
SELECT 200
{data_source_test_helper.select_literal_query(200)}
""",
)

Expand Down
36 changes: 36 additions & 0 deletions soda-tests/tests/integration/test_schema_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,42 @@
.build()
)

_boolean_schema_test_table_specification = (
TestTableSpecification.builder()
.table_purpose("schema_boolean")
.column_varchar("id")
.column_boolean("is_active")
.build()
)


def test_schema_boolean_data_type_oracle(data_source_test_helper: DataSourceTestHelper):
"""A contract declaring ``data_type: boolean`` must pass the schema check on Oracle, including
pre-23ai (18c/19c) where BOOLEAN has no native SQL type and is stored as NUMBER (which reads
back as ``number``). Guarded by OracleSqlDialect.data_type_names_are_same_or_synonym."""
if data_source_test_helper.data_source_impl.type_name != "oracle":
pytest.skip("Oracle-specific: pre-23ai Oracle stores BOOLEAN as NUMBER")

test_table = data_source_test_helper.ensure_test_table(_boolean_schema_test_table_specification)

data_source_test_helper.enable_soda_cloud_mock(
[
MockResponse(status_code=200, json_object={"fileId": "a81bc81b-dead-4e5d-abff-90865d1e13b1"}),
]
)

data_source_test_helper.assert_contract_pass(
test_table=test_table,
contract_yaml_str="""
checks:
- schema:
columns:
- name: id
- name: is_active
data_type: boolean
""",
)


@pytest.mark.parametrize("table_type", [TableType.TABLE, TableType.MATERIALIZED_VIEW, TableType.VIEW])
def test_schema(data_source_test_helper: DataSourceTestHelper, table_type: TableType):
Expand Down
4 changes: 4 additions & 0 deletions soda-tests/tests/integration/test_soda_data_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,10 @@ def convert_metadata_to_soda_data_type(metadata: ColumnMetadata) -> SodaDataType
"time_default": SodaDataTypeName.TIME,
"boolean_default": SodaDataTypeName.BOOLEAN,
}
# Data sources without a native, round-trippable BOOLEAN (e.g. Oracle < 23ai stores
# BOOLEAN as NUMBER) report the column back as NUMERIC on read-back.
if not data_source_test_helper.supports_native_boolean():
column_mappings["boolean_default"] = SodaDataTypeName.NUMERIC
for column in actual_columns:
column_name = column.column_name
print(
Expand Down
44 changes: 44 additions & 0 deletions soda-tests/tests/unit/test_sql_generation.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@
"""Same guard for COUNT."""
import pytest

with pytest.raises(TypeError):

Check warning on line 352 in soda-tests/tests/unit/test_sql_generation.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this exception test to have only one invocation possibly throwing an exception.

See more on https://sonarcloud.io/project/issues?id=sodadata_soda-sql&issues=AZ9quI2MQWqJIg-zjpos&open=AZ9quI2MQWqJIg-zjpos&pullRequest=2795
COUNT(STAR(), field_alias="x") # type: ignore[call-arg]


Expand Down Expand Up @@ -378,3 +378,47 @@
'current_database() AS "table_catalog"'
)
assert isinstance(LITERAL(None).AS("x"), ALIAS)


def _missing_and_validity(**overrides):
"""Build a MissingAndValidity with all fields defaulted to None, bypassing the YAML
constructor, so a single validity field can be exercised in isolation."""
from soda_core.contracts.impl.contract_verification_impl import MissingAndValidity

mv = MissingAndValidity.__new__(MissingAndValidity)
for field in (
"missing_values",
"missing_format",
"invalid_values",
"invalid_format",
"valid_values",
"valid_format",
"valid_min",
"valid_max",
"valid_length",
"valid_min_length",
"valid_max_length",
"valid_reference_data",
):
setattr(mv, field, None)
for key, value in overrides.items():
setattr(mv, key, value)
return mv


def test_empty_valid_values_renders_portable_always_true_predicate():
"""`valid_values: []` means every value is invalid. It must render as a portable
always-true predicate (`1 = 1`), not a bare boolean literal: a standalone TRUE/FALSE is not
a valid condition on data sources without a native boolean type in predicate position
(e.g. pre-23ai Oracle, where it would render as `1` and raise ORA-00920)."""
sql_dialect: SqlDialect = SqlDialect()
expr = _missing_and_validity(valid_values=[]).is_invalid_expr(COLUMN("c"))
assert sql_dialect.build_expression_sql(expr) == "1 = 1"


def test_empty_invalid_values_renders_portable_always_false_predicate():
"""`invalid_values: []` means nothing is explicitly invalid -> a portable always-false
predicate (`1 = 0`), for the same portability reason as the empty-valid_values case."""
sql_dialect: SqlDialect = SqlDialect()
expr = _missing_and_validity(invalid_values=[]).is_invalid_expr(COLUMN("c"))
assert sql_dialect.build_expression_sql(expr) == "1 = 0"
Loading