betodealmeida opened a new pull request, #30578: URL: https://github.com/apache/superset/pull/30578
<!--- Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/ Example: fix(dashboard): load charts correctly --> ### SUMMARY <!--- Describe the change below, including rationale and design decisions --> An improvement to https://github.com/apache/superset/pull/30350. Today, when a DB engine spec has no corresponding `sqlglot` dialect, we use a generic one to parse queries, which works fine. When converting the AST back to a string to pretty-print SQL, though, we hit a problem: in some databases the query becomes invalid. For example, this Firebolt query: ```sql SELECT col FROM t WHERE col NOT IN (1, 2); ``` Gets formatted to: ```sql SELECT col FROM t WHERE NOT col IN (1, 2); ``` While equivalent, the latter is invalid in Firebolt. The long term solution is to add a Firebolt dialect to `sqlglot`, at least handling this edge case. As a more generic solution, this PR implements logic where, when a DB engine spec doesn't have a corresponding dialect in `sqlglot`, the query is pretty-printed using `sqlparse.format`. This way we have the strictness and correctness of `sqlglot` when parsing queries, and the leniency of `sqlparse` when formatting them. ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF <!--- Skip this if not applicable --> N/A ### TESTING INSTRUCTIONS <!--- Required! What steps can be taken to manually verify the changes? --> Added unit tests covering the problem. ### 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
