Skip to content

Commit 89bb4ec

Browse files
aarthy-dkclaude
andcommitted
fix(test-execution): render empty baseline/tolerance params as NULL
CAT measures substitute baseline params (counts, averages, standard deviations) and tolerances directly into SQL. When a test definition has an empty value, the substitution produced invalid SQL such as CAST( AS FLOAT) — erroring the test and poisoning the aggregated CAT batch (forcing a slow single rerun). Add a shared null_if_empty() helper and apply it to the numeric baseline params and tolerances in both the execution query builder and the source data lookup service. BASELINE_VALUE (used as a quoted literal, a number, or an IN-list) and Freshness_Trend's BASELINE_SUM (a quoted timestamp the template handles via NULLIF) are left as-is, since "NULL" is not valid SQL in those contexts. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 6c42249 commit 89bb4ec

4 files changed

Lines changed: 124 additions & 16 deletions

File tree

testgen/commands/queries/execute_tests_query.py

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
import pandas as pd
88

99
from testgen.common import read_template_sql_file
10-
from testgen.common.clean_sql import concat_columns
10+
from testgen.common.clean_sql import concat_columns, null_if_empty
1111
from testgen.common.database.database_service import (
1212
fetch_dict_from_db,
1313
get_flavor_service,
@@ -348,16 +348,27 @@ def _get_params(self, test_def: TestExecutionDef | None = None) -> dict:
348348
"CONCAT_COLUMNS": concat_columns(test_def.column_name, self.null_value) if test_def.column_name else "",
349349
"SKIP_ERRORS": test_def.skip_errors or 0,
350350
"CUSTOM_QUERY": test_def.custom_query,
351-
"BASELINE_CT": test_def.baseline_ct,
352-
"BASELINE_UNIQUE_CT": test_def.baseline_unique_ct,
351+
# Numeric baseline params feed division/arithmetic in CAT measures.
352+
# Render empty values as NULL so the measure evaluates to NULL instead of producing
353+
# invalid SQL like CAST( AS FLOAT). Cannot default to 0 — several are denominators.
354+
"BASELINE_CT": null_if_empty(test_def.baseline_ct),
355+
"BASELINE_UNIQUE_CT": null_if_empty(test_def.baseline_unique_ct),
356+
# BASELINE_VALUE is a quoted literal, an unquoted number, or an IN-list depending on
357+
# test type — no single empty-substitution is valid SQL, so it is templated as-is.
353358
"BASELINE_VALUE": test_def.baseline_value,
354-
"BASELINE_VALUE_CT": test_def.baseline_value_ct,
359+
"BASELINE_VALUE_CT": null_if_empty(test_def.baseline_value_ct),
355360
"THRESHOLD_VALUE": test_def.threshold_value or 0,
356-
"BASELINE_SUM": test_def.baseline_sum,
357-
"BASELINE_AVG": test_def.baseline_avg,
358-
"BASELINE_SD": test_def.baseline_sd,
359-
"LOWER_TOLERANCE": "NULL" if test_def.lower_tolerance in (None, "") else test_def.lower_tolerance,
360-
"UPPER_TOLERANCE": "NULL" if test_def.upper_tolerance in (None, "") else test_def.upper_tolerance,
361+
# Freshness_Trend uses BASELINE_SUM as a quoted timestamp (the template's
362+
# NULLIF('{...}', '') handles empty); Other tests use it as a numeric sum.
363+
"BASELINE_SUM": (
364+
test_def.baseline_sum
365+
if test_def.test_type == "Freshness_Trend"
366+
else null_if_empty(test_def.baseline_sum)
367+
),
368+
"BASELINE_AVG": null_if_empty(test_def.baseline_avg),
369+
"BASELINE_SD": null_if_empty(test_def.baseline_sd),
370+
"LOWER_TOLERANCE": null_if_empty(test_def.lower_tolerance),
371+
"UPPER_TOLERANCE": null_if_empty(test_def.upper_tolerance),
361372
# SUBSET_CONDITION should be replaced after CUSTOM_QUERY
362373
# since the latter may contain the former
363374
"SUBSET_CONDITION": test_def.subset_condition or "1=1",

testgen/common/clean_sql.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,19 @@ def quote_identifiers(identifiers: str, flavor: str) -> str:
5050
return ", ".join(quoted_values)
5151

5252

53+
def null_if_empty(value: object) -> object:
54+
"""Return the literal ``"NULL"`` when ``value`` is empty (``None`` or ``""``), else ``value``.
55+
56+
For numeric test parameters substituted into SQL templates (baseline counts,
57+
averages, standard deviations, tolerances). An empty substitution produces invalid SQL
58+
such as ``CAST( AS FLOAT)``; ``"NULL"`` makes the surrounding expression evaluate to NULL
59+
instead. A real ``0`` is preserved (``0 in (None, "")`` is ``False``). Not suitable for
60+
params used as quoted literals or IN-lists (e.g. ``BASELINE_VALUE``, ``BASELINE_SUM`` for
61+
Freshness_Trend), where ``"NULL"`` would not be valid SQL.
62+
"""
63+
return "NULL" if value in (None, "") else value
64+
65+
5366
def concat_columns(columns: str, null_value: str):
5467
# Prepares SQL expression to concatenate comma-separated column list
5568
expression = ""

testgen/common/source_data_service.py

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
import pandas as pd
1212
from sqlalchemy import text
1313

14-
from testgen.common.clean_sql import concat_columns
14+
from testgen.common.clean_sql import concat_columns, null_if_empty
1515
from testgen.common.database.database_service import get_flavor_service, replace_params
1616
from testgen.common.date_service import parse_fuzzy_date
1717
from testgen.common.models import get_current_session
@@ -207,12 +207,14 @@ def _build_query_standard(issue_data: dict, limit: int) -> str | None:
207207
if (parsed_test_date := parse_fuzzy_date(issue_data["test_date"]))
208208
else None,
209209
"CUSTOM_QUERY": test_definition.custom_query,
210+
# BASELINE_VALUE varies (quoted literal / number / IN-list) by test type — templated as-is.
211+
# The numeric baselines render empty as NULL to avoid invalid SQL like CAST( AS FLOAT).
210212
"BASELINE_VALUE": test_definition.baseline_value,
211-
"BASELINE_CT": test_definition.baseline_ct,
212-
"BASELINE_AVG": test_definition.baseline_avg,
213-
"BASELINE_SD": test_definition.baseline_sd,
214-
"LOWER_TOLERANCE": "NULL" if test_definition.lower_tolerance in (None, "") else test_definition.lower_tolerance,
215-
"UPPER_TOLERANCE": "NULL" if test_definition.upper_tolerance in (None, "") else test_definition.upper_tolerance,
213+
"BASELINE_CT": null_if_empty(test_definition.baseline_ct),
214+
"BASELINE_AVG": null_if_empty(test_definition.baseline_avg),
215+
"BASELINE_SD": null_if_empty(test_definition.baseline_sd),
216+
"LOWER_TOLERANCE": null_if_empty(test_definition.lower_tolerance),
217+
"UPPER_TOLERANCE": null_if_empty(test_definition.upper_tolerance),
216218
"THRESHOLD_VALUE": test_definition.threshold_value or 0,
217219
# SUBSET_CONDITION should be replaced after CUSTOM_QUERY
218220
# since the latter may contain the former

tests/unit/commands/queries/test_execute_tests_query.py

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
from datetime import UTC, datetime
2-
from unittest.mock import patch
2+
from unittest.mock import MagicMock, patch
33
from uuid import uuid4
44

55
import pytest
@@ -495,3 +495,85 @@ def test_aggregate_cat_tests_handles_null_max_query_chars():
495495
assert grouped_defs == [[td]]
496496

497497

498+
# --- TestExecutionSQL._get_params baseline guards ---
499+
500+
501+
def _make_params_execution_sql() -> TestExecutionSQL:
502+
"""Build a minimal TestExecutionSQL for exercising _get_params without a database."""
503+
instance = TestExecutionSQL.__new__(TestExecutionSQL)
504+
flavor_service = MagicMock()
505+
flavor_service.quote_character = '"'
506+
flavor_service.varchar_type = "VARCHAR"
507+
instance.flavor_service = flavor_service
508+
instance.flavor = "postgresql"
509+
instance.table_group = MagicMock(id=uuid4())
510+
instance.test_run = MagicMock(test_suite_id=uuid4(), id=uuid4())
511+
instance.run_date = datetime(2026, 1, 1, tzinfo=UTC)
512+
return instance
513+
514+
515+
def test_get_params_empty_baseline_counts_become_null():
516+
"""Empty baseline counts must render as NULL, not "", to avoid CAST( AS FLOAT) syntax errors."""
517+
instance = _make_params_execution_sql()
518+
params = instance._get_params(_make_td(test_type="Missing_Pct", baseline_ct="", baseline_value_ct=""))
519+
assert params["BASELINE_CT"] == "NULL"
520+
assert params["BASELINE_VALUE_CT"] == "NULL"
521+
522+
523+
def test_get_params_none_baseline_counts_become_null():
524+
instance = _make_params_execution_sql()
525+
params = instance._get_params(_make_td(test_type="Missing_Pct", baseline_ct=None, baseline_value_ct=None))
526+
assert params["BASELINE_CT"] == "NULL"
527+
assert params["BASELINE_VALUE_CT"] == "NULL"
528+
529+
530+
def test_get_params_populated_baseline_counts_pass_through():
531+
instance = _make_params_execution_sql()
532+
params = instance._get_params(_make_td(test_type="Missing_Pct", baseline_ct="1000", baseline_value_ct="950"))
533+
assert params["BASELINE_CT"] == "1000"
534+
assert params["BASELINE_VALUE_CT"] == "950"
535+
536+
537+
def test_get_params_zero_baseline_count_is_not_nulled():
538+
"""A real 0 is a meaningful value and must not be coerced to NULL."""
539+
instance = _make_params_execution_sql()
540+
params = instance._get_params(_make_td(test_type="Row_Ct_Pct", baseline_ct=0))
541+
assert params["BASELINE_CT"] == 0
542+
543+
544+
def test_get_params_empty_numeric_baselines_become_null():
545+
"""All numeric baseline params render NULL when empty."""
546+
instance = _make_params_execution_sql()
547+
params = instance._get_params(_make_td(
548+
test_type="Avg_Shift",
549+
baseline_unique_ct="", baseline_avg="", baseline_sd="", baseline_sum="",
550+
))
551+
assert params["BASELINE_UNIQUE_CT"] == "NULL"
552+
assert params["BASELINE_AVG"] == "NULL"
553+
assert params["BASELINE_SD"] == "NULL"
554+
# Non-Freshness test types null-guard BASELINE_SUM (numeric use in Incr_Avg_Shift)
555+
assert params["BASELINE_SUM"] == "NULL"
556+
557+
558+
def test_get_params_freshness_baseline_sum_kept_raw_when_empty():
559+
"""Freshness_Trend quotes BASELINE_SUM (NULLIF('', '') in template) — must stay empty, not 'NULL'."""
560+
instance = _make_params_execution_sql()
561+
params = instance._get_params(_make_td(test_type="Freshness_Trend", baseline_sum=""))
562+
assert params["BASELINE_SUM"] == ""
563+
564+
565+
def test_get_params_baseline_value_left_unguarded():
566+
"""BASELINE_VALUE has non-uniform usage (quoted/number/IN-list) — not coerced to NULL."""
567+
instance = _make_params_execution_sql()
568+
params = instance._get_params(_make_td(test_type="Constant", baseline_value=""))
569+
assert params["BASELINE_VALUE"] == ""
570+
571+
572+
def test_get_params_empty_tolerances_become_null():
573+
"""Tolerances use the same NULL guard."""
574+
instance = _make_params_execution_sql()
575+
params = instance._get_params(_make_td(test_type="Volume_Trend", lower_tolerance="", upper_tolerance=""))
576+
assert params["LOWER_TOLERANCE"] == "NULL"
577+
assert params["UPPER_TOLERANCE"] == "NULL"
578+
579+

0 commit comments

Comments
 (0)