gabotorresruiz commented on PR #42087: URL: https://github.com/apache/superset/pull/42087#issuecomment-5207887503
Two small things on the description and docs, neither blocking. The summary says the save-time check can be deferred past via "the create path (which has no expression validation)". That one does not quite hold: `DatasetPostSchema` (`superset/datasets/schemas.py:160`) does not accept `columns` or `metrics` at all, so create cannot set an expression in the first place. The write paths that really skip validation are import v1 (`ImportV1ColumnSchema.expression` at `schemas.py:275`, `ImportV1MetricSchema.expression` at `schemas.py:315`, and nothing under `superset/commands/dataset/importers/v1/` calls `validate_stored_expression`) and duplicate (`superset/commands/dataset/duplicate.py:91,104`, which copies expressions verbatim). Naming those two makes the case for this PR stronger, and both are live today. Second, this is a user-visible behavior change worth an `UPDATING.md` entry under `## Next`. `ALLOW_ADHOC_SUBQUERY` defaults to False, so any existing dataset with a sub-query in a stored column or metric works today and starts failing after this lands, with no migration step and nothing in the error naming the offending dataset column. There is precedent for exactly this at `UPDATING.md:983` (PR 19242, when adhoc sub-queries were first disabled by default). Something short like: ```md - [42087](https://github.com/apache/superset/pull/42087): Stored calculated-column and metric expressions are now validated at query time with the same sub-query policy as adhoc expressions. Datasets whose stored expressions contain sub-queries will fail at query time unless `ALLOW_ADHOC_SUBQUERY` is enabled. ``` -- 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]
