villebro commented on PR #20729: URL: https://github.com/apache/superset/pull/20729#issuecomment-1209318314
@john-bodley I have to agree with @dungdm93 here - Over the years the Presto spec has caused me considerable grief due to the code being IMO messy and difficult to maintain. For that reason I've been very happy to see the work that @dungdm93 has done on the Trino spec, which IMO is of a great quality and easy follow and extend. So rather than double up on using the old Presto spec code, I'd rather see us gradually phasing it out. WRT to compatibility of the drivers, I think they're fairly far away from each other right now. For instance, currently clicking on a table in SQL Lab with the recommended `trino` driver gives off a metadata fetching error (causes a 500 in the backend): <img width="853" alt="image" src="https://user-images.githubusercontent.com/33317356/183643500-ebb7b70c-761b-4805-83ed-7209280b6f1b.png";> This is due to the `latest_partition` method in the Presto spec being incompatible with the `trino` driver. There are other similar issues; I'm opening a PR soon to fix query cancellation, which also works slightly differently, but there appear to be other differences, too. If we really want to share code between the Presto and Trino specs I would recommend doing something similar to what we're doing for Postgres and Postgres-like databases, where we have the uncontroversial shared pieces in an abstract `PostgresBaseEngineSpec` which the actual implementations, like `PostgresEngineSpec`, `HanaEngineSpec`, `RedshiftEngineSpec` etc use. In this case, we could have a `PrestoBaseEngineSpec` into which we move the high quality shared code, and then leave the messy bits in the old `PrestoEngineSpec`, while gradually refactoring them into the shared `PrestoBaseEngineSpec`, all while adding proper and relevant tests for them. -- 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]
![https://user-images.githubusercontent.com/33317356/183643500-ebb7b70c-761b-4805-83ed-7209280b6f1b.png"](https://user-images.githubusercontent.com/33317356/183643500-ebb7b70c-761b-4805-83ed-7209280b6f1b.png"){kind=link}