Allow custom check messages#1092
Conversation
Co-authored-by: Isaac
|
✅ 735/735 passed, 43 skipped, 6h25m21s total Running from acceptance #4775 |
…eholders
The custom-message field is renamed to message_expr and now accepts either a
Spark SQL expression string or a Spark Column object. The expression is
evaluated as-is — DQX no longer substitutes {rule_name}, {check_func_name}, or
{column_value} placeholders. Callers reference columns and identifiers directly
inside the expression.
- DQRule / DQForEachColRule: rename message -> message_expr (str | Column | None)
- to_dict serialises message_expr only when it is a string (Column is in-process)
- DQRuleManager._build_message_col simplified: F.expr(str) or use Column directly
- checks_serializer: read message_expr key from metadata, pass to constructors
- Unit tests rewritten for the new attribute name and Column variant
- Integration tests rewritten without placeholders; the existing
test_apply_checks_without_custom_message_unchanged is dropped (the Engine's
no-message default path is already covered by surrounding tests)
- Docs (quality_checks.mdx) rewritten to drop the placeholder section, add a
Column-based Python example, and annotate each example with the rendered
message string
Co-authored-by: Isaac
mwojtyczka
left a comment
There was a problem hiding this comment.
Small, well-isolated change with good happy-path test coverage. Posting 11 inline comments below covering: a security exposure (arbitrary F.expr evaluation of strings sourced from YAML/metadata), a UX trap with unquoted strings, several maintainability/polish items, and one test bug.
One non-inline issue: PR description doesn't match the implementation.
The description says:
Add an optional
messagecallable parameter ... that allows users to define custom check failure messages. The callable receives rule context (rule_name,check_func_name,check_func_args,column_value) and returns a Spark Column expression…
But the actual API is message_expr: str | Column | None — no callable, no rule context. Either the description should be rewritten to match what shipped, or the implementation should provide the callable form the description promises. Reviewers (and future readers running git log / GitHub PR archeology) rely on the description for context.
There was a problem hiding this comment.
Pull request overview
This PR introduces support for overriding default DQX check failure messages via a new optional message_expr field on rules, allowing users to supply either a Spark SQL expression string or a Spark Column expression to generate custom messages for failed checks.
Changes:
- Add
message_expr: str | Column | NonetoDQRuleand propagate it throughDQForEachColRule. - Update result-building to use
message_expras the failure message when provided. - Extend metadata deserialization to accept
message_expr, and add unit/integration tests plus docs for the new behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_custom_messages.py | Adds unit tests for message_expr acceptance/propagation/serialization behavior. |
| tests/integration/test_custom_messages.py | Adds integration coverage verifying custom messages appear in results for class-based and metadata-based checks. |
| src/databricks/labs/dqx/rule.py | Adds message_expr field to rules and ensures it is propagated/serialized (string-only). |
| src/databricks/labs/dqx/manager.py | Uses a dedicated builder to select between default messages and custom message_expr. |
| src/databricks/labs/dqx/checks_serializer.py | Deserializes message_expr from metadata and passes it into created rules. |
| docs/dqx/docs/reference/quality_checks.mdx | Documents how to use message_expr from Python and YAML. |
Comments suppressed due to low confidence (1)
src/databricks/labs/dqx/checks_serializer.py:276
message_expris now deserialized from metadata, but checks persisted via the table storage path won’t round-trip it:CHECKS_TABLE_SCHEMA/DataFrameConverter.to_dataframe()/from_dataframe()(inchecks_storage.py) currently have no place to store/readmessage_expr. This means saving checks withmessage_exprto a table will silently drop it on load, and can also cause rule_set_fingerprint/idempotency mismatches because fingerprints includemessage_exprwhile the stored rows do not.
criticality = check_def.get("criticality", "error")
filter_str = check_def.get("filter")
user_metadata = check_def.get("user_metadata")
message_expr = check_def.get("message_expr")
# Exclude `column` and `columns` from check_func_kwargs
# as these are always included in the check function call
check_func_kwargs = {k: v for k, v in func_args.items() if k not in {"column", "columns"}}
# treat non-registered function as row-level checks
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # or use literal run time if explicitly overridden | ||
| run_time_expr = F.current_timestamp() if self.run_time_overwrite is None else F.lit(self.run_time_overwrite) | ||
|
|
||
| message_col = self._build_message_col(condition) |
mwojtyczka
left a comment
There was a problem hiding this comment.
Re-review of custom check messages. Solid, well-tested feature with clean null-preservation semantics. One higher-impact concern around the safety check producing false positives on ordinary message text, plus a few smaller notes left inline.
| * Wrap column references with `coalesce` to avoid null messages. In Spark SQL, `concat(..., null)` returns `null`. | ||
| * Pass literal string values with quotes (e.g. `'Email must not be null'`) to ensure they are not evaluated as SQL expressions. | ||
| * Avoid long messages or referencing unbounded string columns. Message text is **truncated at 500 characters**. | ||
| * Avoid unquoted SQL keywords that may be executed naively (e.g. `DELETE` or `TRUNCATE`). |
| if isinstance(self.message_expr, str): | ||
| self._validate_message_expression(self.message_expr) |
| user_metadata = check_def.get("user_metadata") | ||
| message_expr = check_def.get("message_expr") |
Changes
Add an optional
message_exprparameter toDQRulethat allows users to define custom check failure messages as SparkColumnobjects or SQL expression strings. WhenmessageisNone(the default), the default message behavior is preserved. When provided, the custom message replaces the default message for failed rows.Skipped violation test helper
Introduces a
build_skipped_violationmethod for testing skipped checks (e.g. due to invalid expressions, column references, or custom messages).Linked issues
Resolves #958
Tests