Skip to content

Commit bc7449b

Browse files
authored
fixes #28364: tableCustomSQLQuery honors computePassedFailedRowCount (#28375)
* fix(data-quality): tableCustomSQLQuery honors computePassedFailedRowCount The SQLAlchemy override of result_with_failed_samples gated only on Strategy.ROWS and Failed status, dropping the parent mixin's consent check. Test cases with computePassedFailedRowCount=false still had failed-row samples attached, leaking the data the flag was meant to suppress. Restore the consent gate alongside the strategy guard so the override matches every other validator's contract. Pre-existing samples written before this fix took effect remain in entity_extension; operators can clear them via the UI's "Delete" action or DELETE /api/v1/dataQuality/testCases/{id}/failedRowsSample. * fix(data-quality): address review feedback on consent check + type annotation - Read computePassedFailedRowCount from result.testCase, matching the parent mixin's pattern. Same object in practice, but consistent with FailedSampleValidatorMixin.result_with_failed_samples. - Annotate get_inspection_query return as Optional[str]. The implicit return inferred from get_test_case_param_value was too broad to assign to the Optional[str] field on TestCaseResultResponse. * fix checkstyle * test(data-quality): opt-in to failed-row samples on custom_sql_test The custom_sql_test param previously relied on the tableCustomSQLQuery override that bypassed computePassedFailedRowCount and always returned failed samples for Strategy.ROWS. The hotfix restored the consent gate, so the test must explicitly set computePassedFailedRowCount=True like the 8 sibling entries in FAILING_TEST_PARAMS.
1 parent 272280d commit bc7449b

3 files changed

Lines changed: 163 additions & 16 deletions

File tree

ingestion/src/metadata/data_quality/validations/table/sqlalchemy/tableCustomSQLQuery.py

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -370,26 +370,31 @@ def _get_custom_sql_failed_rows(self) -> Tuple[List[str], List[List[Any]]]: # n
370370
return [], []
371371
return [str(col) for col in rows[0]._fields], [list(row) for row in rows]
372372

373-
def get_inspection_query(self):
373+
def get_inspection_query(self) -> Optional[str]: # noqa: UP045
374374
return self.get_test_case_param_value(
375375
self.test_case.parameterValues, # type: ignore
376376
"sqlExpression",
377377
str,
378378
)
379379

380380
def result_with_failed_samples(self, result: TestCaseResultResponse) -> None:
381-
"""Override: tableCustomSQLQuery uses ROWS strategy check instead of
382-
computePassedFailedRowCount, and sets validateColumns=False."""
383-
if result.testCaseResult.testCaseStatus == TestCaseStatus.Failed and self._get_strategy() == Strategy.ROWS:
384-
result.validateColumns = False
385-
try:
386-
result.failedRowsSample = self.fetch_failed_rows_sample()
387-
except Exception:
388-
logger.debug(traceback.format_exc())
389-
logger.error("Failed to fetch failed rows sample")
390-
391-
try:
392-
result.inspectionQuery = self.get_inspection_query()
393-
except Exception:
394-
logger.debug(traceback.format_exc())
395-
logger.error("Failed to get inspection query")
381+
"""Collect failed-row samples when consent is given and strategy is ROWS."""
382+
if not (
383+
getattr(result.testCase, "computePassedFailedRowCount", False)
384+
and result.testCaseResult.testCaseStatus == TestCaseStatus.Failed
385+
and self._get_strategy() == Strategy.ROWS
386+
):
387+
return
388+
389+
result.validateColumns = False
390+
try:
391+
result.failedRowsSample = self.fetch_failed_rows_sample()
392+
except Exception:
393+
logger.debug(traceback.format_exc())
394+
logger.error("Failed to fetch failed rows sample")
395+
396+
try:
397+
result.inspectionQuery = self.get_inspection_query()
398+
except Exception:
399+
logger.debug(traceback.format_exc())
400+
logger.error("Failed to get inspection query")

