sadpandajoe opened a new pull request, #43148:
URL: https://github.com/apache/superset/pull/43148
### SUMMARY
The Explore Data panel's row-limit selector (Samples tab) offers
100/500/1k/5k/10k
options, but the `/datasource/samples` endpoint silently capped every
request above
1,000 rows back down to 1,000 — with no error or indication that the
requested limit
was overridden. A user could select 5k or 10k rows and silently receive at
most 1,000,
with no signal anything was capped.
Root cause: `get_limit_clause` (`superset/views/datasource/utils.py`)
unconditionally
reset any `per_page` above `SAMPLES_ROW_LIMIT` (config default 1000) back
down to it,
even though the endpoint's own request schema already validates `per_page`
up to
10,000 and the frontend dropdown offers exactly those higher options. This
fix makes
`get_limit_clause` treat `SAMPLES_ROW_LIMIT` as the *default* (used when no
valid
explicit `per_page` is given) while honoring an explicit, already-validated
`per_page`
via `apply_max_row_limit` — the same mechanism the sibling `/chart/data`
SAMPLES path
(`QueryObjectFactory._process_row_limit`) already uses. This aligns the two
existing
samples code paths and removes the inconsistency between what the endpoint
accepts and
what it actually returns.
Scope: this PR only changes the Samples tab's backend behavior. The Data
panel's
Results tab was investigated and found to already honor its row-limit
selection
correctly under default configuration (it is bounded by the chart's own
configured row
limit, a separate and correctly-working mechanism) — no change needed there.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before/after video evidence to follow in this PR (independent environment QA
pending).
Before: on a dataset with 5,000+ rows, selecting "5k rows" or "10k rows" on
the
Samples tab returns and displays at most 1,000 rows, with the row-limit
dropdown
still showing the higher selection and no indication the request was capped.
After: selecting "5k rows" or "10k rows" on the Samples tab returns and
displays up to
the selected number of rows (bounded only by how many rows actually exist).
### TESTING INSTRUCTIONS
1. In SQL Lab, create a dataset with more than 1,000 rows, e.g.:
```sql
WITH RECURSIVE seq(id) AS (
SELECT 1 UNION ALL SELECT id + 1 FROM seq WHERE id < 5000
)
SELECT id FROM seq
```
2. Build a chart in Explore on that dataset and open the Data panel's
Samples tab.
3. Select "5k rows" (or "10k rows") from the row-limit dropdown.
4. Before this change: the grid returns at most 1,000 rows. After this
change: the
grid returns up to the selected number of rows.
5. Automated regression coverage:
`pytest tests/unit_tests/views/datasource/utils_test.py` — covers the
fixed
behavior (`per_page` of 5000/10000 now honored) plus the preserved
edge-case
behavior (`per_page` of 0 and negative values).
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
