Fix a bug in that caused custom validations to always report anomalies

PiperOrigin-RevId: 671804127
tensorflow · Sep 6, 2024 · 3a64d24 · 3a64d24
1 parent 358631a
commit 3a64d24
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 8 deletions.
diff --git a/RELEASE.md b/RELEASE.md
@@ -20,6 +20,7 @@
 
 ## Bug Fixes and Other Changes
 
+*   Fix bug in implementation of custom validations.
 *   Depends on `tensorflow>=2.15,<2.16`.
 
 ## Known Issues

diff --git a/tensorflow_data_validation/anomalies/custom_validation.cc b/tensorflow_data_validation/anomalies/custom_validation.cc
@@ -92,8 +92,8 @@ absl::Status GetFeatureStatistics(
 }
 
 absl::Status MergeAnomalyInfos(const AnomalyInfo& anomaly_info,
-                         const std::string& key,
-                         AnomalyInfo* existing_anomaly_info) {
+                               const std::string& key,
+                               AnomalyInfo* existing_anomaly_info) {
   if (Path(anomaly_info.path()).Compare(Path(existing_anomaly_info->path())) !=
       0) {
     return absl::AlreadyExistsError(
@@ -242,13 +242,15 @@ absl::Status CustomValidateStatistics(
                 "Attempt to run query: ", validation.sql_expression(),
                 " failed with the following error: ",
                 query_result.status().ToString()));
+          } else if (!query_result.value()) {
+            // If the sql_expression evaluates to False, there is an anomaly.
+            TFDV_RETURN_IF_ERROR(UpdateAnomalyResults(
+                feature_pair_validation.feature_test_path(),
+                feature_pair_validation.dataset_name(),
+                feature_pair_validation.base_dataset_name(),
+                feature_pair_validation.feature_base_path(), validation,
+                result));
           }
-          // If the sql_expression evaluates to False, there is an anomaly.
-          TFDV_RETURN_IF_ERROR(UpdateAnomalyResults(
-              feature_pair_validation.feature_test_path(),
-              feature_pair_validation.dataset_name(),
-              feature_pair_validation.base_dataset_name(),
-              feature_pair_validation.feature_base_path(), validation, result));
         }
       }
     }

diff --git a/tensorflow_data_validation/anomalies/custom_validation_test.cc b/tensorflow_data_validation/anomalies/custom_validation_test.cc
@@ -168,6 +168,11 @@ TEST(CustomValidationTest, TestPairValidation) {
                severity: ERROR
                description: 'Test feature has too many zeros.'
              }
+             validations {
+               sql_expression: 'feature_test.num_stats.num_zeros > feature_base.num_stats.num_zeros'
+               severity: ERROR
+               description: 'Base feature has too few zeros.'
+             }
            })pb");
   metadata::v0::Anomalies expected_anomalies = ParseTextProtoOrDie<
       metadata::v0::Anomalies>(