sha174n opened a new pull request, #40392: URL: https://github.com/apache/superset/pull/40392
### SUMMARY Adhoc column and metric SQL is already routed through `validate_adhoc_subquery` and `sanitize_clause` before being turned into a SQL clause, but stored expressions on `TableColumn.expression` and `SqlMetric.expression` went straight to `literal_column` on both the write and read paths. The two code paths share the same intent but enforce different rules. This PR applies the same parser-based validator consistently: - **Write boundary** — `UpdateDatasetCommand` runs every incoming column and metric `expression` through the validator and surfaces failures as field-level `ValidationError`s. - **Query boundary** — `TableColumn.get_sqla_col`, `TableColumn.get_timestamp_expression`, and `SqlMetric.get_sqla_col` re-run the validator on stored expressions before handing them to `literal_column`, so rows written before this change are also covered. No new policy is introduced — the same rules already enforced for adhoc fields now apply consistently to stored expressions. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A — backend change only. ### TESTING INSTRUCTIONS Unit tests added in `tests/unit_tests/connectors/sqla/models_test.py` cover: - `validate_stored_expression` rejects multi-statement input - `validate_stored_expression` rejects set-operation input - `validate_stored_expression` accepts a `CASE` expression - `TableColumn.get_sqla_col` raises `QueryObjectValidationError` for a multi-statement expression - `SqlMetric.get_sqla_col` raises `QueryObjectValidationError` for a set-operation expression Run with: ``` pytest tests/unit_tests/connectors/sqla/models_test.py -k validate_stored_expression -k rejects -k accepts ``` ### ADDITIONAL INFORMATION - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (please mention if not backward compatible) - [x] Includes backend API changes — stored column/metric expressions referencing sub-queries against forbidden tables, set operations, or multi-statement SQL will now fail validation; this matches the existing behaviour already enforced for adhoc SQL. - [ ] Includes JS API changes - [x] Performance impact — one extra parse per stored expression per query, equivalent to what the adhoc path already pays. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
