{{ message }}
Fix "Empty query" when running with cursor past the only ; in play.html#105107
Merged
Conversation
…g `;` `getQueryUnderCursor` builds the substring to run by walking the lexer's token stream and resetting the start position to the offset right after each `;`. If the cursor is past the last `;`, the function would fall through to the "no trailing semicolon" branch and return `all_queries.substring(start)`, where `start` had already been advanced past every whitespace/comment token in the tail — i.e. the returned string was empty and the server reported an "empty query" error. Track the most recently completed `<text-up-to-and-including-`;`>` while walking the tokens. After the loop, if no significant tokens follow the last `;` (detected by `current_query_start === current_offset`), select and return that last completed query instead of the empty tail. This covers the single-query case (`SELECT 1;` with the cursor placed right after `;`, on the next line, or after trailing whitespace/comments) as well as multi-query inputs whose cursor is past every `;`.
Contributor
clickhouse-gh
Bot
added
the
pr-bugfix
Member
Author
|
@groeneai, the |
![clickhouse-gh[bot]](https://avatars.githubusercontent.com/in/928874?s=60&v=4){kind=small}
Contributor
|
Confirmed @alexey-milovidov — both unrelated to this
No action needed on this PR — safe to merge. |
1 task
Member
Author
|
This was fixed by #105146. Let's update the branch. |
With input like `SELECT 1;;` and the cursor placed past the final `;`, the previous fix in this PR still selected the trailing `;` segment as the "last completed query" and returned `;`, so the server still reported "empty query". Track whether the segment up to each `;` contains any content (i.e. `current_query_start` is strictly less than the offset of the `;` token). Only update `last_query_start` / `last_query_end` and only return the segment in the cursor-match branch when it has content. Delimiter-only segments are skipped entirely. This handles `SELECT 1;;`, `SELECT 1;; ` (trailing whitespace after double `;`), `SELECT 1; -- comment\n`, and `SELECT 1;\n\nSELECT 2;\n` with the cursor past every `;`. Bare `;` and empty input still correctly produce no query. Addresses review feedback at #105107 (comment)
1 task
1 task
pamarcos
reviewed
May 19, 2026
pamarcos
left a comment
pamarcos
left a comment
Member
There was a problem hiding this comment.
The code indeed fixes the issue we had. I tested it manually.
Before, with trailing whitespace
Now
but there are some regression cases:
- SELECT 1; ' with cursor at end should try to run the incomplete tail and show a syntax error, not return 1.
- SELECT 1; /* with cursor at end should show an unclosed comment/syntax error, not return 1.
- TRUNCATE TABLE t; ' should not execute TRUNCATE TABLE t when the cursor is in the incomplete tail.
`getQueryUnderCursor` stops receiving tokens when the lexer reports an error. Without checking that tokenization reached the end of the textarea, an incomplete tail such as `SELECT 1; '` could look like whitespace-only trailing content and run the previous completed query. Require the offset to reach the full input before falling back to the last completed query.
Member
I think d59f835 should fix it |
pamarcos
approved these changes
May 20, 2026
The `getQueryUnderCursor` logic regressed once during review (`SELECT 1;;` and incomplete tails like `SELECT 1; '` / `SELECT 1; /*`), so lock the contract with an automated test. There is no JavaScript/WebAssembly runtime in CI, so the browser code in `programs/server/play.html` cannot be run directly. Instead the test in `src/Parsers/tests/gtest_play_get_query_under_cursor.cpp` reproduces the exact token-walking algorithm on top of the real `DB::Lexer` — the same source the Web UI compiles to WebAssembly. It covers the cursor-at-end cases requested in review: `SELECT 1;`, `SELECT 1;;`, `SELECT 1; '`, `SELECT 1; /*`, plus no-semicolon, leading whitespace, trailing comment, and multi-query selection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Member
Author
|
Merged latest
|
Contributor
LLVM Coverage ReportChanged lines: Changed C/C++ lines covered by tests: 50/50 (100.00%) | Lost baseline coverage: none · Uncovered code |
4 tasks
robot-clickhouse-ci-1
added
the
pr-synced-to-cloud
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
4 participants
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.
Fix a bug in the query editor of
programs/server/play.html.If the textarea contains a single query that ends with
;(with optional trailing whitespace, comments, or a newline) and the cursor is placed past that;, pressing Run returned an empty string fromgetQueryUnderCursorand the server reported an "empty query" error instead of running the only query in the textarea.getQueryUnderCursoradvancescurrent_query_startpast every whitespace/comment token at the head of each query, so when the cursor was past the last;, the start had already walked all the way to the end of the input andsubstring(start)was empty.Now we additionally track the start/end of the most recently completed
<text-up-to-and-including-;>while walking the tokens. After the loop, if no significant tokens follow the last;(detected bycurrent_query_start === current_offset), we select and return that last completed query instead of the empty tail.Verified manually for: cursor placed right after
;; cursor on the next line; cursor after trailing whitespace; multiple queries with the cursor past every;; no-semicolon queries (unchanged); leading whitespace (unchanged).Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):
Fixed a bug in the Web UI (
play.html): running a query while the cursor was past the only trailing;reported "empty query" instead of running the query.Documentation entry for user-facing changes
Note
Low Risk
Low risk: isolated client-side query selection logic change in
play.html, primarily affecting how the active query is chosen near trailing;delimiters.Overview
Fixes query selection in
programs/server/play.htmlso Run no longer submits an empty query when the cursor is positioned after a trailing semicolon.getQueryUnderCursornow tracks the most recent completed non-empty query, skips delimiter-only segments (e.g.;;), and if the textarea ends with only whitespace/comments after the last;, it selects/returns that last query instead of an empty tail.Reviewed by Cursor Bugbot for commit 88b84f9. Bugbot is set up for automated code reviews on this repo. Configure here.
Version info
26.6.1.439