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

Remove collation info from MSSQL column type #7963

Merged
merged 1 commit into from
Aug 5, 2019
Merged

Remove collation info from MSSQL column type #7963

merged 1 commit into from
Aug 5, 2019

Conversation

villebro
Copy link
Member

@villebro villebro commented Aug 1, 2019

CATEGORY

Choose one

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

SUMMARY

MSSQL columns can have lengthy types due to collations, for example NVARCHAR(50) COLLATE SQL_LATIN1_GENERAL_CP1_CI_AS, exceeding the 32 character limit in Superset. This PR copies logic from the MySQL spec which removes the unnecessary collation info.

TEST PLAN

Test locally

ADDITIONAL INFORMATION

REVIEWERS

@vitorbellini @syazwan0913

@codecov-io
Copy link

codecov-io commented Aug 1, 2019

Codecov Report

Merging #7963 into master will decrease coverage by 0.01%.
The diff coverage is 16.66%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #7963      +/-   ##
=========================================
- Coverage   65.61%   65.6%   -0.02%     
=========================================
  Files         469     469              
  Lines       22381   22387       +6     
  Branches     2432    2432              
=========================================
+ Hits        14686   14687       +1     
- Misses       7574    7579       +5     
  Partials      121     121
Impacted Files Coverage Δ
superset/db_engine_specs/mssql.py 64.28% <16.66%> (-12.99%) ⬇️

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 a09f258...5505b15. Read the comment docs.

Copy link
Member

@mistercrunch mistercrunch left a comment

Choose a reason for hiding this comment

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

LGTM

@villebro villebro merged commit b856666 into apache:master Aug 5, 2019
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.34.0 labels Feb 28, 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/S 🚢 0.34.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tables from MSSQL not showing on Table View
3 participants