-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
[bugfix] Correctly quote table and schema in select_star #8181
Conversation
@villebro would you mind adding a unit test? |
tests/model_tests.py
Outdated
@@ -126,6 +126,28 @@ def test_select_star(self): | |||
) | |||
assert sql.startswith(expected) | |||
|
|||
def test_select_star_with_exotic_names(self): | |||
main_db = get_example_database() |
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.
@villebro I'm not overly familiar with the new example database but I gather the engine is related to the tox environment, i.e., py36-mysql
, py35-sqlite
, etc.
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.
It's my understanding that the example db falls back to SQLALCHEMY_DATABASE_URI
if SQLALCHEMY_EXAMPLES_URI
is undefined, i.e. it should test all engines that have a tox env.
Codecov Report
@@ Coverage Diff @@
## master #8181 +/- ##
=========================================
- Coverage 73.75% 66.2% -7.56%
=========================================
Files 115 479 +364
Lines 12053 22930 +10877
Branches 0 2524 +2524
=========================================
+ Hits 8890 15181 +6291
- Misses 3163 7615 +4452
- Partials 0 134 +134
Continue to review full report at Codecov.
|
* Fix select_star table quoting bug * Add unit test for fully qualified names in select_star * Rename main_db to db * Rename test function
CATEGORY
Choose one
SUMMARY
When clicking
Explore in SQL Lab
from a chart, datasources with a schema are quoted incorrectly.TEST PLAN
Tested locally
ADDITIONAL INFORMATION
REVIEWERS
@squalou @mistercrunch @john-bodley