New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SQL Lab] Add function names to autocomplete #9012
[SQL Lab] Add function names to autocomplete #9012
Conversation
@@ -320,7 +324,7 @@ def get_quoter(self): | |||
return self.get_dialect().identifier_preparer.quote | |||
|
|||
def get_df( # pylint: disable=too-many-locals | |||
self, sql: str, schema: str, mutator: Optional[Callable] = None | |||
self, sql: str, schema: Optional[str] = None, mutator: Optional[Callable] = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.get_sla_engine
(which is called on line 336) sets schema to None by default so I moved this up a level to make my db query easier (since it's not schema dependent)
superset/db_engine_specs/hive.py
Outdated
:param database: The database to get functions for | ||
:return: A list of function names useable in the database | ||
""" | ||
df = database.get_df("SHOW FUNCTIONS") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove the interim df
variable, i.e.,
return database.get_df("SHOW FUNCTIONS")["tab_name"].tolist()
b7aa583
to
99a14c7
Compare
Codecov Report
@@ Coverage Diff @@
## master #9012 +/- ##
==========================================
+ Coverage 59.15% 59.15% +<.01%
==========================================
Files 367 367
Lines 11682 11686 +4
Branches 2864 2866 +2
==========================================
+ Hits 6910 6913 +3
- Misses 4593 4594 +1
Partials 179 179
Continue to review full report at Codecov.
|
99a14c7
to
6d13b75
Compare
I seem to have just missed the train.. LGTM, but something worth considering: some dbs support defining UDFs per account/database/schema (e.g. Snowflake: |
eek, sorry about that @villebro! I didn't realize some dbs supported UDFs per schema. Do you think it's worth adding that functionality now, given that i've only implemented hive and presto, or should we wait for someone more familiar with Snowflake or other dbs to update the function signature for what they require? |
No panic, I'm pretty excited about getting these in for Snowflake, so can do the necessary changes in a separate PR 👍 |
Sounds good! Feel free to tag me on your PR and i'll review |
Thanks, will do! |
CATEGORY
Choose one
SUMMARY
Adds a new function to
db_engine_specs
for getting function names available in a database. Also implements this function for Presto and Hive.Then passes this result through the database API and adds it to the SQL Lab editor autocomplete
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Hive:
Presto:
TEST PLAN
Test with Hive and Presto and see function autocompletes. Test with mysql and ensure that autocompletes still work
ADDITIONAL INFORMATION
REVIEWERS
to: @dpgaspar @villebro @john-bodley @graceguo-supercat @betodealmeida