villebro opened a new pull request, #22405: URL: https://github.com/apache/superset/pull/22405
### SUMMARY When exploring a query containing a temporal column I noticed that the query would fail unless persisted as a virtual dataset when the `GENERIC_CHART_AXES` feature flag is enabled. Digging around I noticed that the `superset.models.helpers` module duplicates essentially all logic from the `superset.connectors.sqla.models` module. From a maintenance perspective this is highly problematic, as the SQLA connector model is by far one of the biggest pieces of tech debt in the backend, along with being very fragile and often updated with minor fixes. Duplicating this code without having tests in place to ensure that they're in sync makes it difficult for contributors to know that changes need to be applied in both modules. This is just a hotfix to make the `Query` class from the helpers module work with `GENERIC_X_AXES`. However, IMO we need to deduplicate this code ASAP, as they've already diverged quite a lot, with many fixes to the SQLA model not having made their way into the helpers module (#22280 is an example of such). ### AFTER Now a query that hasn't been saved as a virtual dataset works with temporal filters:  ### BEFORE Previously the same chart would fail.  ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF <!--- Skip this if not applicable --> ### TESTING INSTRUCTIONS <!--- Required! What steps can be taken to manually verify the changes? --> ### ADDITIONAL INFORMATION <!--- Check any relevant boxes with "x" --> <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue --> - [ ] Has associated issue: - [ ] Required feature flags: - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API -- 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: notifications-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
