dolthub/dolt#10866: fix(sql-shell): parse input per character instead of per line by codeaucafe · Pull Request #11202 · dolthub/dolt

codeaucafe · 2026-06-12T07:39:46Z

Summary

The interactive dolt sql shell accumulated input one physical line at a time and decided "is this statement done?" by checking whether the trimmed line ended with ;. That check has no awareness of SQL quotes, comments, or the active DELIMITER, causing four bugs reported in #10860–#10862 and #10865. This PR replaces the blind suffix check with dolt's existing per-character StreamScanner (already used by batch mode) and replaces the single-statement Parse call in the shell callback with a scanner-driven split-and-execute loop.

The shell-side completeness check is wired in via a new opt-in IsComplete callback added to the dolthub/ishell fork (PR: dolthub/ishell#8). go/go.mod contains a replace directive pointing at to PR's head commit: 061915e.

Issues fixed

#10860 — select null; select null; on one line gave syntax error at position 20 near 'select'. The accumulated buffer was fed to sqlparser.Parse once; that parser accepts only a single statement.
#10861 — select '; was treated as complete because the line ends with ;, even though that ; is inside an open string literal.
#10862 — select /* ; and select -- ; were treated as complete for the same reason; the ; is inside an open block or line comment.
#10865 — pressing Enter on an empty line dropped the shell into the -> continuation prompt instead of re-issuing the primary > prompt.

Changes

`go/cmd/dolt/commands/sql_statement_scanner.go`

Add NewStreamScannerWithDelimiter(r io.Reader, delimiter string) constructor. The shell processes one command at a time, so the delimiter set by a prior DELIMITER command must be passed in rather than discovered in-band (as batch mode does when scanning a whole script). NewStreamScanner delegates to this and is unchanged for existing callers.
Add endedAtEOF bool field on StreamScanner, reset at the top of each Scan() call and set when the input runs out before the active delimiter is matched.
Add three accessors exposing parse state to callers: InsideQuote(), InsideBlockComment(), EndedAtEOF().
Add IsShellInputComplete(text, delimiter string) bool. Runs the scanner over the accumulated buffer and returns false when the input ends inside an open quote or block comment, or with a trailing unterminated statement. Blank/whitespace-only input returns true so an empty Enter does not trigger continuation mode (fixes Dolt shell expects more input after empty line whereas MySQL shell returns nothing #10865).

`go/cmd/dolt/commands/sql.go`

Set IsComplete: IsShellInputComplete in UninterpretedConfig. The ishell library calls this after each physical line with the cumulative buffer and the current delimiter; returning true stops accumulation. This is the single change that makes completeness detection SQL-aware (fixes Dolt shell does not ignore delimiters inside quotes #10861, Dolt shell does not ignore delimiters inside comments #10862, Dolt shell expects more input after empty line whereas MySQL shell returns nothing #10865).
Replace the single sqlparser.Parse(query) call in the Uninterpreted callback with a NewStreamScannerWithDelimiter split-and-execute loop. Each statement in the buffer is parsed and executed individually so multiple ;-separated statements on one line each run and print their own result set (fixes Dolt shell has issues parsing multiple statements in the same line #10860). Per-statement errors are printed and execution continues with the next statement, matching MySQL CLI semantics.

Testing

Unit tests (`go/cmd/dolt/commands/sql_statement_scanner_test.go`)

TestIsShellInputComplete — 22 cases covering each issue branch (multi-statement, open quote, open block/line comment, empty input) plus DELIMITER cases.
TestEndedAtEOFFlag — 3 sub-tests verifying the endedAtEOF flag is set on EOF-terminated input and cleared on delimiter-terminated input.

Integration tests (`integration-tests/bats/sql-shell.bats`)

Four new bats tests, one per issue, each driving a real interactive dolt sql session:

sql-shell: multiple statements on one line (#10860)
sql-shell: unclosed quote continues input (#10861)
sql-shell: unclosed comment continues input (#10862)
sql-shell: empty line stays at primary prompt (#10865)

Merge Strategy

merge Add ishell replace directive for dolthub/dolt#11202 doltgresql#2846
- this will allow DoltgreSQL Integration tests in Dolt PR to pass
After Dolt tests pass, merge ishell PR: Add opt-in IsComplete callback to UninterpretedConfig ishell#8
Merge this Dolt PR
clean up replace statements in Doltgres and Dolt go.mod's

Fixes: #10860, #10861, #10862, #10865, #10866

Replace the per-line semicolon suffix check in the interactive SQL shell with the existing per-character StreamScanner, fixing four shell bugs (dolthub#10860, dolthub#10861, dolthub#10862, dolthub#10865). The ishell accumulator decided "done?" by testing whether the trimmed line ended with ";". That test has no awareness of SQL quotes, comments, escapes, or the active DELIMITER, which causes the shell to stop too early (semicolon inside a quote or comment) or never stop (empty Enter triggers continuation mode). Also, a separate assumption that the accumulated buffer is always one statement caused multi-statement lines to fail when fed to a single-statement parser. This consumes the IsComplete callback added to the ishell fork via a go.mod replace directive (codeaucafe/ishell@061915e), pending the upstream ishell PR merge. - Add NewStreamScannerWithDelimiter to seed a fresh scanner with the shell's active delimiter; the shell scans one command at a time so the delimiter set by a prior DELIMITER command must be passed in rather than discovered in-band as batch mode does - Add endedAtEOF bool field on StreamScanner (reset each Scan call, set when input ends before the delimiter) and three accessors: InsideQuote, InsideBlockComment, EndedAtEOF - Add IsShellInputComplete(text, delimiter string) bool which drives the scanner over the accumulated buffer; returns false when input ends inside an open quote or block comment, or with a trailing unterminated statement; blank input returns true (fixes dolthub#10865) - Set IsComplete: IsShellInputComplete in UninterpretedConfig so the completeness decision is SQL-aware (fixes dolthub#10861, dolthub#10862, dolthub#10865) - Replace single sqlparser.Parse(wholeBuffer) call in the shell callback with a NewStreamScannerWithDelimiter split-and-execute loop; each statement in the buffer is parsed and executed individually, printing its own result set (fixes dolthub#10860); per-statement errors print and execution continues, matching MySQL CLI semantics - Add four PTY-driven bats tests via expect scripts, one per issue, that drive a real dolt sql session to verify each fixed behavior Refs: dolthub#10866

angelamayxie · 2026-06-12T18:12:12Z

 github.com/cncf/xds/go v0.0.0-20251210132809-ee656c7534f5/go.mod h1:KdCmV+x/BuvyMxRnYBlmVaq4OLiKW6iRQfvC62cvdkI=
-github.com/cockroachdb/apd/v3 v3.2.3 h1:4Zx+I3R35bFXMnltzmjP79i2cravE4jTRL6ps9Aux80=
-github.com/cockroachdb/apd/v3 v3.2.3/go.mod h1:klXJcjp+FffLTHlhIG69tezTDvdP065naDsHzKhYSqc=
+github.com/codeaucafe/ishell v0.0.0-20260607220657-061915e86568 h1:zAWHQAgb62ygwjQsDscI9btOt/lWEa5NbgMjKL2y1qM=


this should be dolthub/ishell

angelamayxie

please take a look at the failing tests

angelamayxie · 2026-06-12T18:25:34Z

 	gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect
 )

+replace github.com/dolthub/ishell => github.com/codeaucafe/ishell v0.0.0-20260607220657-061915e86568


remove this line. run go get github/com/dolthub/ishell@[commit] to update go.mod and go mod tidy to clean up go.sum

I don't believe this works for forks/outside contributors like myself. I had to do this same thing when making changes in Doltgres that a Dolt Pr I made needed for it to work/be fixed. That's why I did it this way. I'll find those PRs later tonight to show u what I mean.

forgot to add: but I'll also try that just to be sure again

Okay, I think you're right this doesn't work because it's a forked repo, instead of a regular branch in the original repo.

…e/fix/10866-shell-per-character-input-parsing # Conflicts: # go/go.sum

codeaucafe · 2026-06-14T02:38:56Z

@angelamayxie thank you for reviewing. the following CI always fail for my outside contributor PRs:

Benchmark Mini Sysbench...
Benchmark SQL Correctness...
Check for correctness_approved label...
Check for performance_approved label...
Check formatting, Commuters....

Tests Bats Unix Remote/Bats tests is failing at the Install Maven step. For some reason this CI gate has been failing at this step for me for my last 3-4 contributions this year. I'm not entirely sure and never got a reason when I asked and looked into it.

Test Integration with DoltgreSQL/ test-integration... is failing due to it requiring my change to ishell to pass. I had this same issue with before with a change I made to dolt that required me to make use the replace directive pointing in DoltgreSQL in order for this DoltgreSQL integration test suite to pass. This is why I state previously that this replace directive is required for forked contributions like the ones I have to make as an outside contributor. @nicktobey helped me on that change. These are the two PRs for that for reference/example:

I made a PR the same way in Doltgres so we can get this integration test passing: dolthub/doltgresql#2846

codeaucafe · 2026-07-01T06:20:14Z

FYI still working on PR refactor. Made the changes, but reviewing and fixing some issues I found.

Complete the dolt side of moving the SQL statement scanner into the ishell library so interactive shell input is read and scanned in a single streaming pass. Previously dolt re-scanned each accumulated line and then scanned the assembled command again; ishell now scans once and delivers the parsed statements on the Context. Delete the local StreamScanner and consume ishell's instead. The interactive Uninterpreted handler runs the pre-scanned c.Statements directly rather than re-splitting the query. The \edit transform produces new SQL that ishell never saw, so that path still scans its own editor output. Batch mode, debug, and filter-branch keep scanning directly and are repointed at the relocated scanner. - Remove sql_statement_scanner.go and its test; the scanner now lives in github.com/dolthub/ishell - Consume c.Statements in the shell.Uninterpreted handler; keep a per-statement parse-and-execute loop for multi-statement input - Re-scan only the \edit output via ishell.NewStreamScannerWithDelimiter - Repoint execBatchMode, debug.go, and filter-branch.go to ishell.NewStreamScanner and the exported StatementStartLine() - Bump the dolthub/ishell dependency (replace) to the fork branch hosting the relocated scanner and single-pass shell read path - Add PTY bats tests: a large multi-line statement executes correctly, and Ctrl-D at the continuation prompt discards partial input - Tighten the unclosed-quote test to assert on the rendered result cell rather than the echoed input line Refs: dolthub#10866

…e/fix/10866-shell-per-character-input-parsing

Refs: dolthub/ishell#8, dolthub/dolt#11202

codeaucafe · 2026-07-28T06:00:01Z

@angelamayxie I believe this is ready for another review. Most of the new changes were done in ishell like we discussed:

ishell (the meat) — PR #8

Relocated SQL StreamScanner into ishell so input can be scanned where the shell reads it — avoids a circular import.
Rewrote uninterpreted read path for single streaming pass: added uninterpretedReader (adapts line-at-a-time readline into an io.Reader) and readUninterpretedCommand, replacing the old per-line readMultiLinesFunc predicate. Resolved the O(n²) re-scan and the double-scan issues. I wrote a couple unit tests and benchmark tests to verify this, but I wasn't sure if it was overkill to include it or not. If you think its worth I'll make that commit and push it up
Added Context.Statements to give pre-scanned statements to consumers.
Fixed Ctrl-D at continuation prompt to discard the partial statement.
Cleanups: removed a no-op Delimiter() line, reordered a deferred-error check, trimmed stored statement text.
(Uncommitted local: the scaling test + benchmark.)

dolt — PR #11202

Deleted the local scanner, made shell handler consume c.Statements instead of re-scanning; repointed batch/debug/filter-branch to ishell.NewStreamScanner.
Kept the \edit path scanning its own editor output.
Bumped go.mod/go.sum to the pushed ishell fork pseudo-version.
Tests: Ctrl-D discard test, large-input correctness test, tightened the quote-continuation assertion, plus the earlier per-issue tests (Dolt shell has issues parsing multiple statements in the same line #10860/Dolt shell does not ignore delimiters inside quotes #10861/Dolt shell does not ignore delimiters inside comments #10862/Dolt shell expects more input after empty line whereas MySQL shell returns nothing #10865).

doltgresql — PR #2846

go.mod replace bump to the same ishell commit (950b93b)

The \edit transform path splits editor-produced SQL with its own StreamScanner, but left in-stream DELIMITER detection enabled. A DELIMITER line inside edited text would change that scanner's delimiter mid-scan while the shell's own line terminator stayed unchanged, so the split and the shell could disagree about the active delimiter, and the same text behaved differently typed versus edited. Call IgnoreDelimiterStatements() on the scanner, matching how ishell configures the interactive path, so DELIMITER inside edited text is treated as ordinary statement text. Also fix a stale comment in sql-shell-large-input.expect that referenced a Go scaling test in the ishell library that does not exist; the test is a correctness check on single-pass scanning. Refs: dolthub#10866

Add an interactive regression test for the IgnoreDelimiterStatements call on the \edit split scanner. EDITOR is pointed at a fake editor script that replaces the \edit buffer with SQL containing an embedded DELIMITER line, exercising the transform path with text ishell never scanned. The test passes when the edited text produces a syntax error, the same result as typing it at the prompt, and fails explicitly if the embedded DELIMITER is honored and select 42 executes. Verified both ways: the test is green with the fix and red with the IgnoreDelimiterStatements call removed. The fake editor is invoked through sh so no executable bit is required. Refs: dolthub#10866

coffeegoddd added the contribution label Jun 12, 2026

codeaucafe mentioned this pull request Jun 12, 2026

Dolt shell should read input per character/token instead of per line #10866

Open

angelamayxie reviewed Jun 12, 2026

View reviewed changes

Merge branch 'main' of https://github.com/dolthub/dolt into codeaucaf…

b9eacae

…e/fix/10866-shell-per-character-input-parsing # Conflicts: # go/go.sum

codeaucafe mentioned this pull request Jun 14, 2026

Add ishell replace directive for dolthub/dolt#11202 dolthub/doltgresql#2846

Open

codeaucafe force-pushed the codeaucafe/fix/10866-shell-per-character-input-parsing branch from cec63bf to b9eacae Compare June 14, 2026 02:27

angelamayxie reviewed Jun 15, 2026

View reviewed changes

Comment thread go/cmd/dolt/commands/sql_statement_scanner.go Outdated

vishnujayvel mentioned this pull request Jul 12, 2026

Handle \c in the interactive SQL shell #11291

Open

codeaucafe added 2 commits July 27, 2026 22:50

Merge branch 'main' of https://github.com/dolthub/dolt into codeaucaf…

fd00b4d

…e/fix/10866-shell-per-character-input-parsing

codeaucafe added a commit to codeaucafe/doltgresql that referenced this pull request Jul 28, 2026

chore(deps): add ishell replace directive for dolthub/dolt#10866 fix

eef657e

Refs: dolthub/ishell#8, dolthub/dolt#11202

codeaucafe added 2 commits July 31, 2026 18:52

Navigation Menu

Uh oh!

Uh oh!

dolthub/dolt#10866: fix(sql-shell): parse input per character instead of per line - #11202

dolthub/dolt#10866: fix(sql-shell): parse input per character instead of per line#11202
codeaucafe wants to merge 6 commits into
dolthub:mainfrom
codeaucafe:codeaucafe/fix/10866-shell-per-character-input-parsing

codeaucafe commented Jun 12, 2026 •

edited

Loading

Uh oh!

angelamayxie Jun 12, 2026

Uh oh!

angelamayxie left a comment

Uh oh!

angelamayxie Jun 12, 2026

Uh oh!

codeaucafe Jun 12, 2026

Uh oh!

codeaucafe Jun 12, 2026

Uh oh!

angelamayxie Jun 15, 2026

Uh oh!

codeaucafe commented Jun 14, 2026 •

edited

Loading

Uh oh!

Uh oh!

codeaucafe commented Jul 1, 2026 •

edited

Loading

Uh oh!

codeaucafe commented Jul 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

codeaucafe commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Issues fixed

Changes

go/cmd/dolt/commands/sql_statement_scanner.go

go/cmd/dolt/commands/sql.go

Testing

Unit tests (go/cmd/dolt/commands/sql_statement_scanner_test.go)

Integration tests (integration-tests/bats/sql-shell.bats)

Merge Strategy

Uh oh!

angelamayxie Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

angelamayxie left a comment

Choose a reason for hiding this comment

Uh oh!

angelamayxie Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

codeaucafe Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

codeaucafe Jun 12, 2026

Choose a reason for hiding this comment

Uh oh!

angelamayxie Jun 15, 2026

Choose a reason for hiding this comment

Uh oh!

codeaucafe commented Jun 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

codeaucafe commented Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codeaucafe commented Jul 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codeaucafe commented Jun 12, 2026 •

edited

Loading

`go/cmd/dolt/commands/sql_statement_scanner.go`

`go/cmd/dolt/commands/sql.go`

Unit tests (`go/cmd/dolt/commands/sql_statement_scanner_test.go`)

Integration tests (`integration-tests/bats/sql-shell.bats`)

codeaucafe commented Jun 14, 2026 •

edited

Loading

codeaucafe commented Jul 1, 2026 •

edited

Loading