[GitHub] [incubator-superset] bkyryliuk commented on pull request #10498: feat: welcome presto to the suite of tested databases

[GitBox <gi...@apache.org>]

bkyryliuk commented on pull request #10498:
URL: https://github.com/apache/incubator-superset/pull/10498#issuecomment-670083445


   > I think we should aim to put as much db-specific logic in `db_engine_specs`, especially in the examples loading logic. We need to add similar logic for mutating column names, e.g. BigQuery doesn't support columns starting with a number (there's other similar problems for other dbs).
   > 
   > WRT the test assertions, I think those are fine left as-is. Also curious about that extra cache key test failing, that seems like a potential security problem, so I'm happy to help track down what's causing that.
   
   Agree with all the suggestions, my only comment here is this PR is fairly large already - I am happy to tackle the suggestions in the followup PRs.


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org