ingestion/tests/integration/data_quality/test_failed_row_samples.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ def assume_positive(self, df):
203203
TestCaseDefinition(
204204
name="custom_sql_test",
205205
testDefinitionName="tableCustomSQLQuery",
206+
computePassedFailedRowCount=True,
206207
parameterValues=[
207208
TestCaseParameterValue(
208209
name="sqlExpression",
Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
# Copyright 2025 Collate
2+
# Licensed under the Collate Community License, Version 1.0 (the "License");
3+
# you may not use this file except in compliance with the License.
4+
# You may obtain a copy of the License at
5+
# https://github.com/open-metadata/OpenMetadata/blob/main/ingestion/LICENSE
6+
# Unless required by applicable law or agreed to in writing, software
7+
# distributed under the License is distributed on an "AS IS" BASIS,
8+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
9+
# See the License for the specific language governing permissions and
10+
# limitations under the License.
11+
"""Tests for the Custom SQL Query failed-row sample consent gate."""
12+
13+
from datetime import datetime
14+
from unittest.mock import Mock
15+
from uuid import uuid4
16+
17+
import pytest
18+
19+
from metadata.data_quality.api.models import TestCaseResultResponse
20+
from metadata.data_quality.validations.table.base.tableCustomSQLQuery import Strategy
21+
from metadata.data_quality.validations.table.sqlalchemy.tableCustomSQLQuery import (
22+
TableCustomSQLQueryValidator,
23+
)
24+
from metadata.generated.schema.tests.basic import TestCaseResult, TestCaseStatus
25+
from metadata.generated.schema.tests.testCase import TestCase
26+
from metadata.generated.schema.type.entityReference import EntityReference
27+
28+
TEST_CASE_NAME = "test_custom_sql_query_consent"
29+
ENTITY_LINK = "<#E::table::service.db.users>"
30+
31+
32+
def _make_test_case(*, compute_passed_failed_row_count: bool) -> TestCase:
33+
return TestCase(
34+
name=TEST_CASE_NAME,
35+
entityLink=ENTITY_LINK,
36+
testSuite=EntityReference(id=uuid4(), type="TestSuite"),
37+
testDefinition=EntityReference(id=uuid4(), type="TestDefinition"),
38+
computePassedFailedRowCount=compute_passed_failed_row_count,
39+
)
40+
41+
42+
def _make_validator(*, compute_passed_failed_row_count: bool) -> TableCustomSQLQueryValidator:
43+
return TableCustomSQLQueryValidator(
44+
runner=Mock(),
45+
test_case=_make_test_case(compute_passed_failed_row_count=compute_passed_failed_row_count),
46+
execution_date=datetime.now(),
47+
)
48+
49+
50+
def _failed_response(validator: TableCustomSQLQueryValidator) -> TestCaseResultResponse:
51+
return TestCaseResultResponse(
52+
testCase=validator.test_case,
53+
testCaseResult=TestCaseResult(
54+
timestamp=int(datetime.now().timestamp() * 1000),
55+
testCaseStatus=TestCaseStatus.Failed,
56+
),
57+
)
58+
59+
60+
def _passing_response(validator: TableCustomSQLQueryValidator) -> TestCaseResultResponse:
61+
return TestCaseResultResponse(
62+
testCase=validator.test_case,
63+
testCaseResult=TestCaseResult(
64+
timestamp=int(datetime.now().timestamp() * 1000),
65+
testCaseStatus=TestCaseStatus.Success,
66+
),
67+
)
68+
69+
70+
def test_consent_off_does_not_collect_failed_samples(monkeypatch):
71+
"""computePassedFailedRowCount=False → no sample on failure."""
72+
validator = _make_validator(compute_passed_failed_row_count=False)
73+
monkeypatch.setattr(validator, "_get_strategy", lambda: Strategy.ROWS)
74+
75+
fetch_called = False
76+
77+
def _should_not_be_called():
78+
nonlocal fetch_called
79+
fetch_called = True
80+
return Mock()
81+
82+
monkeypatch.setattr(validator, "fetch_failed_rows_sample", _should_not_be_called)
83+
84+
response = _failed_response(validator)
85+
validator.result_with_failed_samples(response)
86+
87+
assert response.failedRowsSample is None
88+
assert response.inspectionQuery is None
89+
assert fetch_called is False
90+
91+
92+
def test_consent_on_collects_failed_samples(monkeypatch):
93+
"""computePassedFailedRowCount=True → sample attached on failure."""
94+
validator = _make_validator(compute_passed_failed_row_count=True)
95+
monkeypatch.setattr(validator, "_get_strategy", lambda: Strategy.ROWS)
96+
97+
sample_marker = Mock(name="failed_rows_sample")
98+
inspection_marker = "SELECT * FROM users WHERE 1=1"
99+
monkeypatch.setattr(validator, "fetch_failed_rows_sample", lambda: sample_marker)
100+
monkeypatch.setattr(validator, "get_inspection_query", lambda: inspection_marker)
101+
102+
response = _failed_response(validator)
103+
validator.result_with_failed_samples(response)
104+
105+
assert response.failedRowsSample is sample_marker
106+
assert response.inspectionQuery == inspection_marker
107+
assert response.validateColumns is False
108+
109+
110+
def test_consent_on_but_strategy_is_count_does_not_collect(monkeypatch):
111+
"""Strategy.COUNT → no sample regardless of consent."""
112+
validator = _make_validator(compute_passed_failed_row_count=True)
113+
monkeypatch.setattr(validator, "_get_strategy", lambda: Strategy.COUNT)
114+
monkeypatch.setattr(
115+
validator,
116+
"fetch_failed_rows_sample",
117+
lambda: pytest.fail("fetch should not be called for Strategy.COUNT"),
118+
)
119+
120+
response = _failed_response(validator)
121+
validator.result_with_failed_samples(response)
122+
123+
assert response.failedRowsSample is None
124+
assert response.inspectionQuery is None
125+
126+
127+
def test_passing_status_does_not_collect_even_with_consent(monkeypatch):
128+
"""Passing status → no sample regardless of consent."""
129+
validator = _make_validator(compute_passed_failed_row_count=True)
130+
monkeypatch.setattr(validator, "_get_strategy", lambda: Strategy.ROWS)
131+
monkeypatch.setattr(
132+
validator,
133+
"fetch_failed_rows_sample",
134+
lambda: pytest.fail("fetch should not be called when the test passed"),
135+
)
136+
137+
response = _passing_response(validator)
138+
validator.result_with_failed_samples(response)
139+
140+
assert response.failedRowsSample is None
141+
assert response.inspectionQuery is None

0 commit comments

Comments
 (0)