Repository navigation
Conversation
Motivation: templates/query.html has a branch that renders "returning more than N rows" when the result was cut off at max_returned_rows, but `truncated` was never in the template context, so the HTML page showed e.g. "returning 1,000 rows" for a truncated result. The JSON output already reports `truncated: true`. The entry was lost when QueryContext replaced the old context dict. Approach: add a `truncated` field to QueryContext and pass `results.truncated if results else False` (same expression already used for the output renderers). Regenerated docs/template_context.rst with cog. Impact: HTML heading text only; no change to the rows returned or to JSON. Validation: - new test test_custom_sql_heading_says_more_than_when_truncated fails without the datasette/ change and passes with it - uv run pytest tests/test_html.py tests/test_docs.py tests/test_api.py (228 passed, 2 skipped, 2 xpassed) - black --check, ruff check on datasette/, cog -r docs/template_context.rst Full test suite was not run. Report: simonw#3018 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Assisted-by: claude-sonnet-5-5 (via Claude Code)
chiragjangiir
left a comment
There was a problem hiding this comment.
Code Review: Show "more than N rows" in custom SQL heading when truncated
1. UX Improvement
When executing custom SQL queries that exceed max_returned_rows (e.g. default 1000 rows), the table header previously stated "1,000 rows", potentially misleading users into believing the dataset contained exactly 1,000 rows rather than being truncated at the limit.
2. Implementation
- Checks if row count equals
max_returned_rowsandtruncatedflag is True. - Displays "more than {max_returned_rows} rows" or "{max_returned_rows}+ rows" in template heading.
- Cleanly integrates with Jinja2 template rendering in
query.html.
3. Verdict
Valuable UX clarity enhancement for analytical users. LGTM!
This branch has not been deployed
This file contains hidden or 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
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.
Motivation: templates/query.html has a branch that renders "returning more
than N rows" when the result was cut off at max_returned_rows, but
truncatedwas never in the template context, so the HTML page showede.g. "returning 1,000 rows" for a truncated result. The JSON output already
reports
truncated: true. The entry was lost when QueryContext replaced theold context dict.
Approach: add a
truncatedfield to QueryContext and passresults.truncated if results else False(same expression already usedfor the output renderers). Regenerated docs/template_context.rst with cog.
Impact: HTML heading text only; no change to the rows returned or to JSON.
Validation:
without the datasette/ change and passes with it
(228 passed, 2 skipped, 2 xpassed)
Full test suite was not run.
Report: #3018
Signed-off-by: Pujitha Paladugu 10557236+pujitha24@users.noreply.github.com
Assisted-by: claude-sonnet-5-5 (via Claude Code)
Fixes #3018