-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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(chart): Set max row limit + removed the option to use an empty row limit value #26151
Conversation
Thanks @CorbinBullard for the PR. Is this recreation of a previous PR? If so would you mind linking said PR in the PR description to help provide context/lineage. |
Here is the original PR that was reverted #25579 |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #26151 +/- ##
==========================================
+ Coverage 69.18% 69.19% +0.01%
==========================================
Files 1945 1946 +1
Lines 75970 75920 -50
Branches 8467 8469 +2
==========================================
- Hits 52558 52533 -25
+ Misses 21225 21200 -25
Partials 2187 2187
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
The max value should come from SQL_MAX_ROW and not be a fixed 100,000.
…perset into chart/row-limit/revert
superset-frontend/packages/superset-ui-chart-controls/src/constants.ts
Outdated
Show resolved
Hide resolved
superset-frontend/packages/superset-ui-chart-controls/src/constants.ts
Outdated
Show resolved
Hide resolved
superset-frontend/packages/superset-ui-chart-controls/src/types.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Michael S. Molina <[email protected]>
b11616b
to
5e2be22
Compare
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.
LGTM
…w limit value (#26151) Co-authored-by: Lily Kuang <[email protected]> Co-authored-by: Michael S. Molina <[email protected]>
…w limit value (#26151) Co-authored-by: Lily Kuang <[email protected]> Co-authored-by: Michael S. Molina <[email protected]>
…w limit value (apache#26151) Co-authored-by: Lily Kuang <[email protected]> Co-authored-by: Michael S. Molina <[email protected]>
Using Superset 3.1, I am limited to 100 000 rows even if I set both SQL_MAX_ROW and ROW_LIMIT. Is that the expected behavior? How can I set a higher limit? |
I debugged further. For that, I had to use an already existing chart that had a limit of more than 100 000 prior to the upgrade to 3.1. |
…w limit value (apache#26151) Co-authored-by: Lily Kuang <[email protected]> Co-authored-by: Michael S. Molina <[email protected]>
…w limit value (apache#26151) Co-authored-by: Lily Kuang <[email protected]> Co-authored-by: Michael S. Molina <[email protected]>
SUMMARY
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
After: