[GitHub] [incubator-superset] DiggidyDave edited a comment on issue #7422: Add "validation_only" queries to the backend

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

DiggidyDave edited a comment on issue #7422: Add "validation_only" queries to the backend
URL: https://github.com/apache/incubator-superset/pull/7422#issuecomment-488318130
 
 
   A couple of high-level points and suggestions for consideration:
   
   1. I'm not a big database guy, but **the term 'validation query'** seems to have an [established meaning](https://www.google.com/search?q=database+validation+query) and I fear this will be unnecessarily overloading a term and possibly causing confusion.  Someone implementing a new type might ask "does my database support validation queries?" and find [this comment on SO](https://stackoverflow.com/a/10684260/3407502) and do the wrong thing.  Consider:
   
       - renaming it to something less ambiguous and more self-documenting
       - adding more comments to the base class documenting the member property and method
   
   2. Tying this to a database's built in implementation is perhaps limiting. It seem unlikely that many (any?) new databases will supply this functionality, but if there was a configurable "per-dialect" or "per database-type" endpoint exposing an abstract API that we define, than lots of options could be plugged in there (including forwarding through to the provider). Consider:
   
       - removing the validation support from `BaseEngineSpec` and having a configurable sql validation endpoint map, which by default has one impl for Hive that forwards to the server.  That way we reserve the right to implement the API to make this feature available to other db types going forward. We might say _But we can always add that later!_, and that is true, but if we stub out the interfaces and make the pattern clear and self-documenting now, then the community might pick up the work to add verification endpoints for other database types. The pattern we pick now is the pattern we are going to live with.

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


With regards,
Apache Git Services

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