-
Notifications
You must be signed in to change notification settings - Fork 128
Update handling of metadata columns during schema validation #1002
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 9 commits
3c5aae6
0e8d61b
49624f3
64aa561
36b7dca
08d235a
8083221
02291ea
d2f88ca
1f92e48
f6e7246
c3f5587
c91c18f
32a5f14
5115968
ab4df8b
5a87c5e
b6728d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
| import logging | ||
| from concurrent import futures | ||
| from collections.abc import Callable | ||
| from dataclasses import replace | ||
| from datetime import datetime | ||
| from functools import cached_property | ||
| from typing import Any | ||
|
|
@@ -14,6 +15,7 @@ | |
| from pyspark.sql.streaming import StreamingQuery | ||
|
|
||
| from databricks.labs.dqx.base import DQEngineBase, DQEngineCoreBase | ||
| from databricks.labs.dqx import check_funcs | ||
| from databricks.labs.dqx.checks_resolver import resolve_custom_check_functions_from_path | ||
| from databricks.labs.dqx.checks_serializer import deserialize_checks | ||
| from databricks.labs.dqx.config_serializer import ConfigSerializer | ||
|
|
@@ -345,6 +347,29 @@ def _all_are_dq_rules(checks: list[DQRule]) -> bool: | |
| """Check if all elements in the checks list are instances of DQRule.""" | ||
| return all(isinstance(check, DQRule) for check in checks) | ||
|
|
||
| def _preselect_schema_validation_columns(self, df: DataFrame, check: DQRule) -> DQRule: | ||
|
ghanse marked this conversation as resolved.
Outdated
|
||
| """ | ||
| Determines the selected columns for schema validation checks. This is required when using `has_valid_schema` to | ||
| ignore columns added during quality checking. This includes: | ||
| * DQX result columns (e.g. '_warnings' and '_errors') | ||
| * Internal columns used for dataset-level checks | ||
|
|
||
| Args: | ||
| df: Input DataFrame | ||
| check: DQRule to be modified | ||
| """ | ||
|
|
||
| # Default columns to all columns of the current DataFrame if not explicitly set | ||
| if check.check_func_kwargs.get("columns"): | ||
| return check | ||
|
|
||
|
ghanse marked this conversation as resolved.
|
||
| if check.check_func_args and len(check.check_func_args) >= 3: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would remove this condition to make it more generic. We will have to use this function for anomaly detection as well, which requires a list of columns. There are a couple of issues with using check_func_args here:
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I used |
||
| return check | ||
|
|
||
| rule_kwargs = check.check_func_kwargs.copy() | ||
| rule_kwargs["columns"] = [col for col in df.columns if col not in set(self._result_column_names.values())] | ||
| return replace(check, check_func_kwargs=rule_kwargs) | ||
|
|
||
| def _append_empty_checks(self, df: DataFrame) -> DataFrame: | ||
| """Append empty checks at the end of DataFrame. | ||
|
|
||
|
|
@@ -389,8 +414,13 @@ def _create_results_array( | |
| current_df = df | ||
|
|
||
| for check in checks: | ||
| normalized_check = ( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should make it more generic, since more checks will require this in the future. This will also allow the creation of custom functions that operate on the full schema.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated |
||
| self._preselect_schema_validation_columns(df, check) | ||
| if check.check_func is check_funcs.has_valid_schema | ||
| else check | ||
| ) | ||
| manager = DQRuleManager( | ||
| check=check, | ||
| check=normalized_check, | ||
| df=current_df, | ||
| spark=self.spark, | ||
| run_id=self.run_id, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The list comprehension on line 2019 performs O(n*m) lookups where n is the number of selected columns and m is the number of ignore columns. Convert
ignore_column_namesto a set before the list comprehension for O(n) performance:ignore_set = set(ignore_column_names)then useif col not in ignore_set.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good feedback