ege-st opened a new pull request, #24942: URL: https://github.com/apache/superset/pull/24942
<!--- Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/ Example: fix(dashboard): load charts correctly --> ### SUMMARY This fixes two bugs in the Apache Pinot DB Engine spec: 1. If a column was marked as `temporal` and no time grain was provided then the query would be constructed with illegal `{}` around the column name causing Pinot to reject the query as syntactically invalid. The code has been updated to remove the incorrect `{}` 2. In some cases, `get_timestamp_expr` will be called by `models.adhoc_column_to_sqla` and `None` is explicitly passed for the `pdf` parameter. This would cause the Pinot `get_timestamp_expr` to fault. To fix this, the `models.adhoc_column_to_sqla` method is updated to get the `python_date_format` for a column (if that data is available). (Without this fix, queries created for charts simply do _not_ use the date format a user sets for a temporal column). ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF <!--- Skip this if not applicable --> ### TESTING INSTRUCTIONS <!--- Required! What steps can be taken to manually verify the changes? --> 1. Connected to a Pinot data set and marked a `long` column as `epoch_ms` and tested creating a Bar Chart V2 to confirm that the query would be correctly constructed (this tested the issue where `None` is passed for the `pdf` parameter. 2. Setting a String column as temporal and using `%Y-%m-%d` as the PDF value and confirming that when a chart is created the correct Date Time conversion function is used to parse the text date time. ### 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: [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]
