Skip to content
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

Merged
merged 4 commits into from
Sep 5, 2019
Merged

[bugfix] Correctly quote table and schema in select_star #8181

merged 4 commits into from
Sep 5, 2019

Conversation

villebro
Copy link
Member

@villebro villebro commented Sep 5, 2019

CATEGORY

Choose one

  • Bug Fix
  • Enhancement (new features, refinement)
  • Refactor
  • Add tests
  • Build / Development Environment
  • Documentation

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

@villebro villebro added the v0.34 label Sep 5, 2019
@villebro villebro changed the title Fix select_star table quoting bug [bugfix] Correctly quote table and schema in select_star Sep 5, 2019
@john-bodley
Copy link
Member

@villebro would you mind adding a unit test?

@@ -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()
Copy link
Member

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.

Copy link
Member Author

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-io
Copy link

Codecov Report

Merging #8181 into master will decrease coverage by 7.55%.
The diff coverage is n/a.

Impacted file tree graph

@@            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
Impacted Files Coverage Δ
superset/connectors/sqla/models.py 84.38% <ø> (ø) ⬆️
superset/assets/src/components/Checkbox.jsx 100% <0%> (ø)
...ations/deckgl/layers/Polygon/PolygonChartPlugin.js 0% <0%> (ø)
.../assets/src/dashboard/reducers/dashboardFilters.js 88.23% <0%> (ø)
...ets/src/dashboard/components/dnd/DragDroppable.jsx 94.59% <0%> (ø)
...c/visualizations/deckgl/layers/Polygon/Polygon.jsx 0% <0%> (ø)
superset/assets/src/components/EditableTitle.jsx 81.81% <0%> (ø)
superset/assets/src/setup/setupPlugins.js 0% <0%> (ø)
...t/assets/src/components/InfoTooltipWithTrigger.jsx 41.66% <0%> (ø)
.../src/dashboard/components/UndoRedoKeylisteners.jsx 9.52% <0%> (ø)
... and 356 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ae0dc30...7eda040. Read the comment docs.

@villebro villebro merged commit 4e1e54b into apache:master Sep 5, 2019
@mistercrunch mistercrunch added 🍒 0.34.1 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.35.0 labels Feb 28, 2024
cccs-rc pushed a commit to CybercentreCanada/superset that referenced this pull request Mar 6, 2024
* Fix select_star table quoting bug

* Add unit test for fully qualified names in select_star

* Rename main_db to db

* Rename test function
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/XS v0.34 🍒 0.34.1 🚢 0.35.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wrong "backquotes" around datasource when "Explorer in SQL Lab" from a Chart
4 participants