diff --git a/docs/dqx/docs/dev/docs_authoring.mdx b/docs/dqx/docs/dev/docs_authoring.mdx index 4e2e60612..a2ff797c5 100644 --- a/docs/dqx/docs/dev/docs_authoring.mdx +++ b/docs/dqx/docs/dev/docs_authoring.mdx @@ -215,7 +215,7 @@ leak the component markup into these elements. Any page, section, or subsection heading can be tagged. The following conventions are used for tagging feature documentation: -* Untagged features represent generally-available functionality. If a version tag is missing, the feature has been available since before release version 0.9.0. +* Untagged features represent generally available functionality. If a version tag is missing, the feature has been available since before release version 0.9.0. * If a feature page is tagged, all untagged subheadings share the same version and lifecycle stage. * If any subheading is tagged, that capability has it own version and lifecycle stage which differs from the page-level version and lifecycle stage. * Sections with structural content, explanations, or examples should not be tagged. @@ -224,13 +224,10 @@ feature documentation: The following components are available: -*`` — a status badge linked - to the matching section of the [Feature lifecycle](/docs/reference/feature_lifecycle) reference. -*`` — "Available since DQX - v0.14.0", linked to that release's notes. -*`` - — "Deprecated in DQX v0.16.0", with an optional replacement named in the tooltip. -*`` — the row container that places the tags on their own line under the heading. +* `` — a status badge linked to the matching section of the [Feature Lifecycle](/docs/reference/feature_lifecycle) reference. +* `` — "Available since v0.14.0", linked to that release's notes. +* `` — "Deprecated in v0.16.0", with an optional replacement named in the tooltip. +* `` — the row container that places the tags on their own line under the heading. ## Content alignment and structure of folders diff --git a/docs/dqx/docs/reference/feature_lifecycle.mdx b/docs/dqx/docs/reference/feature_lifecycle.mdx index 49ef38c9a..8cb4ec829 100644 --- a/docs/dqx/docs/reference/feature_lifecycle.mdx +++ b/docs/dqx/docs/reference/feature_lifecycle.mdx @@ -1,10 +1,10 @@ --- sidebar_position: 500.5 -title: Feature lifecycle -sidebar_label: Feature lifecycle +title: Feature Lifecycle +sidebar_label: Feature Lifecycle --- -# Feature lifecycle +# Feature Lifecycle DQX features move through release stages as they mature. When a feature is not yet generally available, its documentation page shows a status badge next to the title — for example, the diff --git a/docs/dqx/docs/reference/quality_checks.mdx b/docs/dqx/docs/reference/quality_checks.mdx index 1c32b7991..db3df646d 100644 --- a/docs/dqx/docs/reference/quality_checks.mdx +++ b/docs/dqx/docs/reference/quality_checks.mdx @@ -4727,6 +4727,56 @@ When using custom message expressions: +## SQL escaping for checked columns +Most rules accept a `column` argument that can be a column name or a SQL expression. DQX automatically back-quotes column names that are not valid SQL identifiers (e.g. `Customer Name` or `Päivämäärä`) on a best-effort basis. Columns that cannot be resolved are reported as skipped rather than failing a run. + +Because column names and a SQL expression cannot always be differentiated, some ambiguous cases are not back-quoted automatically: +* A name that contains expression characters such as parentheses (e.g. `amount (usd)`) +* An operator-free expression that is treated as a name (e.g. `col not null`). + +When working with these cases, back-quote the column name when defining the check. + + + +```yaml +# Back-quote ambiguous column names when defining checks +- name: amount_is_null_or_empty + check: + function: is_not_null_and_not_empty + arguments: + column: "`amount (usd)`" + +- name: col_not_null_is_null_or_empty + check: + function: is_not_null_and_not_empty + arguments: + column: "`col not null`" +``` + + +```python +import yaml +import pyspark.sql.functions as F +from databricks.labs.dqx.rule import DQRowRule +from databricks.labs.dqx import check_funcs + +# Back-quote ambiguous column names when defining checks +checks = [ + DQRowRule( + name="amount_is_null_or_empty", + check_func=check_funcs.is_not_null_and_not_empty, + column="`amount (usd)`", + ), + DQRowRule( + name="col_not_null_is_null_or_empty", + check_func=check_funcs.is_not_null_and_not_empty, + column="`col not null`", + ), +] +``` + + + ## Converting checks between formats In DQX, checks can be defined either as Python classes or YAML declarations. When using YAML, the files are first parsed into dictionaries and then transformed into DQX class instances under the hood. Since both formats share the same internal structure, they are interchangeable and can be safely converted between one another. diff --git a/src/databricks/labs/dqx/check_funcs.py b/src/databricks/labs/dqx/check_funcs.py index 3b1dba864..69651c34b 100644 --- a/src/databricks/labs/dqx/check_funcs.py +++ b/src/databricks/labs/dqx/check_funcs.py @@ -25,6 +25,7 @@ is_sql_query_safe, safe_filter_expr, normalize_col_str, + normalize_column_expr, get_columns_as_strings, to_lowercase, ) @@ -5214,8 +5215,8 @@ def get_normalized_column_and_expr(column: str | Column) -> tuple[str, str, Colu - Spark Column expression corresponding to the input. """ col_expr = _get_column_expr(column) - column_str = get_column_name_or_alias(col_expr) - col_str_norm = get_column_name_or_alias(col_expr, normalize=True) + column_str = get_column_name_or_alias(column) + col_str_norm = get_column_name_or_alias(column, normalize=True) return col_str_norm, column_str, col_expr @@ -5349,13 +5350,17 @@ def _get_column_expr(column: Column | str) -> Column: """ Convert a column input (string or Column) into a Spark Column expression. + String inputs are normalized via *normalize_column_expr* so that column names requiring SQL identifier + escaping (e.g. column names with spaces, non-ASCII letters, etc.) are back-quoted and resolve correctly, + while genuine SQL expressions are passed through unchanged. + Args: column: The input column, provided as either a string column name or a Spark Column expression. Returns: A Spark Column expression corresponding to the input. """ - return F.expr(column) if isinstance(column, str) else column + return F.expr(normalize_column_expr(column)) if isinstance(column, str) else column def _handle_fk_composite_keys( diff --git a/src/databricks/labs/dqx/datacontract/contract_rules_generator.py b/src/databricks/labs/dqx/datacontract/contract_rules_generator.py index 6514e672b..dc629fcf6 100644 --- a/src/databricks/labs/dqx/datacontract/contract_rules_generator.py +++ b/src/databricks/labs/dqx/datacontract/contract_rules_generator.py @@ -33,6 +33,7 @@ from databricks.labs.dqx.errors import InvalidPhysicalTypeError, ODCSContractError, ParameterError from databricks.labs.dqx.telemetry import telemetry_logger from databricks.labs.dqx.package_utils import missing_required_packages +from databricks.labs.dqx.utils import normalize_column_expr # DQLLMEngine is referenced only as a type annotation. Eagerly importing it # requires installation of [llm] extras which may not be installed or wanted @@ -745,6 +746,7 @@ def _generate_range_rules_from_options( has_float_limits = (minimum is not None and isinstance(minimum, float)) or ( maximum is not None and isinstance(maximum, float) ) + normalized_column = normalize_column_expr(column_path) if minimum is not None and maximum is not None: if has_float_limits: @@ -753,7 +755,7 @@ def _generate_range_rules_from_options( "check": { "function": "sql_expression", "arguments": { - "expression": f"{column_path} >= {minimum} AND {column_path} <= {maximum}", + "expression": f"{normalized_column} >= {minimum} AND {normalized_column} <= {maximum}", "columns": [column_path], }, }, @@ -794,7 +796,7 @@ def _generate_range_rules_from_options( "check": { "function": "sql_expression", "arguments": { - "expression": f"{column_path} >= {minimum}", + "expression": f"{normalized_column} >= {minimum}", "columns": [column_path], }, }, @@ -834,7 +836,7 @@ def _generate_range_rules_from_options( "check": { "function": "sql_expression", "arguments": { - "expression": f"{column_path} <= {maximum}", + "expression": f"{normalized_column} <= {maximum}", "columns": [column_path], }, }, @@ -879,13 +881,15 @@ def _generate_string_length_rules_from_options( if min_length is None and max_length is None: return [] + normalized_column = normalize_column_expr(column_path) + if min_length is not None and max_length is not None and min_length == max_length: return [ { "check": { "function": "sql_expression", "arguments": { - "expression": f"LENGTH({column_path}) = {min_length}", + "expression": f"LENGTH({normalized_column}) = {min_length}", "columns": [column_path], }, }, @@ -905,7 +909,7 @@ def _generate_string_length_rules_from_options( "check": { "function": "sql_expression", "arguments": { - "expression": f"LENGTH({column_path}) >= {min_length} AND LENGTH({column_path}) <= {max_length}", + "expression": f"LENGTH({normalized_column}) >= {min_length} AND LENGTH({normalized_column}) <= {max_length}", "columns": [column_path], }, }, @@ -925,7 +929,7 @@ def _generate_string_length_rules_from_options( "check": { "function": "sql_expression", "arguments": { - "expression": f"LENGTH({column_path}) >= {min_length}", + "expression": f"LENGTH({normalized_column}) >= {min_length}", "columns": [column_path], }, }, @@ -945,7 +949,7 @@ def _generate_string_length_rules_from_options( "check": { "function": "sql_expression", "arguments": { - "expression": f"LENGTH({column_path}) <= {max_length}", + "expression": f"LENGTH({normalized_column}) <= {max_length}", "columns": [column_path], }, }, diff --git a/src/databricks/labs/dqx/manager.py b/src/databricks/labs/dqx/manager.py index 25bf8e885..9aa727e0b 100644 --- a/src/databricks/labs/dqx/manager.py +++ b/src/databricks/labs/dqx/manager.py @@ -17,6 +17,7 @@ from databricks.labs.dqx.utils import ( get_column_name_or_alias, is_simple_column_expression, + normalize_column_expr, quote_column_name, is_sql_query_safe, safe_filter_expr, @@ -127,11 +128,11 @@ def invalid_columns(self) -> list[str]: """ invalid_cols = [] - if self.check.column is not None and self._is_invalid_column(self.check.column): + if self.check.column is not None and self._is_invalid_check_column(self.check.column): invalid_cols.append(self._display_column_name(self.check.column)) elif self.check.columns is not None: # either column or columns can be provided, but not both for column in self.check.columns: - if self._is_invalid_column(column): + if self._is_invalid_check_column(column): invalid_cols.append(self._display_column_name(column)) return invalid_cols @@ -328,21 +329,27 @@ def _get_invalid_cols_message(self) -> str: return invalid_cols_message + def _is_invalid_check_column(self, column: str | Column) -> bool: + """ + Returns True if a check column cannot be resolved in the input DataFrame, otherwise False. + + A string column is validated exactly as it will be executed: it is passed through + *normalize_column_expr* (which back-quotes names requiring SQL identifier escaping, e.g. + "Customer Name") first, so validation and execution agree and an unresolvable name is skipped + rather than aborting the run. + """ + resolved: str | Column = normalize_column_expr(column) if isinstance(column, str) else column + return self._is_invalid_column(resolved) + def _is_invalid_column(self, column: str | Column) -> bool: """ - Returns True if the specified column is invalid (i.e., cannot be resolved in the input DataFrame), - otherwise False. + Returns True if the specified column or expression is invalid (i.e., cannot be resolved in the + input DataFrame), otherwise False. """ try: col_expr = F.expr(column) if isinstance(column, str) else column _ = self.df.select(col_expr).schema # perform logical plan validation without triggering computation except AnalysisException as e: - # The input string may be a SQL expression (e.g. "a + b") or a plain column name that may require - # SQL identifier escaping (spaces / non-ASCII / reserved chars, e.g. "Customer Name"). To validate - # the input string, we first attempt F.expr() and retry failing strings as a backtick-quoted identifiers. - # String expressions parse cleanly and only genuine single-identifier names pass fallback validation. - if isinstance(column, str) and not self._is_invalid_quoted_column(column): - return False # If column is not accessible or column expression cannot be evaluated, an AnalysisException is thrown. # Note: This does not cover all error conditions. Some issues only appear during a Spark action. logger.debug( @@ -351,14 +358,3 @@ def _is_invalid_column(self, column: str | Column) -> bool: ) return True return False - - def _is_invalid_quoted_column(self, column: str) -> bool: - """ - Returns True if the string cannot be resolved as a single backtick-quoted column identifier, - otherwise False. - """ - try: - _ = self.df.select(F.expr(quote_column_name(column))).schema - except AnalysisException: - return True - return False diff --git a/src/databricks/labs/dqx/utils.py b/src/databricks/labs/dqx/utils.py index 0e5aba931..38cc653b8 100644 --- a/src/databricks/labs/dqx/utils.py +++ b/src/databricks/labs/dqx/utils.py @@ -78,6 +78,9 @@ def _validate_spark_column(value: Any) -> Any: COLUMN_NORMALIZE_EXPRESSION = re.compile("[^a-zA-Z0-9]+") COLUMN_PATTERN = re.compile(r"Column<'(.*?)(?: AS (\w+))?'>$", re.DOTALL) INVALID_COLUMN_NAME_PATTERN = re.compile(r"[\s,;{}\(\)\n\t=]+") +VALID_UNQUOTED_IDENTIFIER_PATTERN = re.compile(r"^[A-Za-z_][A-Za-z0-9_]*$") +SQL_EXPRESSION_STRUCTURE_PATTERN = re.compile(r"""[()\[\]{},;'"`*]""") +SQL_EXPRESSION_OPERATOR_PATTERN = re.compile(r"[-+/%<>=!&|~^]\s|\s[-+/%<>=!&|~^]") _UNRESOLVED_PLACEHOLDER_PATTERN = re.compile(r"\{\{[^}]*\}\}") # Destructive SQL statement keywords rejected by `is_sql_query_safe`. SELECT is intentionally @@ -149,6 +152,8 @@ def get_column_name_or_alias( Args: column: Column, ConnectColumn (if PySpark Connect available), or string representing a column. normalize: If True, normalizes the column name (removes special characters, converts to lowercase). + For string inputs, any surrounding back-quotes the user supplied (e.g. "`Customer Name`") are + stripped first so quotes do not leak into display names or generated check names. allow_simple_expressions_only: If True, raises an error if the column expression is not a simple expression. Complex PySpark expressions (e.g., conditionals, arithmetic, or nested transformations), cannot be fully reconstructed correctly when converting to string (e.g. F.col("a") + F.lit(1)). @@ -161,7 +166,9 @@ def get_column_name_or_alias( InvalidParameterError: If the column expression is invalid or unsupported. """ if isinstance(column, str): - col_str = column + col_str = unquote_column_name(column) + if normalize: + col_str = normalize_col_str(col_str) else: # Extract the last alias or column name from the PySpark Column string representation. # Strip the representation first to guard against trailing whitespace or CRLF line endings @@ -308,6 +315,62 @@ def quote_column_name(name: str) -> str: return f"`{escaped}`" +def unquote_column_name(name: str) -> str: + """ + Removes surrounding back-quotes from a column name, reversing :func:`quote_column_name`. + + A column reference that the user already back-quoted (e.g. "`Customer Name`") is unwrapped to its + plain form ("Customer Name") for use in display names and messages. Strings that are not a single + back-quoted identifier (plain names, dotted paths, SQL expressions) are returned unchanged. + + Args: + name: Column reference provided as a string. + + Returns: + The column name without surrounding back-quotes. + """ + if len(name) >= 2 and name.startswith("`") and name.endswith("`"): + return name[1:-1].replace("``", "`") + return name + + +def normalize_column_expr(column: str) -> str: + """ + Prepares a column reference string for use with ``F.expr``, back-quoting names that require SQL + identifier escaping while leaving SQL expressions untouched. + + Check functions accept a column as either a plain name or a SQL expression string. A plain name that + is not a valid bare identifier (spaces, dashes, non-ASCII letters, etc., e.g. "Customer Name", + "gross-margin" or "Päivämäärä") does not parse when passed to *F.expr* and must be back-quoted. A + SQL expression (e.g. "a + b", "substr(x, 1, 2)", "*") must be passed through as-is. + + Name and expression cannot be told apart with certainty from a string alone. This function uses a + conservative heuristic. A string is treated as an expression, and returned unchanged, when it + contains characters that typically appear in expressions (parentheses, brackets, quotes, comma, star) + or arithmetic/comparison operators that are whitespace-separated (e.g. "a + b"). Everything else is + treated as a dotted column path: each segment is left alone if it is a valid bare identifier. Otherwise + it is back-quoted. + + This heuristic intentionally excludes two ambiguous cases: names that contain expression characters + (e.g. "amount (usd)"), and operator-free SQL expressions such as "col IS NOT NULL" (which is treated + as a name). Callers with such columns should back-quote the name themselves, pass a Column expression, + or use the *sql_expression* check. + + Args: + column: Column reference provided as a string (plain name, nested path, or SQL expression). + + Returns: + A string safe to pass to *F.expr*. + """ + if SQL_EXPRESSION_STRUCTURE_PATTERN.search(column) or SQL_EXPRESSION_OPERATOR_PATTERN.search(column): + return column + + return ".".join( + segment if VALID_UNQUOTED_IDENTIFIER_PATTERN.match(segment) else quote_column_name(segment) + for segment in column.split(".") + ) + + def normalize_col_str(col_str: str) -> str: """ Normalizes string to be compatible with metastore column names by applying the following transformations: diff --git a/tests/integration/test_apply_checks.py b/tests/integration/test_apply_checks.py index 403e1cf2d..5ee7fe86f 100755 --- a/tests/integration/test_apply_checks.py +++ b/tests/integration/test_apply_checks.py @@ -10442,6 +10442,165 @@ def test_apply_checks_with_has_valid_schema_special_char_columns_are_valid(ws, s assert_df_equality(checked.sort("id"), expected.sort("id"), ignore_nullable=True) +def test_apply_checks_by_metadata_on_column_requiring_escaping_runs(ws, spark): + """Ensures row-level checks execute on a column whose name requires SQL identifier escaping for checks defined using YAML.""" + dq_engine = DQEngine(workspace_client=ws, extra_params=EXTRA_PARAMS) + + schema = "id int, `Päivämäärä` int, `Customer Name` string" + test_df = spark.createDataFrame([[1, None, None], [2, 20, "test_customer"]], schema) + + checks = [ + { + "name": "Päivämäärä_not_null", + "criticality": "error", + "check": {"function": "is_not_null", "arguments": {"column": "Päivämäärä"}}, + }, + { + "name": "customer_name_not_null", + "criticality": "error", + "check": {"function": "is_not_null", "arguments": {"column": "Customer Name"}}, + }, + ] + actual = dq_engine.apply_checks_by_metadata(test_df, checks) + + expected = spark.createDataFrame( + [ + [ + 1, + None, + None, + [ + { + "name": "Päivämäärä_not_null", + "message": "Column 'Päivämäärä' value is null", + "columns": ["Päivämäärä"], + "filter": None, + "function": "is_not_null", + "run_time": RUN_TIME, + "run_id": RUN_ID, + "user_metadata": {}, + }, + { + "name": "customer_name_not_null", + "message": "Column 'Customer Name' value is null", + "columns": ["Customer Name"], + "filter": None, + "function": "is_not_null", + "run_time": RUN_TIME, + "run_id": RUN_ID, + "user_metadata": {}, + }, + ], + None, + ], + [2, 20, "test_customer", None, None], + ], + schema + REPORTING_COLUMNS, + ) + + assert_df_equality(actual.sort("id"), expected.sort("id"), ignore_nullable=True) + + +def test_apply_checks_on_column_requiring_escaping_runs(ws, spark): + """Ensures row-level checks execute on a column whose name requires SQL identifier escaping for checks defined using DQX classes.""" + dq_engine = DQEngine(workspace_client=ws, extra_params=EXTRA_PARAMS) + + schema = "id int, `Päivämäärä` int, `Customer Name` string" + test_df = spark.createDataFrame([[1, None, None], [2, 20, "test_customer"]], schema) + + checks = [ + DQRowRule( + name="Päivämäärä_not_null", + criticality="error", + check_func=check_funcs.is_not_null, + column="Päivämäärä", + ), + DQRowRule( + name="customer_name_not_null", + criticality="error", + check_func=check_funcs.is_not_null, + column="Customer Name", + ), + ] + actual = dq_engine.apply_checks(test_df, checks) + + expected = spark.createDataFrame( + [ + [ + 1, + None, + None, + [ + { + "name": "Päivämäärä_not_null", + "message": "Column 'Päivämäärä' value is null", + "columns": ["Päivämäärä"], + "filter": None, + "function": "is_not_null", + "run_time": RUN_TIME, + "run_id": RUN_ID, + "user_metadata": {}, + }, + { + "name": "customer_name_not_null", + "message": "Column 'Customer Name' value is null", + "columns": ["Customer Name"], + "filter": None, + "function": "is_not_null", + "run_time": RUN_TIME, + "run_id": RUN_ID, + "user_metadata": {}, + }, + ], + None, + ], + [2, 20, "test_customer", None, None], + ], + schema + REPORTING_COLUMNS, + ) + + assert_df_equality(actual.sort("id"), expected.sort("id"), ignore_nullable=True) + + +def test_apply_checks_on_unresolvable_column_is_skipped_and_run_completes(ws, spark): + """A check on a column that cannot be resolved (even after escaping) is skipped rather than aborting the + run, and other checks in the same set still evaluate. This is the behaviour #1481 regressed: validation + and execution now agree, so an unresolvable name is reported as skipped instead of crashing later.""" + dq_engine = DQEngine(workspace_client=ws, extra_params=EXTRA_PARAMS) + + schema = "id int, `Customer Name` string" + test_df = spark.createDataFrame([[1, None], [2, "test_customer"]], schema) + + checks = [ + { + "name": "missing_col_not_null", + "criticality": "error", + "check": {"function": "is_not_null", "arguments": {"column": "Does Not Exist"}}, + }, + { + "name": "customer_name_not_null", + "criticality": "error", + "check": {"function": "is_not_null", "arguments": {"column": "Customer Name"}}, + }, + ] + checked = dq_engine.apply_checks_by_metadata(test_df, checks) + errors_by_row = {row["id"]: row["_errors"] for row in checked.select("id", "_errors").collect()} + + # The unresolvable column can't be evaluated, so it's reported as skipped on every row rather than + # aborting the run. + for row_id in (1, 2): + skipped = [e for e in (errors_by_row[row_id] or []) if e["name"] == "missing_col_not_null"] + assert len(skipped) == 1, f"row {row_id}: {errors_by_row[row_id]}" + assert skipped[0]["skipped"] is True + + # The escaping-required column still runs end-to-end: it flags the null for row id=1 and passes id=2. + resolved_row1 = [e for e in (errors_by_row[1] or []) if e["name"] == "customer_name_not_null"] + assert len(resolved_row1) == 1 + assert resolved_row1[0]["message"] == "Column 'Customer Name' value is null" + resolved_row2 = [e for e in (errors_by_row[2] or []) if e["name"] == "customer_name_not_null"] + assert len(resolved_row2) == 0 + + def test_apply_checks_unsafe_filter_is_skipped_and_other_checks_still_run(ws, spark): """A check with an unsafe (destructive-SQL) filter is skipped through DQRuleManager while every other check in the same rule set is still evaluated end-to-end — the run is not aborted.""" diff --git a/tests/unit/test_datacontract_generator.py b/tests/unit/test_datacontract_generator.py index 95dde10d3..3ebcdb483 100644 --- a/tests/unit/test_datacontract_generator.py +++ b/tests/unit/test_datacontract_generator.py @@ -2375,6 +2375,34 @@ def test_field_with_only_maximum_constraint(self, generator): finally: os.unlink(temp_path) + def test_sql_expression_back_quotes_column_requiring_escaping(self, generator): + """Tests that rules parse correctly for columns that require SQL escaping.""" + contract_dict = self.create_basic_contract( + properties=[ + { + "name": "Päivämäärä", + "physicalType": "DOUBLE", + "logicalType": "number", + "logicalTypeOptions": {"minimum": 0.0, "maximum": 100.0}, + } + ] + ) + + temp_path = self.create_test_contract_file(custom_contract=contract_dict) + + try: + rules = generator.generate_rules_from_contract( + contract_file=temp_path, generate_predefined_rules=True, process_text_rules=False + ) + + range_rule = next(r for r in rules if r["check"]["function"] == "sql_expression") + assert range_rule["check"]["arguments"]["expression"] == "`Päivämäärä` >= 0.0 AND `Päivämäärä` <= 100.0" + # The columns list keeps the raw name (used for resolution / skip messaging). + assert range_rule["check"]["arguments"]["columns"] == ["Päivämäärä"] + + finally: + os.unlink(temp_path) + def test_field_with_only_integer_maximum_constraint(self, generator): """Test that field with only integer maximum generates is_aggr_not_greater_than rule.""" contract_dict = self.create_basic_contract( diff --git a/tests/unit/test_row_checks.py b/tests/unit/test_row_checks.py index 4fb780f61..1ac7683cb 100644 --- a/tests/unit/test_row_checks.py +++ b/tests/unit/test_row_checks.py @@ -7,6 +7,7 @@ from databricks.labs.dqx.utils import get_column_name_or_alias from databricks.labs.dqx.check_funcs import ( + get_normalized_column_and_expr, is_equal_to, is_not_equal_to, is_in_range, @@ -34,6 +35,24 @@ LIMIT_VALUE_ERROR = "Limit is not provided" +@pytest.mark.parametrize( + "column, expected_display, expected_normalized", + [ + ("col1", "col1", "col1"), + ("Customer Name", "Customer Name", "customer_name"), + ("Päivämäärä", "Päivämäärä", "p_iv_m_r"), + # A name the user already back-quoted is displayed without the quotes + ("`Customer Name`", "Customer Name", "customer_name"), + ], +) +def test_get_normalized_column_and_expr_names_come_from_input(column, expected_display, expected_normalized): + """Display and normalized names come from the input string, not the back-quoted expression, so that + escaping a name for execution does not leak back-quotes into messages and generated check names.""" + col_str_norm, column_str, _ = get_normalized_column_and_expr(column) + assert column_str == expected_display + assert col_str_norm == expected_normalized + + @pytest.mark.parametrize("min_limit, max_limit", [(None, 1), (1, None)]) def test_col_is_in_range_missing_limits(min_limit, max_limit): with pytest.raises(MissingParameterError, match=LIMIT_VALUE_ERROR): diff --git a/tests/unit/test_utils.py b/tests/unit/test_utils.py index 0a3f802ad..7f319a4de 100644 --- a/tests/unit/test_utils.py +++ b/tests/unit/test_utils.py @@ -26,6 +26,8 @@ get_file_extension, resolve_variables, quote_column_name, + unquote_column_name, + normalize_column_expr, ) from databricks.labs.dqx.rule import normalize_bound_args from databricks.labs.dqx.errors import InvalidParameterError, InvalidConfigError, UnsafeSqlQueryError @@ -945,3 +947,67 @@ def test_quote_column_name(): column_name = "my `column` name" result = quote_column_name(column_name) assert result == "`my ``column`` name`" + + +@pytest.mark.parametrize( + "name, expected", + [ + # Back-quoted identifiers are unwrapped + ("`Customer Name`", "Customer Name"), + ("`my ``column`` name`", "my `column` name"), + # Plain names, dotted paths and expressions are returned unchanged + ("Customer Name", "Customer Name"), + ("id", "id"), + ("struct_col.field1", "struct_col.field1"), + ("a + b", "a + b"), + ("*", "*"), + ], +) +def test_unquote_column_name(name: str, expected: str): + assert unquote_column_name(name) == expected + + +@pytest.mark.parametrize( + "name", + ["Customer Name", "`Customer Name`", "id", "struct_col.field1"], +) +def test_unquote_reverses_quote_column_name(name: str): + assert unquote_column_name(quote_column_name(name)) == name + + +@pytest.mark.parametrize( + "column, expected", + [ + # Valid bare identifiers are left untouched + ("id", "id"), + ("col_1", "col_1"), + ("_private", "_private"), + # Nested-field paths keep working; each valid segment is left alone + ("struct_col.field1", "struct_col.field1"), + ("a.b.c", "a.b.c"), + # Names requiring SQL escaping are back-quoted + ("Päivämäärä", "`Päivämäärä`"), + ("Ääkkönen", "`Ääkkönen`"), + ("Customer Name", "`Customer Name`"), + # A character that is an operator only when whitespace-separated is treated as part of the name + ("gross-margin", "`gross-margin`"), + ("col#1", "`col#1`"), + # Only the segment that needs escaping is quoted in a nested path + ("parent.Odd Name", "parent.`Odd Name`"), + # Already back-quoted names contain a backtick and are passed through unchanged + ("`Customer Name`", "`Customer Name`"), + # SQL expressions are passed through unchanged + ("a + b", "a + b"), + ("gross - margin", "gross - margin"), + ("substr(x, 1, 2)", "substr(x, 1, 2)"), + ("amount * 1.5", "amount * 1.5"), + ("col > 5", "col > 5"), + ("*", "*"), + # Ambiguous cases the conservative heuristic intentionally does not escape: names containing + # expression characters, and operator-free expressions (treated as a name). + ("amount (usd)", "amount (usd)"), + ("col IS NOT NULL", "`col IS NOT NULL`"), + ], +) +def test_normalize_column_expr(column: str, expected: str): + assert normalize_column_expr(column) == expected