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

[dashboards] Increasing position_json to MEDIUMTEXT for MySQL #5618

Merged

Conversation

john-bodley
Copy link
Member

Most SQLA engines are unbounded for the TEXT type however for MySQL it's restricted to ~ 64 kB which can cause overflow for the dashboards.position_json column for the V2 layout.

This PR ensures that for MySQL engines the TEXT column is changed from TEXT(~ 64 kB) to MEDIUMTEXT (~ 16 MB). All other engines remained unchanged.

to: @graceguo-supercat @michellethomas @mistercrunch @williaster

Copy link

@graceguo-supercat graceguo-supercat left a comment

Choose a reason for hiding this comment

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

LGTM!!



def MediumText():
return Text().with_variant(MEDIUMTEXT(), 'mysql')
Copy link
Member

Choose a reason for hiding this comment

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

oh nice! SQLAlchemy keeps on revealing its secrets :)

@john-bodley john-bodley force-pushed the john-bodley-position-json-medium-text branch from 21f41cf to eec969f Compare August 13, 2018 20:56
@codecov-io
Copy link

Codecov Report

Merging #5618 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5618      +/-   ##
==========================================
+ Coverage   63.62%   63.62%   +<.01%     
==========================================
  Files         359      359              
  Lines       22823    22827       +4     
  Branches     2534     2534              
==========================================
+ Hits        14520    14524       +4     
  Misses       8288     8288              
  Partials       15       15
Impacted Files Coverage Δ
superset/models/core.py 85.19% <100%> (+0.02%) ⬆️
superset/utils.py 89.13% <100%> (+0.05%) ⬆️

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 2685ab4...eec969f. Read the comment docs.

@john-bodley john-bodley merged commit e0c02be into apache:master Aug 13, 2018
john-bodley added a commit to john-bodley/superset that referenced this pull request Aug 13, 2018
@john-bodley john-bodley deleted the john-bodley-position-json-medium-text branch August 14, 2018 00:16
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.28.0 labels Feb 27, 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 🚢 0.28.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants