Skip to content

Commit 504c6ab

Browse files
author
ci bot
committed
Merge branch 'misc-fixes' into 'enterprise'
feat: skip view sampling on unsupported flavors + capture object_type See merge request dkinternal/testgen/dataops-testgen!543
2 parents c38f573 + 10f8a59 commit 504c6ab

28 files changed

Lines changed: 326 additions & 42 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/commands/queries/refresh_data_chars_query.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ class RefreshDataCharsSQL:
3030
"general_type",
3131
"column_type",
3232
"db_data_type",
33+
"object_type",
3334
"approx_record_ct",
3435
"record_ct",
3536
)
@@ -157,6 +158,7 @@ def get_staging_data_chars(self, data_chars: list[ColumnChars], run_date: dateti
157158
column.general_type,
158159
column.column_type,
159160
column.db_data_type,
161+
column.object_type,
160162
column.approx_record_ct,
161163
column.record_ct,
162164
]

testgen/commands/run_profiling.py

Lines changed: 38 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
write_to_app_db,
2020
)
2121
from testgen.common.database.column_chars import ColumnChars
22-
from testgen.common.database.database_service import ThreadedProgress
22+
from testgen.common.database.database_service import ThreadedProgress, get_flavor_service
2323
from testgen.common.job_context import job_context
2424
from testgen.common.mixpanel_service import MixpanelService
2525
from testgen.common.models import get_current_session, with_database_session
@@ -86,8 +86,9 @@ def run_profiling(
8686
if data_chars:
8787
sql_generator = ProfilingSQL(connection, table_group, profiling_run)
8888

89-
_run_column_profiling(sql_generator, data_chars)
90-
_run_frequency_analysis(sql_generator)
89+
sampling_params = _compute_sampling_params(sql_generator, data_chars)
90+
_run_column_profiling(sql_generator, data_chars, sampling_params)
91+
_run_frequency_analysis(sql_generator, sampling_params)
9192
_run_hygiene_issue_detection(sql_generator)
9293

9394
# if table_group.profile_do_pair_rules == "Y":
@@ -143,26 +144,40 @@ def _exclude_xde_columns(data_chars: list[ColumnChars], table_group_id: UUID) ->
143144
return filtered
144145

145146

146-
def _run_column_profiling(sql_generator: ProfilingSQL, data_chars: list[ColumnChars]) -> None:
147+
def _compute_sampling_params(
148+
sql_generator: ProfilingSQL, data_chars: list[ColumnChars]
149+
) -> dict[str, TableSampling]:
150+
table_group = sql_generator.table_group
151+
sampling_params: dict[str, TableSampling] = {}
152+
if not table_group.profile_use_sampling:
153+
return sampling_params
154+
155+
sampleable_types = get_flavor_service(sql_generator.flavor).sampleable_object_types
156+
for column in data_chars:
157+
if sampling_params.get(column.table_name):
158+
continue
159+
if sampleable_types is not None and column.object_type not in sampleable_types:
160+
continue
161+
result = calculate_sampling_params(
162+
table_name=column.table_name,
163+
record_count=column.record_ct,
164+
sample_percent_raw=table_group.profile_sample_percent,
165+
min_sample=table_group.profile_sample_min_count,
166+
)
167+
if result:
168+
sampling_params[column.table_name] = result
169+
return sampling_params
170+
171+
172+
def _run_column_profiling(
173+
sql_generator: ProfilingSQL, data_chars: list[ColumnChars], sampling_params: dict[str, TableSampling]
174+
) -> None:
147175
profiling_run = sql_generator.profiling_run
148176
profiling_run.set_progress("col_profiling", "Running")
149177
profiling_run.save()
150178
get_current_session().commit()
151179

152180
LOG.info(f"Running column profiling queries: {len(data_chars)}")
153-
table_group = sql_generator.table_group
154-
sampling_params: dict[str, TableSampling] = {}
155-
if table_group.profile_use_sampling:
156-
for column in data_chars:
157-
if not sampling_params.get(column.table_name):
158-
result = calculate_sampling_params(
159-
table_name=column.table_name,
160-
record_count=column.record_ct,
161-
sample_percent_raw=table_group.profile_sample_percent,
162-
min_sample=table_group.profile_sample_min_count,
163-
)
164-
if result:
165-
sampling_params[column.table_name] = result
166181

167182
def update_column_progress(progress: ThreadedProgress) -> None:
168183
profiling_run.set_progress(
@@ -218,7 +233,7 @@ def update_column_progress(progress: ThreadedProgress) -> None:
218233
)
219234

220235

221-
def _run_frequency_analysis(sql_generator: ProfilingSQL) -> None:
236+
def _run_frequency_analysis(sql_generator: ProfilingSQL, sampling_params: dict[str, TableSampling]) -> None:
222237
profiling_run = sql_generator.profiling_run
223238
profiling_run.set_progress("freq_analysis", "Running")
224239
profiling_run.save()
@@ -227,7 +242,7 @@ def _run_frequency_analysis(sql_generator: ProfilingSQL) -> None:
227242
error_data = None
228243
try:
229244
LOG.info("Selecting columns for frequency analysis")
230-
frequency_columns = fetch_dict_from_db(*sql_generator.get_frequency_analysis_columns())
245+
frequency_columns = [ColumnChars(**column) for column in fetch_dict_from_db(*sql_generator.get_frequency_analysis_columns())]
231246

232247
if frequency_columns:
233248
LOG.info(f"Running frequency analysis queries: {len(frequency_columns)}")
@@ -240,7 +255,10 @@ def update_frequency_progress(progress: ThreadedProgress) -> None:
240255
get_current_session().commit()
241256

242257
frequency_results, result_columns, error_data = fetch_from_db_threaded(
243-
[sql_generator.run_frequency_analysis(ColumnChars(**column)) for column in frequency_columns],
258+
[
259+
sql_generator.run_frequency_analysis(column, sampling_params.get(column.table_name))
260+
for column in frequency_columns
261+
],
244262
use_target_db=True,
245263
max_threads=sql_generator.connection.max_threads,
246264
progress_callback=update_frequency_progress,

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/database/column_chars.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,13 @@
11
import dataclasses
2+
from enum import StrEnum
3+
4+
5+
class ObjectType(StrEnum):
6+
TABLE = "TABLE"
7+
VIEW = "VIEW"
8+
MATERIALIZED_VIEW = "MATERIALIZED_VIEW"
9+
EXTERNAL = "EXTERNAL"
10+
OTHER = "OTHER"
211

312

413
@dataclasses.dataclass
@@ -11,6 +20,7 @@ class ColumnChars:
1120
column_type: str | None = None
1221
db_data_type: str | None = None
1322
is_decimal: bool = False
23+
object_type: ObjectType | None = None
1424
approx_record_ct: int | None = None
1525
# This should not default to 0 since we don't always retrieve actual row counts
1626
# UI relies on the null value to know that the approx_record_ct should be displayed instead

testgen/common/database/flavor/flavor_service.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from sqlalchemy import create_engine as sqlalchemy_create_engine
77
from sqlalchemy.engine.base import Engine
88

9-
from testgen.common.database.column_chars import ColumnChars
9+
from testgen.common.database.column_chars import ColumnChars, ObjectType
1010
from testgen.common.encrypt import DecryptText
1111

1212
SQLFlavor = Literal["redshift", "redshift_spectrum", "snowflake", "mssql", "postgresql", "databricks", "bigquery", "oracle", "sap_hana", "salesforce_data360"]
@@ -116,6 +116,7 @@ def row_limit_clauses(self, n: int) -> tuple[str, str]:
116116

117117
qualifies_table_refs_with_schema = True
118118
metadata_via_api = False
119+
sampleable_object_types: frozenset[ObjectType] | None = None
119120

120121
def get_schema_columns(self, _params: ResolvedConnectionParams, _schema: str) -> list[ColumnChars] | None:
121122
"""Return column metadata without querying information_schema.

testgen/common/database/flavor/mssql_flavor_service.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
from sqlalchemy.engine import URL
22

33
from testgen import settings
4+
from testgen.common.database.column_chars import ObjectType
45
from testgen.common.database.flavor.flavor_service import FlavorService, ResolvedConnectionParams
56

67

@@ -10,6 +11,8 @@ class MssqlFlavorService(FlavorService):
1011
escaped_underscore = "[_]"
1112
row_limiting_clause = "top"
1213
url_scheme = "mssql+pyodbc"
14+
# TABLESAMPLE is rejected on views; SQL Server has no materialized views, so only base tables.
15+
sampleable_object_types = frozenset({ObjectType.TABLE})
1316

1417
def get_connection_string_from_fields(self, params: ResolvedConnectionParams) -> str:
1518
connection_url = URL.create(

testgen/common/database/flavor/postgresql_flavor_service.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from urllib.parse import quote_plus
22

3+
from testgen.common.database.column_chars import ObjectType
34
from testgen.common.database.flavor.flavor_service import ResolvedConnectionParams
45
from testgen.common.database.flavor.redshift_flavor_service import RedshiftFlavorService
56

@@ -8,6 +9,8 @@ class PostgresqlFlavorService(RedshiftFlavorService):
89

910
escaped_underscore = "\\_"
1011
url_scheme = "postgresql"
12+
# TABLESAMPLE applies only to tables and materialized views
13+
sampleable_object_types = frozenset({ObjectType.TABLE, ObjectType.MATERIALIZED_VIEW})
1114

1215
def get_connection_string_from_fields(self, params: ResolvedConnectionParams) -> str:
1316
if params.host.startswith("/"):

testgen/common/models/data_table.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
)
1818
from sqlalchemy.dialects import postgresql
1919

20+
from testgen.common.database.column_chars import ObjectType
2021
from testgen.common.models import get_current_session
2122
from testgen.common.models.data_column import DataColumnChars
2223
from testgen.common.models.entity import Entity
@@ -42,6 +43,7 @@ class TableProfilingOverview:
4243
table_groups_id: UUID
4344
schema_name: str | None
4445
table_name: str
46+
object_type: ObjectType | None
4547
record_ct: int | None
4648
column_ct: int | None
4749
dq_score_profiling: float | None
@@ -61,6 +63,7 @@ class DataTable(Entity):
6163
table_groups_id: UUID = Column(postgresql.UUID(as_uuid=True), ForeignKey("table_groups.id"))
6264
schema_name: str | None = Column(String)
6365
table_name: str = Column(String)
66+
object_type: ObjectType | None = Column(String)
6467
column_ct: int | None = Column(BigInteger)
6568
record_ct: int | None = Column(BigInteger)
6669
approx_record_ct: int | None = Column(BigInteger)
@@ -110,6 +113,7 @@ def get_profiling_overview(
110113
cls.table_groups_id,
111114
cls.schema_name,
112115
cls.table_name,
116+
cls.object_type,
113117
cls.record_ct,
114118
cls.column_ct,
115119
cls.dq_score_profiling,

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

0 commit comments

Comments
 (0)