You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "sadpandajoe (via GitHub)" <gi...@apache.org> on 2026/08/14 00:36:57 UTC

[PR] fix(explore): samples endpoint now honors requested row limit [superset]

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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): samples endpoint now honors requested row limit [superset]

Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #43148:
URL: https://github.com/apache/superset/pull/43148#discussion_r3799006947


##########
superset/views/datasource/utils.py:
##########
@@ -45,9 +49,11 @@ def get_limit_clause(page: Optional[int], per_page: Optional[int]) -> dict[str,
 
     if isinstance(page, int) and isinstance(per_page, int):
         limit = int(per_page)
-        if limit < 0 or limit > samples_row_limit:
+        if limit < 0:
             # reset limit value if input is invalid
             limit = samples_row_limit
+        elif limit:
+            limit = apply_max_row_limit(limit)

Review Comment:
   **Suggestion:** Applying `apply_max_row_limit` before calculating the page offset changes the effective page size, but `get_samples` still reports the original `per_page`. When `SQL_MAX_ROW` is lower than the requested value, page 2 is therefore offset by the capped size while the API advertises the uncapped size, causing pages to overlap or return unexpected ranges. Calculate pagination consistently using the effective limit, and expose that effective limit to callers, or reject/report the capped request. [api mismatch]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ Samples pagination reports requested rather than effective page size.
   - ⚠️ Deployments with low `SQL_MAX_ROW` show unexpected page ranges.
   - ⚠️ Explore Samples clients cannot reliably interpret pagination metadata.
   ```
   </details>
   
   [![Fix in Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=11ae993abd574b48af27c64919572ce6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [![Fix in VSCode Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=11ae993abd574b48af27c64919572ce6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <details>
   <summary><b>Prompt for AI Agent 🤖 </b></summary>
   
   ```mdx
   This is a comment left during a code review.
   
   **Path:** superset/views/datasource/utils.py
   **Line:** 55:56
   **Comment:**
   	*Api Mismatch: Applying `apply_max_row_limit` before calculating the page offset changes the effective page size, but `get_samples` still reports the original `per_page`. When `SQL_MAX_ROW` is lower than the requested value, page 2 is therefore offset by the capped size while the API advertises the uncapped size, causing pages to overlap or return unexpected ranges. Calculate pagination consistently using the effective limit, and expose that effective limit to callers, or reject/report the capped request.
   
   Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
   Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
   ```
   </details>
   <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43148&comment_hash=8aee1c3e86743528f296411eb3eda2d39063c8143533e4eff037bb1190360cd5&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43148&comment_hash=8aee1c3e86743528f296411eb3eda2d39063c8143533e4eff037bb1190360cd5&reaction=dislike'>👎</a>



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): samples endpoint now honors requested row limit [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #43148:
URL: https://github.com/apache/superset/pull/43148#issuecomment-5289840847

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) is building environment on [GHA](https://github.com/apache/superset/actions/runs/31773051322) for [648e671](https://github.com/apache/superset/commit/648e671)


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): samples endpoint now honors requested row limit [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #43148:
URL: https://github.com/apache/superset/pull/43148#issuecomment-5289973289

   🎪 [Showtime](https://github.com/mistercrunch/superset-showtime) deployed environment on [GHA](https://github.com/apache/superset/actions/runs/31773051322) for [648e671](https://github.com/apache/superset/commit/648e671)
   
   • **Environment:** http://35.90.5.183:8080 (admin/admin)
   • **Lifetime:** 48h auto-cleanup
   • **Updates:** New commits create fresh environments automatically


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): samples endpoint now honors requested row limit [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43148:
URL: https://github.com/apache/superset/pull/43148#issuecomment-5288004410

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): samples endpoint now honors requested row limit [superset]

Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #43148:
URL: https://github.com/apache/superset/pull/43148#issuecomment-5324138080

   ### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
   
   
   |  Name | Link |
   |:-:|------------------------|
   |<span aria-hidden="true">🔨</span> Latest commit | 55f71b832c0f1579421d7afe2f87c0ab54f270b3 |
   |<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a83f100ea1e230008505dd0 |
   |<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-43148--superset-docs-preview.netlify.app](https://deploy-preview-43148--superset-docs-preview.netlify.app) |
   |<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br />![QR Code](https://app.netlify.com/qr-code/eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJ1cmwiOiJodHRwczovL2RlcGxveS1wcmV2aWV3LTQzMTQ4LS1zdXBlcnNldC1kb2NzLXByZXZpZXcubmV0bGlmeS5hcHAifQ.dp2GjhHKV1EHIGKT0H9lZHhOQzZZOpHgAFj4Q8YYYsQ)<br /><br />_Use your smartphone camera to open QR code link._</details> |
   |<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcvBCYAwDEDRXXI2EC2KuIFjRJNiodpQClbE3e3tH95_wXI6rcAC0MGW-dqP1j5U1GoxZUXhwmh8acScbozhDAV3tuZFG3lWacfEs_M9kXKvgyOieaRRhOD7AQ) |
   ---
   <!-- [superset-docs-preview Preview](https://deploy-preview-43148--superset-docs-preview.netlify.app) -->
   _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): samples endpoint now honors requested row limit [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43148:
URL: https://github.com/apache/superset/pull/43148#issuecomment-5325825324

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): samples endpoint now honors requested row limit [superset]

Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe merged PR #43148:
URL: https://github.com/apache/superset/pull/43148


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): samples endpoint now honors requested row limit [superset]

Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #43148:
URL: https://github.com/apache/superset/pull/43148#issuecomment-5288034124

   ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/43148?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
   :x: Patch coverage is `25.00000%` with `3 lines` in your changes missing coverage. Please review.
   :white_check_mark: Project coverage is 57.27%. Comparing base ([`537f0fd`](https://app.codecov.io/gh/apache/superset/commit/537f0fd2dbe19e6d0fbf136c484150e567b0961f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`648e671`](https://app.codecov.io/gh/apache/superset/commit/648e6712a90b12e11ef0a8614758c7f71eba03f1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   :warning: Report is 1 commits behind head on master.
   
   | [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/43148?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
   |---|---|---|
   | [superset/views/datasource/utils.py](https://app.codecov.io/gh/apache/superset/pull/43148?src=pr&el=tree&filepath=superset%2Fviews%2Fdatasource%2Futils.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvdmlld3MvZGF0YXNvdXJjZS91dGlscy5weQ==) | 25.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/43148?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
   > :exclamation:  There is a different number of reports uploaded between BASE (537f0fd) and HEAD (648e671). Click for more details.
   > 
   > <details><summary>HEAD has 46 uploads less than BASE</summary>
   >
   >| Flag | BASE (537f0fd) | HEAD (648e671) |
   >|------|------|------|
   >|python|38|2|
   >|presto|6|1|
   >|hive|6|1|
   ></details>
   
   <details><summary>Additional details and impacted files</summary>
   
   
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #43148      +/-   ##
   ==========================================
   - Coverage   66.67%   57.27%   -9.41%     
   ==========================================
     Files        2871     2871              
     Lines      163328   163330       +2     
     Branches    37674    37675       +1     
   ==========================================
   - Hits       108904    93543   -15361     
   - Misses      52296    68898   +16602     
   + Partials     2128      889    -1239     
   ```
   
   | [Flag](https://app.codecov.io/gh/apache/superset/pull/43148/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
   |---|---|---|
   | [hive](https://app.codecov.io/gh/apache/superset/pull/43148/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `38.21% <25.00%> (-0.01%)` | :arrow_down: |
   | [mysql](https://app.codecov.io/gh/apache/superset/pull/43148/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [postgres](https://app.codecov.io/gh/apache/superset/pull/43148/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [presto](https://app.codecov.io/gh/apache/superset/pull/43148/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.16% <25.00%> (-0.01%)` | :arrow_down: |
   | [python](https://app.codecov.io/gh/apache/superset/pull/43148/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `40.21% <25.00%> (-19.14%)` | :arrow_down: |
   | [sqlite](https://app.codecov.io/gh/apache/superset/pull/43148/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   | [unit](https://app.codecov.io/gh/apache/superset/pull/43148/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
   </details>
   
   [:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/43148?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).   
   :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
   <details><summary> :rocket: New features to boost your workflow: </summary>
   
   - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
   - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
   </details>


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): samples endpoint now honors requested row limit [superset]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #43148:
URL: https://github.com/apache/superset/pull/43148#discussion_r3786331971


##########
superset/views/datasource/utils.py:
##########
@@ -45,9 +49,11 @@ def get_limit_clause(page: Optional[int], per_page: Optional[int]) -> dict[str,
 
     if isinstance(page, int) and isinstance(per_page, int):
         limit = int(per_page)
-        if limit < 0 or limit > samples_row_limit:
+        if limit < 0:
             # reset limit value if input is invalid
             limit = samples_row_limit
+        elif limit:
+            limit = apply_max_row_limit(limit)

Review Comment:
   `get_limit_clause` applies `apply_max_row_limit(limit)`, which can reduce an explicitly validated `per_page` (SamplesRequestSchema allows up to 10000) when `SQL_MAX_ROW` is configured below the requested value. In that case the endpoint will silently cap the request and also compute `row_offset` using the capped `limit`, which changes pagination semantics (e.g. page 2 offsets by the capped size, not the requested size). Consider aligning request validation with the effective maximum (e.g. reject `per_page` above `SQL_MAX_ROW`), and/or returning/using the effective capped per-page consistently so callers aren’t surprised by a silent override.



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): samples endpoint now honors requested row limit [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43148:
URL: https://github.com/apache/superset/pull/43148#issuecomment-5296923000

   <!-- Bito Reply -->
   The pull request addresses the issue of silent limit overrides in `get_limit_clause` by removing the hard ceiling that previously capped `per_page` at `samples_row_limit`. The updated implementation now honors the requested `per_page` (up to the global `SQL_MAX_ROW` limit via `apply_max_row_limit`) instead of resetting it to `samples_row_limit` when the input exceeds that value. This change ensures that pagination semantics remain consistent with the user's request, as demonstrated by the new regression tests in `tests/unit_tests/views/datasource/utils_test.py` which verify that `per_page` values like 5000 or 10000 are now correctly returned in the `row_limit` field.
   
   **superset/views/datasource/utils.py**
   ```
   if limit < 0:
               # reset limit value if input is invalid
               limit = samples_row_limit
           elif limit:
               limit = apply_max_row_limit(limit)
   ```


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org


Re: [PR] fix(explore): samples endpoint now honors requested row limit [superset]

Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #43148:
URL: https://github.com/apache/superset/pull/43148#issuecomment-5298429849

   AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
   
   <details>
   <summary>
   Bito Usage Guide
   </summary>
   
   **Commands**
         
   Type the following command in the pull request comment and save the comment.
         
   - `/review` - Manually triggers a full AI review.
   
   - `/pause` - Pauses automatic reviews on this pull request.
   - `/resume` - Resumes automatic reviews.
   - `/resolve` - Marks all Bito-posted review comments as resolved.
   - `/abort` - Cancels all in-progress reviews.
   
         
   Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
         
   **Configuration**
   
   This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
         
   **Documentation & Help**
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
   - <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
   
   </details>
   


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org