feat: add support for query offset #10010
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dpgaspar
reviewed
Jun 8, 2020
superset/charts/schemas.py
Outdated
| @@ -664,6 +664,7 @@ class ChartDataQueryObjectSchema(Schema): | |||
| row_limit = fields.Integer( | |||
| description='Maximum row count. Default: `config["ROW_LIMIT"]`', | |||
| ) | |||
| row_offset = fields.Integer(description="Number of rows to skip. Default: `0`",) | |||
There was a problem hiding this comment.
consider enforcing row_offset is >0
validate=[Range(min=0, error=_("row_offset must be greater than 0"))])
| self.login(username="admin") | ||
| table = self.get_table_by_name("birth_names") | ||
| request_payload = get_query_context(table.name, table.id, table.type) | ||
| request_payload["queries"][0]["row_limit"] = 5 |
There was a problem hiding this comment.
Would be nice to a couple of simple tests with invalid row_limit and row_offset. negative values, bigger then SAMPLES_ROW_LIMIT
There was a problem hiding this comment.
Added schema validation tests (tests/chart/schema_tests.py) for validation of row_limit and row_offset and e2e tests (tests/chart/api_tests.py) to ensure regular and sample row limits produce correct row counts.
Codecov Report
@@ Coverage Diff @@
## master #10010 +/- ##
==========================================
+ Coverage 65.59% 70.41% +4.82%
==========================================
Files 585 585
Lines 31046 31056 +10
Branches 3277 3277
==========================================
+ Hits 20364 21868 +1504
+ Misses 10501 9077 -1424
+ Partials 181 111 -70
Continue to review full report at Codecov.
|
dpgaspar
approved these changes
Jun 9, 2020
auxten
pushed a commit
to auxten/incubator-superset
that referenced
this pull request
Nov 20, 2020
* feat: add support for query offset * Address comments and add new tests
mistercrunch
added
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SUMMARY
In order to support server side pagination, support for
row_offsetneeds to be added to to complementrow_limitinQueryObject.TEST PLAN
CI + new tests
ADDITIONAL INFORMATION