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

fix(teradata): LIMIT syntax #18240

Merged
merged 42 commits into from
Feb 1, 2022
Merged

fix(teradata): LIMIT syntax #18240

merged 42 commits into from
Feb 1, 2022

Conversation

dmcnulla
Copy link
Contributor

@dmcnulla dmcnulla commented Feb 1, 2022

This PR is a continuation of #15930, all of the context is there.

mccushjack and others added 30 commits June 9, 2021 17:12
updates to set_or_update_query_limit_top
adding apply_limit_to_sql and get_dbapi_mapped_exception
rebase for Teradata fixes for merge for 15930
rebase for issue 15930 and update to new Teradata driver
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
@nytai nytai changed the title Fix unit fix(teradata): LIMIT syntax Feb 1, 2022
@nytai
Copy link
Member

nytai commented Feb 1, 2022

this looks like the one!

Copy link
Member

@nytai nytai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving based on #15930

@codecov
Copy link

codecov bot commented Feb 1, 2022

Codecov Report

Merging #18240 (3216e6a) into master (4ad5ad0) will increase coverage by 0.02%.
The diff coverage is 61.02%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #18240      +/-   ##
==========================================
+ Coverage   66.04%   66.06%   +0.02%     
==========================================
  Files        1591     1591              
  Lines       62398    62547     +149     
  Branches     6283     6283              
==========================================
+ Hits        41210    41324     +114     
- Misses      19567    19602      +35     
  Partials     1621     1621              
Flag Coverage Δ
hive 52.13% <19.11%> (-0.05%) ⬇️
mysql 81.25% <61.02%> (-0.03%) ⬇️
postgres 81.29% <61.02%> (-0.03%) ⬇️
presto 51.97% <19.11%> (-0.05%) ⬇️
python 81.73% <61.02%> (-0.03%) ⬇️
sqlite 80.99% <61.02%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
superset/db_engine_specs/teradata.py 62.75% <61.02%> (-27.25%) ⬇️
superset/views/base_api.py 97.89% <0.00%> (-0.33%) ⬇️
superset/views/core.py 77.59% <0.00%> (-0.15%) ⬇️
superset/constants.py 100.00% <0.00%> (ø)
superset/utils/urls.py 100.00% <0.00%> (ø)
superset/views/utils.py 82.20% <0.00%> (ø)
superset/connectors/druid/models.py 82.99% <0.00%> (ø)
superset/reports/notifications/email.py 100.00% <0.00%> (ø)
superset/queries/saved_queries/api.py 95.28% <0.00%> (+0.04%) ⬆️
superset/connectors/sqla/models.py 88.51% <0.00%> (+0.08%) ⬆️
... and 11 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 4ad5ad0...3216e6a. Read the comment docs.

@nytai nytai merged commit 8c37654 into apache:master Feb 1, 2022
shcoderAlex pushed a commit to casual-precision/superset that referenced this pull request Feb 7, 2022
Co-authored-by: Mccush, Jack <jack.mccush@teradata.com>
Co-authored-by: Jack McCush <33156805+mccushjack@users.noreply.github.com>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: David McNulla <david.mcnulla@teradata.com>
ofekisr pushed a commit to nielsen-oss/superset that referenced this pull request Feb 8, 2022
Co-authored-by: Mccush, Jack <jack.mccush@teradata.com>
Co-authored-by: Jack McCush <33156805+mccushjack@users.noreply.github.com>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: David McNulla <david.mcnulla@teradata.com>
bwang221 pushed a commit to casual-precision/superset that referenced this pull request Feb 10, 2022
Co-authored-by: Mccush, Jack <jack.mccush@teradata.com>
Co-authored-by: Jack McCush <33156805+mccushjack@users.noreply.github.com>
Co-authored-by: Beto Dealmeida <roberto@dealmeida.net>
Co-authored-by: David McNulla <david.mcnulla@teradata.com>
@episkey24
Copy link

Hey, I'm still facing this error with the latest version of Superset. I've installed the sqlalchemy-teradata and the ODBC drivers. The connection seems to be working fine but this LIMIT keyword syntax error in SQL editor is still coming.
Is there a step I'm missing here?
@villebro @dmcnulla @mccushjack @betodealmeida @nytai

@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.5.0 labels Mar 13, 2024
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/L 🚢 1.5.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants