From 9d1701508dcaf1ba2c8cd9dc24f426b63ec4d896 Mon Sep 17 00:00:00 2001 From: David Dansby <39511285+codeaucafe@users.noreply.github.com> Date: Fri, 12 Jun 2026 00:14:57 -0700 Subject: [PATCH 1/2] fix(sql-shell): parse input per character instead of per line Replace the per-line semicolon suffix check in the interactive SQL shell with the existing per-character StreamScanner, fixing four shell bugs (#10860, #10861, #10862, #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 #10865) - Set IsComplete: IsShellInputComplete in UninterpretedConfig so the completeness decision is SQL-aware (fixes #10861, #10862, #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 #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: #10866 --- go/cmd/dolt/commands/sql.go | 58 +++++++----- go/cmd/dolt/commands/sql_statement_scanner.go | 62 ++++++++++++- .../commands/sql_statement_scanner_test.go | 93 +++++++++++++++++++ go/go.mod | 2 + go/go.sum | 4 +- .../sql-shell-comment-continuation.expect | 49 ++++++++++ .../bats/sql-shell-empty-enter.expect | 26 ++++++ .../sql-shell-multi-statement-line.expect | 28 ++++++ .../bats/sql-shell-quote-continuation.expect | 30 ++++++ integration-tests/bats/sql-shell.bats | 40 ++++++++ 10 files changed, 367 insertions(+), 25 deletions(-) create mode 100644 integration-tests/bats/sql-shell-comment-continuation.expect create mode 100644 integration-tests/bats/sql-shell-empty-enter.expect create mode 100644 integration-tests/bats/sql-shell-multi-statement-line.expect create mode 100644 integration-tests/bats/sql-shell-quote-continuation.expect diff --git a/go/cmd/dolt/commands/sql.go b/go/cmd/dolt/commands/sql.go index 4a1101d562e..7f5f38ecdf0 100644 --- a/go/cmd/dolt/commands/sql.go +++ b/go/cmd/dolt/commands/sql.go @@ -39,6 +39,8 @@ import ( "golang.org/x/text/transform" "gopkg.in/src-d/go-errors.v1" + eventsapi "github.com/dolthub/eventsapi_schema/dolt/services/eventsapi/v1alpha1" + "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/cmd/dolt/cli/prompt" "github.com/dolthub/dolt/go/cmd/dolt/commands/engine" @@ -50,7 +52,6 @@ import ( "github.com/dolthub/dolt/go/libraries/utils/iohelp" "github.com/dolthub/dolt/go/libraries/utils/osutil" "github.com/dolthub/dolt/go/store/val" - eventsapi "github.com/dolthub/eventsapi_schema/dolt/services/eventsapi/v1alpha1" ) var sqlDocs = cli.CommandDocumentationContent{ @@ -732,6 +733,7 @@ func execShell(sqlCtx *sql.Context, qryist cli.Queryist, format engine.PrintResu LineTerminator: ";", SpecialTerminators: verticalOutputLineTerminators, BackSlashCmds: backSlashCommands, + IsComplete: IsShellInputComplete, } shell := ishell.NewUninterpreted(&shellConf) @@ -849,33 +851,47 @@ func execShell(sqlCtx *sql.Context, qryist cli.Queryist, format engine.PrintResu trackHistory(shell, query+";") } lastSqlCmd = query - sqlStmt, err := sqlparser.Parse(query) - // silently skip empty statements - if err == nil || err == sqlparser.ErrEmpty { - var sqlSch sql.Schema - var rowIter sql.RowIter - sqlSch, rowIter, _, err = processParsedQuery(sqlCtx, query, qryist, sqlStmt) + + // Split into individual statements so multiple on one line each run. + scanner := NewStreamScannerWithDelimiter(strings.NewReader(query), shell.LineTerminator()) + for scanner.Scan() { + stmt := scanner.Text() + if strings.TrimSpace(stmt) == "" { + continue + } + sqlStmt, err := sqlparser.Parse(stmt) + if err == sqlparser.ErrEmpty { + continue + } if err != nil { - verr := formatQueryError("", err) + shell.Println(color.RedString(err.Error())) + continue + } + sqlSch, rowIter, _, execErr := processParsedQuery(sqlCtx, stmt, qryist, sqlStmt) + if execErr != nil { + verr := formatQueryError("", execErr) shell.Println(verr.Verbose()) - } else if rowIter != nil { + continue + } + if rowIter != nil { switch closureFormat { case engine.FormatTabular, engine.FormatVertical: - err = engine.PrettyPrintResultsExtended(sqlCtx, closureFormat, sqlSch, rowIter, pagerEnabled, toggleWarnings, true, binaryAsHex) + if printErr := engine.PrettyPrintResultsExtended(sqlCtx, closureFormat, sqlSch, rowIter, pagerEnabled, toggleWarnings, true, binaryAsHex); printErr != nil { + verr := formatQueryError("", printErr) + shell.Println(verr.Verbose()) + } default: - err = engine.PrettyPrintResults(sqlCtx, closureFormat, sqlSch, rowIter, pagerEnabled, toggleWarnings, true, binaryAsHex) - } - if err != nil { - verr := formatQueryError("", err) - shell.Println(verr.Verbose()) - } - } else { - if _, isUseStmt := sqlStmt.(*sqlparser.Use); isUseStmt { - cli.Println("Database Changed") + if printErr := engine.PrettyPrintResults(sqlCtx, closureFormat, sqlSch, rowIter, pagerEnabled, toggleWarnings, true, binaryAsHex); printErr != nil { + verr := formatQueryError("", printErr) + shell.Println(verr.Verbose()) + } } + } else if _, isUseStmt := sqlStmt.(*sqlparser.Use); isUseStmt { + cli.Println("Database Changed") } - } else { - shell.Println(color.RedString(err.Error())) + } + if scanErr := scanner.Err(); scanErr != nil { + shell.Println(color.RedString(scanErr.Error())) } } diff --git a/go/cmd/dolt/commands/sql_statement_scanner.go b/go/cmd/dolt/commands/sql_statement_scanner.go index bcd9159edde..83a43201860 100755 --- a/go/cmd/dolt/commands/sql_statement_scanner.go +++ b/go/cmd/dolt/commands/sql_statement_scanner.go @@ -18,6 +18,7 @@ import ( "bytes" "fmt" "io" + "strings" "unicode" ) @@ -54,11 +55,22 @@ type StreamScanner struct { fill int lineNum int isEOF bool + // endedAtEOF reports whether the last Scan() ended at EOF before the delimiter. + endedAtEOF bool } -// NewStreamScanner returns a new StreamScanner +// NewStreamScanner returns a new StreamScanner that splits on ";". func NewStreamScanner(r io.Reader) *StreamScanner { - return &StreamScanner{inp: r, buf: make([]byte, pageSize), maxSize: maxStatementBufferBytes, delimiter: []byte(";"), state: new(qState)} + return NewStreamScannerWithDelimiter(r, ";") +} + +// NewStreamScannerWithDelimiter returns a new StreamScanner that splits on +// |delimiter| (defaulting to ";" when empty). +func NewStreamScannerWithDelimiter(r io.Reader, delimiter string) *StreamScanner { + if delimiter == "" { + delimiter = ";" + } + return &StreamScanner{inp: r, buf: make([]byte, pageSize), maxSize: maxStatementBufferBytes, delimiter: []byte(delimiter), state: new(qState)} } type qState struct { @@ -92,6 +104,7 @@ func (qs qState) ignoreDelimiters() bool { func (s *StreamScanner) Scan() bool { s.resetState() + s.endedAtEOF = false if s.i >= s.fill { // initialize buffer @@ -132,6 +145,7 @@ func (s *StreamScanner) Scan() bool { } else if s.isEOF && s.i == s.fill { // token terminates with file s.state.end = s.fill + s.endedAtEOF = true return true } // haven't found delimiter yet, keep reading @@ -376,3 +390,47 @@ func (s *StreamScanner) seekDelimiter() (error, bool) { } return nil, false } + +// InsideQuote reports whether the scanner is currently inside a quoted string. +func (s *StreamScanner) InsideQuote() bool { + return s.state.quoteChar != 0 +} + +// InsideBlockComment reports whether the scanner is currently inside an +// unclosed /* ... */ block comment. +func (s *StreamScanner) InsideBlockComment() bool { + return s.state.insideBlockComment +} + +// EndedAtEOF reports whether the last Scan() ended at EOF before the delimiter. +func (s *StreamScanner) EndedAtEOF() bool { + return s.endedAtEOF +} + +// IsShellInputComplete reports whether |text| forms one or more complete +// statements terminated by |delimiter|. It is false when input ends inside an +// open quote or block comment, or with a trailing unterminated statement; +// whitespace-only input is complete (issue #10865). +func IsShellInputComplete(text string, delimiter string) bool { + if strings.TrimSpace(text) == "" { + return true + } + sc := NewStreamScannerWithDelimiter(strings.NewReader(text), delimiter) + var lastEndedAtEOF bool + var lastBytesNonEmpty bool + for sc.Scan() { + lastEndedAtEOF = sc.EndedAtEOF() + lastBytesNonEmpty = len(strings.TrimSpace(sc.Text())) > 0 + } + if sc.Err() != nil { + // On a scanner error, report complete so the executor surfaces it. + return true + } + if sc.InsideQuote() || sc.InsideBlockComment() { + return false + } + if lastEndedAtEOF && lastBytesNonEmpty { + return false + } + return true +} diff --git a/go/cmd/dolt/commands/sql_statement_scanner_test.go b/go/cmd/dolt/commands/sql_statement_scanner_test.go index f7b291d395b..fff5aa6b9e4 100755 --- a/go/cmd/dolt/commands/sql_statement_scanner_test.go +++ b/go/cmd/dolt/commands/sql_statement_scanner_test.go @@ -288,3 +288,96 @@ in quote'`, }) } } + +// TestEndedAtEOFFlag covers the per-Scan termination distinction that +// IsShellInputComplete relies on. A delimiter-terminated statement must +// leave EndedAtEOF false; an unterminated trailing statement must leave it +// true. +func TestEndedAtEOFFlag(t *testing.T) { + t.Run("delimiter terminated", func(t *testing.T) { + scanner := NewStreamScanner(strings.NewReader("select 1;")) + require.True(t, scanner.Scan()) + assert.False(t, scanner.EndedAtEOF()) + assert.False(t, scanner.Scan()) + }) + t.Run("unterminated EOF", func(t *testing.T) { + scanner := NewStreamScanner(strings.NewReader("select 1")) + require.True(t, scanner.Scan()) + assert.True(t, scanner.EndedAtEOF()) + assert.False(t, scanner.Scan()) + }) + t.Run("flag resets across scans", func(t *testing.T) { + // First statement terminates; second hits EOF unterminated. + scanner := NewStreamScanner(strings.NewReader("select 1; select 2")) + require.True(t, scanner.Scan()) + assert.False(t, scanner.EndedAtEOF()) + require.True(t, scanner.Scan()) + assert.True(t, scanner.EndedAtEOF()) + }) +} + +// TestIsShellInputComplete covers the interactive-shell completion +// predicate. Each case captures one branch of the four bug scenarios +// referenced by dolthub/dolt#10866, plus the DELIMITER and pure-comment +// edges that StreamScanner emits as empty tokens. The delimiter argument +// mirrors the shell's active DELIMITER; most cases use the default ";". +func TestIsShellInputComplete(t *testing.T) { + testcases := []struct { + name string + input string + delimiter string + want bool + }{ + {"empty", "", ";", true}, + {"whitespace only", " ", ";", true}, + {"only newlines", "\n\n", ";", true}, + {"unterminated single statement", "select 1", ";", false}, + {"terminated single statement", "select 1;", ";", true}, + {"terminated multi statement (#10860)", "select null; select null;", ";", true}, + {"trailing unterminated multi", "select 1; select 2", ";", false}, + {"open single quote (#10861)", "select '", ";", false}, + {"open double quote (#10861)", "select \"", ";", false}, + {"open backtick", "select `", ";", false}, + {"semicolon inside open quote (#10861)", "select ';", ";", false}, + {"closed quote then delimiter", "select 'foo';", ";", true}, + {"backslash-escaped quote leaves string open", "select '\\';", ";", false}, + {"doubled backslash closes string", "select '\\\\';", ";", true}, + {"open block comment (#10862)", "select /* ;", ";", false}, + {"closed block comment then delimiter", "select /* x */ null;", ";", true}, + {"line comment with semicolon and no newline", "select -- ;", ";", false}, + {"line comment with newline then terminator", "select -- ;\nnull;", ";", true}, + {"DELIMITER alone", "DELIMITER //", ";", true}, + {"DELIMITER then terminated", "DELIMITER //\nselect 1//", ";", true}, + {"DELIMITER then unterminated", "DELIMITER //\nselect 1", ";", false}, + {"pure line comment input", "-- hello\n", ";", true}, + // A pure block comment with no trailing terminator produces a + // non-empty trailing token at EOF (StreamScanner emits empty tokens + // only for pure line comments). The shell waits for the user to + // type a delimiter, matching MySQL's behavior after a comment. + {"pure block comment input", "/* hello */\n", ";", false}, + + // Non-";" delimiter cases (DELIMITER active in the shell). The + // predicate sees the cumulative buffer with the delimiter still + // present (the callback strips the trailing delimiter only after + // ishell delivers the buffer). Under a custom delimiter, internal ";" + // must not be treated as terminators; only the custom delimiter + // completes a statement. + {"custom delim: trigger body still being typed is incomplete", + "CREATE TRIGGER t BEFORE INSERT ON x FOR EACH ROW\nBEGIN\nSET NEW.v = 1;\nSET NEW.v = 2;\nEND;", "#", false}, + {"custom delim: trigger body completed by custom delimiter", + "CREATE TRIGGER t BEFORE INSERT ON x FOR EACH ROW\nBEGIN\nSET NEW.v = 1;\nSET NEW.v = 2;\nEND; #", "#", true}, + {"custom delim: single statement terminated by custom delimiter", + "select 1 #", "#", true}, + {"custom delim: unterminated single statement", "select 1", "#", false}, + {"custom delim: multi-char // unterminated", "select 1", "//", false}, + {"custom delim: multi-char // terminated", "select 1 //", "//", true}, + {"custom delim: empty stays complete", "", "#", true}, + } + + for _, tt := range testcases { + t.Run(tt.name, func(t *testing.T) { + got := IsShellInputComplete(tt.input, tt.delimiter) + assert.Equal(t, tt.want, got, "input=%q delimiter=%q", tt.input, tt.delimiter) + }) + } +} diff --git a/go/go.mod b/go/go.mod index ba54cb35543..6374da18293 100644 --- a/go/go.mod +++ b/go/go.mod @@ -208,4 +208,6 @@ require ( gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect ) +replace github.com/dolthub/ishell => github.com/codeaucafe/ishell v0.0.0-20260607220657-061915e86568 + go 1.26.2 diff --git a/go/go.sum b/go/go.sum index 6d02aedd82f..e64e62f86b6 100644 --- a/go/go.sum +++ b/go/go.sum @@ -187,6 +187,8 @@ github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDk github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f/go.mod h1:M8M6+tZqaGXZJjfX53e64911xZQV5JYwmTeXPW+k8Sc= github.com/cncf/xds/go v0.0.0-20251210132809-ee656c7534f5 h1:6xNmx7iTtyBRev0+D/Tv1FZd4SCg8axKApyNyRsAt/w= github.com/cncf/xds/go v0.0.0-20251210132809-ee656c7534f5/go.mod h1:KdCmV+x/BuvyMxRnYBlmVaq4OLiKW6iRQfvC62cvdkI= +github.com/codeaucafe/ishell v0.0.0-20260607220657-061915e86568 h1:zAWHQAgb62ygwjQsDscI9btOt/lWEa5NbgMjKL2y1qM= +github.com/codeaucafe/ishell v0.0.0-20260607220657-061915e86568/go.mod h1:ehexgi1mPxRTk0Mok/pADALuHbvATulTh6gzr7NzZto= github.com/colinmarc/hdfs/v2 v2.1.1/go.mod h1:M3x+k8UKKmxtFu++uAZ0OtDU8jR3jnaZIAc6yK4Ue0c= github.com/coreos/go-systemd/v22 v22.3.3-0.20220203105225-a9a7ef127534/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= @@ -216,8 +218,6 @@ github.com/dolthub/go-mysql-server v0.20.1-0.20260505171600-5a9dda3f04ff h1:q3GZ github.com/dolthub/go-mysql-server v0.20.1-0.20260505171600-5a9dda3f04ff/go.mod h1:55n1yslSIZ5uewFbtd82DsYt3f9vUKwnRN5GZJie+nE= github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63 h1:OAsXLAPL4du6tfbBgK0xXHZkOlos63RdKYS3Sgw/dfI= github.com/dolthub/gozstd v0.0.0-20240423170813-23a2903bca63/go.mod h1:lV7lUeuDhH5thVGDCKXbatwKy2KW80L4rMT46n+Y2/Q= -github.com/dolthub/ishell v0.0.0-20260414231531-5f031e3e9037 h1:oIW9HwuWrhxv+4HZxA+QQSKHLqWFyXZ2FmNjUYwkdiM= -github.com/dolthub/ishell v0.0.0-20260414231531-5f031e3e9037/go.mod h1:ehexgi1mPxRTk0Mok/pADALuHbvATulTh6gzr7NzZto= github.com/dolthub/jsonpath v0.0.2-0.20240227200619-19675ab05c71 h1:bMGS25NWAGTEtT5tOBsCuCrlYnLRKpbJVJkDbrTRhwQ= github.com/dolthub/jsonpath v0.0.2-0.20240227200619-19675ab05c71/go.mod h1:2/2zjLQ/JOOSbbSboojeg+cAwcRV0fDLzIiWch/lhqI= github.com/dolthub/sqllogictest/go v0.0.0-20240618184124-ca47f9354216 h1:JWkKRE4EHUcEVQCMRBej8DYxjYjRz/9MdF/NNQh0o70= diff --git a/integration-tests/bats/sql-shell-comment-continuation.expect b/integration-tests/bats/sql-shell-comment-continuation.expect new file mode 100644 index 00000000000..ead18738b53 --- /dev/null +++ b/integration-tests/bats/sql-shell-comment-continuation.expect @@ -0,0 +1,49 @@ +#!/usr/bin/expect -f +# A ; inside an unclosed block or line comment does not end the statement. + +set timeout 10 +spawn dolt sql + +expect { + "> " {} + timeout { send_user "\nFAIL: no initial prompt\n"; exit 1 } +} + +# block comment +send "select /* ;\r" +expect { + -exact "-> " {} + -re "syntax error" { send_user "\nFAIL block: treated as complete\n"; exit 1 } + timeout { send_user "\nFAIL block: no continuation\n"; exit 1 } +} +send "*/ null;\r" +expect { + "NULL" {} + -re "syntax error" { send_user "\nFAIL block: parse failed\n"; exit 1 } + timeout { send_user "\nFAIL block: no NULL row\n"; exit 1 } +} +expect { + "> " {} + timeout { send_user "\nFAIL block: no primary prompt\n"; exit 1 } +} + +# line comment +send "select -- ;\r" +expect { + -exact "-> " {} + -re "syntax error" { send_user "\nFAIL line: treated as complete\n"; exit 1 } + timeout { send_user "\nFAIL line: no continuation\n"; exit 1 } +} +send "null;\r" +expect { + "NULL" {} + -re "syntax error" { send_user "\nFAIL line: parse failed\n"; exit 1 } + timeout { send_user "\nFAIL line: no NULL row\n"; exit 1 } +} +expect { + "> " {} + timeout { send_user "\nFAIL line: no primary prompt\n"; exit 1 } +} + +send "exit;\r" +expect eof diff --git a/integration-tests/bats/sql-shell-empty-enter.expect b/integration-tests/bats/sql-shell-empty-enter.expect new file mode 100644 index 00000000000..dc1ea770788 --- /dev/null +++ b/integration-tests/bats/sql-shell-empty-enter.expect @@ -0,0 +1,26 @@ +#!/usr/bin/expect -f +# An empty Enter stays at the primary prompt instead of entering continuation. + +set timeout 10 +spawn dolt sql + +expect { + "> " {} + timeout { send_user "\nFAIL: no initial prompt\n"; exit 1 } +} + +send "\r" +after 200 +send "select 10865 as ok;\r" +expect { + -re "10865" {} + -re "syntax" { send_user "\nFAIL: syntax error after empty Enter\n"; exit 1 } + timeout { send_user "\nFAIL: no response\n"; exit 1 } +} +expect { + "> " {} + timeout { send_user "\nFAIL: no primary prompt\n"; exit 1 } +} + +send "exit;\r" +expect eof diff --git a/integration-tests/bats/sql-shell-multi-statement-line.expect b/integration-tests/bats/sql-shell-multi-statement-line.expect new file mode 100644 index 00000000000..f6014634176 --- /dev/null +++ b/integration-tests/bats/sql-shell-multi-statement-line.expect @@ -0,0 +1,28 @@ +#!/usr/bin/expect -f +# Multiple ;-separated statements on one line run as separate statements. + +set timeout 10 +spawn dolt sql + +expect { + "> " {} + timeout { send_user "\nFAIL: no initial prompt\n"; exit 1 } +} + +send "select null; select null;\r" +set null_seen 0 +expect { + -re "\\| NULL +\\|" { + incr null_seen + if {$null_seen < 2} { exp_continue } + } + -re "syntax error" { send_user "\nFAIL: syntax error on multi-statement line\n"; exit 1 } + timeout { send_user "\nFAIL: only saw $null_seen NULL row(s)\n"; exit 1 } +} +expect { + "> " {} + timeout { send_user "\nFAIL: no primary prompt\n"; exit 1 } +} + +send "exit;\r" +expect eof diff --git a/integration-tests/bats/sql-shell-quote-continuation.expect b/integration-tests/bats/sql-shell-quote-continuation.expect new file mode 100644 index 00000000000..5b55105060a --- /dev/null +++ b/integration-tests/bats/sql-shell-quote-continuation.expect @@ -0,0 +1,30 @@ +#!/usr/bin/expect -f +# A ; inside an unclosed quote does not end the statement; input continues. + +set timeout 10 +spawn dolt sql + +expect { + "> " {} + timeout { send_user "\nFAIL: no initial prompt\n"; exit 1 } +} + +send "select ';\r" +expect { + -exact "-> " {} + -re "syntax error" { send_user "\nFAIL: treated as complete\n"; exit 1 } + timeout { send_user "\nFAIL: no continuation prompt\n"; exit 1 } +} +send "foo';\r" +expect { + "foo" {} + -re "syntax error" { send_user "\nFAIL: parse failed\n"; exit 1 } + timeout { send_user "\nFAIL: no result\n"; exit 1 } +} +expect { + "> " {} + timeout { send_user "\nFAIL: no primary prompt\n"; exit 1 } +} + +send "exit;\r" +expect eof diff --git a/integration-tests/bats/sql-shell.bats b/integration-tests/bats/sql-shell.bats index 959e72cb1c9..8c6d036b4a2 100644 --- a/integration-tests/bats/sql-shell.bats +++ b/integration-tests/bats/sql-shell.bats @@ -189,6 +189,46 @@ teardown() { $BATS_TEST_DIRNAME/sql-shell-empty-prompt.expect } +# bats test_tags=no_lambda +@test "sql-shell: multiple statements on one line (#10860)" { + skiponwindows "Need to install expect and make this script work on windows." + if [ "$SQL_ENGINE" = "remote-engine" ]; then + skip "Presently sql command will not connect to remote server due to lack of lock file where there are not DBs." + fi + run expect $BATS_TEST_DIRNAME/sql-shell-multi-statement-line.expect + [ "$status" -eq 0 ] +} + +# bats test_tags=no_lambda +@test "sql-shell: unclosed quote continues input (#10861)" { + skiponwindows "Need to install expect and make this script work on windows." + if [ "$SQL_ENGINE" = "remote-engine" ]; then + skip "Presently sql command will not connect to remote server due to lack of lock file where there are not DBs." + fi + run expect $BATS_TEST_DIRNAME/sql-shell-quote-continuation.expect + [ "$status" -eq 0 ] +} + +# bats test_tags=no_lambda +@test "sql-shell: unclosed comment continues input (#10862)" { + skiponwindows "Need to install expect and make this script work on windows." + if [ "$SQL_ENGINE" = "remote-engine" ]; then + skip "Presently sql command will not connect to remote server due to lack of lock file where there are not DBs." + fi + run expect $BATS_TEST_DIRNAME/sql-shell-comment-continuation.expect + [ "$status" -eq 0 ] +} + +# bats test_tags=no_lambda +@test "sql-shell: empty line stays at primary prompt (#10865)" { + skiponwindows "Need to install expect and make this script work on windows." + if [ "$SQL_ENGINE" = "remote-engine" ]; then + skip "Presently sql command will not connect to remote server due to lack of lock file where there are not DBs." + fi + run expect $BATS_TEST_DIRNAME/sql-shell-empty-enter.expect + [ "$status" -eq 0 ] +} + @test "sql-shell: works with ANSI_QUOTES SQL mode" { if [ $SQL_ENGINE = "remote-engine" ]; then skip "Presently sql command will not connect to remote server due to lack of lock file where there are not DBs." From c752a0faf55e78f96416dae9ff4596d3380bcb3c Mon Sep 17 00:00:00 2001 From: David Dansby <39511285+codeaucafe@users.noreply.github.com> Date: Mon, 27 Jul 2026 22:50:14 -0700 Subject: [PATCH 2/2] refactor(sql-shell): consume statements scanned by ishell 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: #10866 --- go/cmd/dolt/commands/debug.go | 3 +- go/cmd/dolt/commands/filter-branch.go | 3 +- go/cmd/dolt/commands/sql.go | 32 +- go/cmd/dolt/commands/sql_statement_scanner.go | 436 ------------------ .../commands/sql_statement_scanner_test.go | 383 --------------- go/go.mod | 2 +- go/go.sum | 4 +- .../bats/sql-shell-ctrld-continuation.expect | 34 ++ .../bats/sql-shell-large-input.expect | 37 ++ .../bats/sql-shell-quote-continuation.expect | 13 +- integration-tests/bats/sql-shell.bats | 20 + 11 files changed, 126 insertions(+), 841 deletions(-) delete mode 100755 go/cmd/dolt/commands/sql_statement_scanner.go delete mode 100755 go/cmd/dolt/commands/sql_statement_scanner_test.go create mode 100644 integration-tests/bats/sql-shell-ctrld-continuation.expect create mode 100644 integration-tests/bats/sql-shell-large-input.expect diff --git a/go/cmd/dolt/commands/debug.go b/go/cmd/dolt/commands/debug.go index 2a9ed6bcf41..b5a3c323dac 100644 --- a/go/cmd/dolt/commands/debug.go +++ b/go/cmd/dolt/commands/debug.go @@ -31,6 +31,7 @@ import ( "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/analyzer" + "github.com/dolthub/ishell" "github.com/pkg/profile" "github.com/sirupsen/logrus" textunicode "golang.org/x/text/encoding/unicode" @@ -391,7 +392,7 @@ func debugAnalyze(ctx *sql.Context, tempDir string, sqlEng *engine.SqlEngine, sq cli.CliErr = origCliErr }() - scanner := NewStreamScanner(sqlFile) + scanner := ishell.NewStreamScanner(sqlFile) var query string for scanner.Scan() { if fileReadProg != nil { diff --git a/go/cmd/dolt/commands/filter-branch.go b/go/cmd/dolt/commands/filter-branch.go index 80c23d5e8d8..e8a0925b678 100644 --- a/go/cmd/dolt/commands/filter-branch.go +++ b/go/cmd/dolt/commands/filter-branch.go @@ -23,6 +23,7 @@ import ( sqle "github.com/dolthub/go-mysql-server" "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/analyzer" + "github.com/dolthub/ishell" "github.com/dolthub/dolt/go/cmd/dolt/cli" "github.com/dolthub/dolt/go/cmd/dolt/commands/engine" @@ -281,7 +282,7 @@ func processFilterQuery(ctx context.Context, dEnv *env.DoltEnv, root doltdb.Root return nil, err } - scanner := NewStreamScanner(strings.NewReader(query)) + scanner := ishell.NewStreamScanner(strings.NewReader(query)) if err != nil { return nil, err } diff --git a/go/cmd/dolt/commands/sql.go b/go/cmd/dolt/commands/sql.go index 7f5f38ecdf0..807860caa47 100644 --- a/go/cmd/dolt/commands/sql.go +++ b/go/cmd/dolt/commands/sql.go @@ -625,7 +625,7 @@ func validateSqlArgs(apr *argparser.ArgParseResults) error { // execBatchMode runs all the queries in the input reader func execBatchMode(ctx *sql.Context, qryist cli.Queryist, input io.Reader, continueOnErr bool, format engine.PrintResultFormat, binaryAsHex bool) error { - scanner := NewStreamScanner(input) + scanner := ishell.NewStreamScanner(input) var query string for scanner.Scan() { // The session we get is wrapped in a command begin/end block. @@ -648,7 +648,7 @@ func execBatchMode(ctx *sql.Context, qryist cli.Queryist, input io.Reader, conti if err == sqlparser.ErrEmpty { continue } else if err != nil { - err = buildBatchSqlErr(scanner.state.statementStartLine, query, err) + err = buildBatchSqlErr(scanner.StatementStartLine(), query, err) if !continueOnErr { return err } else { @@ -660,7 +660,7 @@ func execBatchMode(ctx *sql.Context, qryist cli.Queryist, input io.Reader, conti ctx.SetQueryTime(time.Now()) sqlSch, rowIter, _, err := processParsedQuery(ctx, query, qryist, sqlStatement) if err != nil { - err = buildBatchSqlErr(scanner.state.statementStartLine, query, err) + err = buildBatchSqlErr(scanner.StatementStartLine(), query, err) if !continueOnErr { return err } else { @@ -679,7 +679,7 @@ func execBatchMode(ctx *sql.Context, qryist cli.Queryist, input io.Reader, conti } err = engine.PrettyPrintResults(ctx, format, sqlSch, rowIter, false, false, false, binaryAsHex) if err != nil { - err = buildBatchSqlErr(scanner.state.statementStartLine, query, err) + err = buildBatchSqlErr(scanner.StatementStartLine(), query, err) if !continueOnErr { return err } else { @@ -691,7 +691,7 @@ func execBatchMode(ctx *sql.Context, qryist cli.Queryist, input io.Reader, conti } if err := scanner.Err(); err != nil { - return buildBatchSqlErr(scanner.state.statementStartLine, query, err) + return buildBatchSqlErr(scanner.StatementStartLine(), query, err) } return nil @@ -733,7 +733,6 @@ func execShell(sqlCtx *sql.Context, qryist cli.Queryist, format engine.PrintResu LineTerminator: ";", SpecialTerminators: verticalOutputLineTerminators, BackSlashCmds: backSlashCommands, - IsComplete: IsShellInputComplete, } shell := ishell.NewUninterpreted(&shellConf) @@ -846,16 +845,26 @@ func execShell(sqlCtx *sql.Context, qryist cli.Queryist, format engine.PrintResu } } } else { + // statements holds the individual SQL statements to run. For + // normal input, ishell already scanned the typed input into + // complete statements (single pass). The \edit transform produces + // new SQL that ishell never saw, so split it here. + statements := c.Statements if cmdType == TransformCommand { query = newQuery trackHistory(shell, query+";") + statements = nil + scanner := ishell.NewStreamScannerWithDelimiter(strings.NewReader(query), shell.LineTerminator()) + for scanner.Scan() { + statements = append(statements, scanner.Text()) + } + if scanErr := scanner.Err(); scanErr != nil { + shell.Println(color.RedString(scanErr.Error())) + } } lastSqlCmd = query - // Split into individual statements so multiple on one line each run. - scanner := NewStreamScannerWithDelimiter(strings.NewReader(query), shell.LineTerminator()) - for scanner.Scan() { - stmt := scanner.Text() + for _, stmt := range statements { if strings.TrimSpace(stmt) == "" { continue } @@ -890,9 +899,6 @@ func execShell(sqlCtx *sql.Context, qryist cli.Queryist, format engine.PrintResu cli.Println("Database Changed") } } - if scanErr := scanner.Err(); scanErr != nil { - shell.Println(color.RedString(scanErr.Error())) - } } nextPrompt, multiPrompt = postCommandUpdate(sqlCtx, qryist) diff --git a/go/cmd/dolt/commands/sql_statement_scanner.go b/go/cmd/dolt/commands/sql_statement_scanner.go deleted file mode 100755 index 83a43201860..00000000000 --- a/go/cmd/dolt/commands/sql_statement_scanner.go +++ /dev/null @@ -1,436 +0,0 @@ -// Copyright 2020 Dolthub, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package commands - -import ( - "bytes" - "fmt" - "io" - "strings" - "unicode" -) - -const maxStatementBufferBytes = 100*1024*1024 + 4096 -const pageSize = 2 << 11 - -const ( - sQuote byte = '\'' - dQuote = '"' - backslash = '\\' - backtick = '`' - hyphen = '-' - asterisk = '*' - slash = '/' - newline = '\n' -) - -const delimPrefixLen = 10 - -var delimPrefix = []byte("delimiter ") - -// StreamScanner is an iterator that reads bytes from |inp| until either -// (1) we match a DELIMITER statement, (2) we match the |delimiter| token, -// or (3) we EOF the file. After each Scan() call, the valid token will -// span from the buffer beginning to |state.end|. -type StreamScanner struct { - inp io.Reader - err error - state *qState - buf []byte - delimiter []byte - maxSize int - i int // current byte pointer - fill int - lineNum int - isEOF bool - // endedAtEOF reports whether the last Scan() ended at EOF before the delimiter. - endedAtEOF bool -} - -// NewStreamScanner returns a new StreamScanner that splits on ";". -func NewStreamScanner(r io.Reader) *StreamScanner { - return NewStreamScannerWithDelimiter(r, ";") -} - -// NewStreamScannerWithDelimiter returns a new StreamScanner that splits on -// |delimiter| (defaulting to ";" when empty). -func NewStreamScannerWithDelimiter(r io.Reader, delimiter string) *StreamScanner { - if delimiter == "" { - delimiter = ";" - } - return &StreamScanner{inp: r, buf: make([]byte, pageSize), maxSize: maxStatementBufferBytes, delimiter: []byte(delimiter), state: new(qState)} -} - -type qState struct { - start int - end int // token end, usually i - len(delimiter) - numConsecutiveBackslashes int // the number of consecutive backslashes encountered - numConsecutiveDelimiterMatches int // the consecutive number of characters that have been matched to the delimiter - statementStartLine int - lineCommentStart int - quoteChar byte // the opening quote character of the current quote being parsed, or 0 if the current parse location isn't inside a quoted string - lastChar byte // the last character parsed - ignoreNextChar bool // whether to ignore the next character - seenNonWhitespaceChar bool // whether we have encountered a non-whitespace character since we returned the last token - insideBlockComment bool - insideLineComment bool -} - -func (qs qState) insideComment() bool { - return qs.insideLineComment || qs.insideBlockComment -} - -func (qs qState) insideQuote() bool { - return qs.quoteChar != 0 -} - -// ignoreDelimiters returns if delimiters should be ignored. If inside a comment or a quote, delimiters, including -// comment delimiters, should be ignored -func (qs qState) ignoreDelimiters() bool { - return qs.insideComment() || qs.insideQuote() -} - -func (s *StreamScanner) Scan() bool { - s.resetState() - s.endedAtEOF = false - - if s.i >= s.fill { - // initialize buffer - if err := s.read(); err != nil { - s.err = err - return false - } - } - - if s.isEOF || s.i == s.fill { - // no token - return false - } - - // discard leading whitespace - if !s.skipWhitespace() { - return false - } - s.truncate() - - s.state.statementStartLine = s.lineNum + 1 - - if err, ok := s.isDelimiterExpr(); err != nil { - s.err = err - return false - } else if ok { - // empty token acks DELIMITER - return true - } - - for { - if err, ok := s.seekDelimiter(); err != nil { - s.err = err - return false - } else if ok { - // delimiter found, scanner holds valid token state - return true - } else if s.isEOF && s.i == s.fill { - // token terminates with file - s.state.end = s.fill - s.endedAtEOF = true - return true - } - // haven't found delimiter yet, keep reading - if err := s.read(); err != nil { - s.err = err - return false - } - } -} - -func (s *StreamScanner) skipWhitespace() bool { - for { - if s.i >= s.fill { - if err := s.read(); err != nil { - s.err = err - return false - } - } - if s.isEOF { - return true - } - if !unicode.IsSpace(rune(s.buf[s.i])) { - break - } - if s.buf[s.i] == '\n' { - s.lineNum++ - } - s.i++ - } - return true -} - -func (s *StreamScanner) truncate() { - // copy size should be 4k or less - s.state.start = s.i - s.state.end = s.i -} - -func (s *StreamScanner) resetState() { - s.state = &qState{} -} - -func (s *StreamScanner) read() error { - if s.fill >= s.maxSize { - // if script exceeds buffer that's OK, if - // a single query exceeds buffer that's not OK - if s.state.start == 0 { - return fmt.Errorf("exceeded max query size") - } - // discard previous queries, resulting buffer will start - // at the current |start| - s.fill -= s.state.start - s.i -= s.state.start - s.state.end = s.state.start - copy(s.buf[:], s.buf[s.state.start:]) - s.state.start = 0 - return s.read() - } - if s.fill == len(s.buf) { - newBufSize := min(len(s.buf)*2, s.maxSize) - newBuf := make([]byte, newBufSize) - copy(newBuf, s.buf) - s.buf = newBuf - } - n, err := s.inp.Read(s.buf[s.fill:]) - if err == io.EOF { - s.isEOF = true - } else if err != nil { - return err - } - s.fill += n - return nil -} - -func (s *StreamScanner) Err() error { - return s.err -} - -func (s *StreamScanner) Bytes() []byte { - return s.buf[s.state.start:s.state.end] -} - -// Text returns the most recent token generated by a call to [Scanner.Scan] -// as a newly allocated string holding its bytes. -func (s *StreamScanner) Text() string { - return string(s.Bytes()) -} - -func (s *StreamScanner) isDelimiterExpr() (error, bool) { - if s.i == 0 && s.fill-s.i < delimPrefixLen { - // need to see first |delimPrefixLen| characters - if err := s.read(); err != nil { - s.err = err - return err, false - } - } - - // valid delimiter state machine check - // "DELIMITER " -> 0+ spaces -> -> 1 space - if s.fill-s.i >= delimPrefixLen && bytes.EqualFold(s.buf[s.i:s.i+delimPrefixLen], delimPrefix) { - delimTokenIdx := s.i - s.i += delimPrefixLen - if !s.skipWhitespace() { - return nil, false - } - if s.isEOF { - // invalid delimiter - s.i = delimTokenIdx - return nil, false - } - delimStart := s.i - for ; !s.isEOF && !unicode.IsSpace(rune(s.buf[s.i])); s.i++ { - if s.i >= s.fill { - if err := s.read(); err != nil { - s.err = err - return err, false - } - } - } - delimEnd := s.i - s.delimiter = make([]byte, delimEnd-delimStart) - copy(s.delimiter, s.buf[delimStart:delimEnd]) - - // discard delimiter token, return empty token - s.truncate() - return nil, true - } - return nil, false -} - -func (s *StreamScanner) seekDelimiter() (error, bool) { - for ; s.i < s.fill; s.i++ { - i := s.i - if !s.state.ignoreNextChar { - // this doesn't handle unicode characters correctly and will break on some things, but it's only used for line - // number reporting. - if !s.state.seenNonWhitespaceChar && !unicode.IsSpace(rune(s.buf[i])) { - s.state.seenNonWhitespaceChar = true - } - - // check if we've matched the delimiter string - if !s.state.ignoreDelimiters() && s.buf[i] == s.delimiter[s.state.numConsecutiveDelimiterMatches] { - s.state.numConsecutiveDelimiterMatches++ - if s.state.numConsecutiveDelimiterMatches == len(s.delimiter) { - s.state.end = s.i - len(s.delimiter) + 1 - s.i++ - s.state.lastChar = 0 - return nil, true - } - s.state.lastChar = s.buf[i] - continue - } else { - s.state.numConsecutiveDelimiterMatches = 0 - } - - switch s.buf[i] { - case newline: - s.lineNum++ - if s.state.insideLineComment { - s.state.insideLineComment = false - // if the entire statement is a line comment, truncate and return as empty - if s.state.start == s.state.lineCommentStart { - s.i++ - s.truncate() - s.state.lastChar = 0 - return nil, true - } - } - case hyphen: - // If inside quote or already inside comment, ignore. Otherwise, if previous character is also a hyphen, - // ie "--", begin line comment. - if !s.state.ignoreDelimiters() && s.state.lastChar == hyphen { - s.state.lineCommentStart = i - 1 - s.state.insideLineComment = true - } - case asterisk: - // If inside quote or already inside comment, ignore. Otherwise, if previous character is a slash, ie - // "/*", begin block comment. - if !s.state.ignoreDelimiters() && s.state.lastChar == slash { - s.state.insideBlockComment = true - } - case slash: - // If previous character is an asterisk, ie "*/", end block comment. - if s.state.insideBlockComment && s.state.lastChar == asterisk { - s.state.insideBlockComment = false - } - case backslash: - s.state.numConsecutiveBackslashes++ - case sQuote, dQuote, backtick: - // ignore quotes inside comments - if s.state.insideComment() { - break - } - - prevNumConsecutiveBackslashes := s.state.numConsecutiveBackslashes - s.state.numConsecutiveBackslashes = 0 - - // escaped quote character - if s.state.lastChar == backslash && prevNumConsecutiveBackslashes%2 == 1 { - break - } - - // currently in a quoted string - if s.state.insideQuote() { - if i+1 >= s.fill { - // require lookahead or EOF - if err := s.read(); err != nil { - return err, false - } - } - - // end quote or two consecutive quote characters (a form of escaping quote chars) - if s.state.quoteChar == s.buf[i] { - var nextChar byte = 0 - if i+1 < s.fill { - nextChar = s.buf[i+1] - } - - if nextChar == s.state.quoteChar { - // escaped quote. skip the next character - s.state.ignoreNextChar = true - } else { - // end quote - s.state.quoteChar = 0 - } - } - - // embedded quote ('"' or "'") - break - } - - // open quote - s.state.quoteChar = s.buf[i] - default: - s.state.numConsecutiveBackslashes = 0 - } - } else { - s.state.ignoreNextChar = false - } - - s.state.lastChar = s.buf[i] - } - return nil, false -} - -// InsideQuote reports whether the scanner is currently inside a quoted string. -func (s *StreamScanner) InsideQuote() bool { - return s.state.quoteChar != 0 -} - -// InsideBlockComment reports whether the scanner is currently inside an -// unclosed /* ... */ block comment. -func (s *StreamScanner) InsideBlockComment() bool { - return s.state.insideBlockComment -} - -// EndedAtEOF reports whether the last Scan() ended at EOF before the delimiter. -func (s *StreamScanner) EndedAtEOF() bool { - return s.endedAtEOF -} - -// IsShellInputComplete reports whether |text| forms one or more complete -// statements terminated by |delimiter|. It is false when input ends inside an -// open quote or block comment, or with a trailing unterminated statement; -// whitespace-only input is complete (issue #10865). -func IsShellInputComplete(text string, delimiter string) bool { - if strings.TrimSpace(text) == "" { - return true - } - sc := NewStreamScannerWithDelimiter(strings.NewReader(text), delimiter) - var lastEndedAtEOF bool - var lastBytesNonEmpty bool - for sc.Scan() { - lastEndedAtEOF = sc.EndedAtEOF() - lastBytesNonEmpty = len(strings.TrimSpace(sc.Text())) > 0 - } - if sc.Err() != nil { - // On a scanner error, report complete so the executor surfaces it. - return true - } - if sc.InsideQuote() || sc.InsideBlockComment() { - return false - } - if lastEndedAtEOF && lastBytesNonEmpty { - return false - } - return true -} diff --git a/go/cmd/dolt/commands/sql_statement_scanner_test.go b/go/cmd/dolt/commands/sql_statement_scanner_test.go deleted file mode 100755 index fff5aa6b9e4..00000000000 --- a/go/cmd/dolt/commands/sql_statement_scanner_test.go +++ /dev/null @@ -1,383 +0,0 @@ -// Copyright 2020 Dolthub, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package commands - -import ( - "strings" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestScanStatements(t *testing.T) { - type testcase struct { - input string - statements []string - lineNums []int - } - - // Some of these include malformed input (e.g. strings that aren't properly terminated) - testcases := []testcase{ - { - input: `insert into foo values (";;';'");`, - statements: []string{ - `insert into foo values (";;';'")`, - }, - }, - { - input: `select ''';;'; select ";\;"`, - statements: []string{ - `select ''';;'`, - `select ";\;"`, - }, - }, - { - input: `select ''';;'; select ";\;`, - statements: []string{ - `select ''';;'`, - `select ";\;`, - }, - }, - { - input: `select ''';;'; select ";\; -;`, - statements: []string{ - `select ''';;'`, - `select ";\; -;`, - }, - }, - { - input: `select '\\'''; select '";";'; select 1`, - statements: []string{ - `select '\\'''`, - `select '";";'`, - `select 1`, - }, - }, - { - input: `select '\\''; select '";";'; select 1`, - statements: []string{ - `select '\\''; select '";"`, - `'; select 1`, - }, - }, - { - input: `insert into foo values(''); select 1`, - statements: []string{ - `insert into foo values('')`, - `select 1`, - }, - }, - { - input: `insert into foo values('''); select 1`, - statements: []string{ - `insert into foo values('''); select 1`, - }, - }, - { - input: `insert into foo values(''''); select 1`, - statements: []string{ - `insert into foo values('''')`, - `select 1`, - }, - }, - { - input: `insert into foo values(""); select 1`, - statements: []string{ - `insert into foo values("")`, - `select 1`, - }, - }, - { - input: `insert into foo values("""); select 1`, - statements: []string{ - `insert into foo values("""); select 1`, - }, - }, - { - input: `insert into foo values(""""); select 1`, - statements: []string{ - `insert into foo values("""")`, - `select 1`, - }, - }, - { - input: `select '\''; select "hell\"o"`, - statements: []string{ - `select '\''`, - `select "hell\"o"`, - }, - }, - { - input: `select * from foo; select baz from foo; -select -a from b; select 1`, - statements: []string{ - "select * from foo", - "select baz from foo", - "select\na from b", - "select 1", - }, - lineNums: []int{ - 1, 1, 2, 3, - }, - }, - { - input: "create table dumb (`hell\\`o;` int primary key);", - statements: []string{ - "create table dumb (`hell\\`o;` int primary key)", - }, - }, - { - input: "create table dumb (`hell``o;` int primary key); select \n" + - "baz from foo;\n" + - "\n" + - "select\n" + - "a from b; select 1\n\n", - statements: []string{ - "create table dumb (`hell``o;` int primary key)", - "select \nbaz from foo", - "select\na from b", - "select 1", - }, - lineNums: []int{ - 1, 1, 4, 5, - }, - }, - { - input: `insert into foo values ('a', "b;", 'c;;"" -'); update foo set baz = bar, -qux = '"hello"""' where xyzzy = ";;';'"; - - -create table foo (a int not null default ';', -primary key (a));`, - statements: []string{ - `insert into foo values ('a', "b;", 'c;;"" -')`, - `update foo set baz = bar, -qux = '"hello"""' where xyzzy = ";;';'"`, - `create table foo (a int not null default ';', -primary key (a))`, - }, - lineNums: []int{ - 1, 2, 6, - }, - }, - { - input: `DELIMITER | -insert into foo values (1,2,3)|`, - statements: []string{ - "", - "insert into foo values (1,2,3)", - }, - lineNums: []int{1, 2}, - }, - { - // https://github.com/dolthub/dolt/issues/10828 - input: `-- comment asdfasdf - delimiter // - select current_user() //`, - statements: []string{ - "", - "", - "select current_user()", - }, - lineNums: []int{1, 2, 3}, - }, - { - // https://github.com/dolthub/dolt/issues/8495 - input: strings.Repeat(" ", 4096) + `insert into foo values (1,2,3)`, - statements: []string{ - "insert into foo values (1,2,3)", - }, - lineNums: []int{1, 2}, - }, - { - input: "DELIMITER" + strings.Repeat(" ", 4096) + `| -insert into foo values (1,2,3)|`, - statements: []string{ - "", - "insert into foo values (1,2,3)", - }, - lineNums: []int{1, 2}, - }, - { - // https://github.com/dolthub/dolt/issues/10694 - input: `-- ' --- can have intermediate comments -CALL dolt_commit('-m', 'message', '--allow-empty'); -CALL dolt_checkout('main');`, - statements: []string{ - "", "", - "CALL dolt_commit('-m', 'message', '--allow-empty')", - "CALL dolt_checkout('main')", - }, - lineNums: []int{1, 2, 3, 4}, - }, - { - input: `/* block comment with lone quote ' -*/ --- can have intermediate comments -CALL dolt_commit('-m', 'message', '--allow-empty'); -CALL dolt_checkout('main');`, - statements: []string{ - `/* block comment with lone quote ' -*/ --- can have intermediate comments -CALL dolt_commit('-m', 'message', '--allow-empty')`, - "CALL dolt_checkout('main')", - }, - lineNums: []int{1, 5}, - }, - { - input: `select * /* -- ignore line comment inside block comment */ from xy; -select x from xy; -- select y from xy; -select * /* ignore multi-line comment with ; -comment; -comment; -*/ from foo; -select '-- ignore line comment -in quote';`, - statements: []string{ - "select * /* -- ignore line comment inside block comment */ from xy", - "select x from xy", - "", - `select * /* ignore multi-line comment with ; -comment; -comment; -*/ from foo`, - `select '-- ignore line comment -in quote'`, - }, - lineNums: []int{1, 2, 2, 3, 7}, - }, - } - - for _, tt := range testcases { - t.Run(tt.input, func(t *testing.T) { - reader := strings.NewReader(tt.input) - scanner := NewStreamScanner(reader) - var i int - for scanner.Scan() { - require.True(t, i < len(tt.statements)) - assert.Equal(t, tt.statements[i], strings.TrimSpace(scanner.Text())) - if tt.lineNums != nil { - assert.Equal(t, tt.lineNums[i], scanner.state.statementStartLine) - } else { - assert.Equal(t, 1, scanner.state.statementStartLine) - } - i++ - } - - require.NoError(t, scanner.Err()) - }) - } -} - -// TestEndedAtEOFFlag covers the per-Scan termination distinction that -// IsShellInputComplete relies on. A delimiter-terminated statement must -// leave EndedAtEOF false; an unterminated trailing statement must leave it -// true. -func TestEndedAtEOFFlag(t *testing.T) { - t.Run("delimiter terminated", func(t *testing.T) { - scanner := NewStreamScanner(strings.NewReader("select 1;")) - require.True(t, scanner.Scan()) - assert.False(t, scanner.EndedAtEOF()) - assert.False(t, scanner.Scan()) - }) - t.Run("unterminated EOF", func(t *testing.T) { - scanner := NewStreamScanner(strings.NewReader("select 1")) - require.True(t, scanner.Scan()) - assert.True(t, scanner.EndedAtEOF()) - assert.False(t, scanner.Scan()) - }) - t.Run("flag resets across scans", func(t *testing.T) { - // First statement terminates; second hits EOF unterminated. - scanner := NewStreamScanner(strings.NewReader("select 1; select 2")) - require.True(t, scanner.Scan()) - assert.False(t, scanner.EndedAtEOF()) - require.True(t, scanner.Scan()) - assert.True(t, scanner.EndedAtEOF()) - }) -} - -// TestIsShellInputComplete covers the interactive-shell completion -// predicate. Each case captures one branch of the four bug scenarios -// referenced by dolthub/dolt#10866, plus the DELIMITER and pure-comment -// edges that StreamScanner emits as empty tokens. The delimiter argument -// mirrors the shell's active DELIMITER; most cases use the default ";". -func TestIsShellInputComplete(t *testing.T) { - testcases := []struct { - name string - input string - delimiter string - want bool - }{ - {"empty", "", ";", true}, - {"whitespace only", " ", ";", true}, - {"only newlines", "\n\n", ";", true}, - {"unterminated single statement", "select 1", ";", false}, - {"terminated single statement", "select 1;", ";", true}, - {"terminated multi statement (#10860)", "select null; select null;", ";", true}, - {"trailing unterminated multi", "select 1; select 2", ";", false}, - {"open single quote (#10861)", "select '", ";", false}, - {"open double quote (#10861)", "select \"", ";", false}, - {"open backtick", "select `", ";", false}, - {"semicolon inside open quote (#10861)", "select ';", ";", false}, - {"closed quote then delimiter", "select 'foo';", ";", true}, - {"backslash-escaped quote leaves string open", "select '\\';", ";", false}, - {"doubled backslash closes string", "select '\\\\';", ";", true}, - {"open block comment (#10862)", "select /* ;", ";", false}, - {"closed block comment then delimiter", "select /* x */ null;", ";", true}, - {"line comment with semicolon and no newline", "select -- ;", ";", false}, - {"line comment with newline then terminator", "select -- ;\nnull;", ";", true}, - {"DELIMITER alone", "DELIMITER //", ";", true}, - {"DELIMITER then terminated", "DELIMITER //\nselect 1//", ";", true}, - {"DELIMITER then unterminated", "DELIMITER //\nselect 1", ";", false}, - {"pure line comment input", "-- hello\n", ";", true}, - // A pure block comment with no trailing terminator produces a - // non-empty trailing token at EOF (StreamScanner emits empty tokens - // only for pure line comments). The shell waits for the user to - // type a delimiter, matching MySQL's behavior after a comment. - {"pure block comment input", "/* hello */\n", ";", false}, - - // Non-";" delimiter cases (DELIMITER active in the shell). The - // predicate sees the cumulative buffer with the delimiter still - // present (the callback strips the trailing delimiter only after - // ishell delivers the buffer). Under a custom delimiter, internal ";" - // must not be treated as terminators; only the custom delimiter - // completes a statement. - {"custom delim: trigger body still being typed is incomplete", - "CREATE TRIGGER t BEFORE INSERT ON x FOR EACH ROW\nBEGIN\nSET NEW.v = 1;\nSET NEW.v = 2;\nEND;", "#", false}, - {"custom delim: trigger body completed by custom delimiter", - "CREATE TRIGGER t BEFORE INSERT ON x FOR EACH ROW\nBEGIN\nSET NEW.v = 1;\nSET NEW.v = 2;\nEND; #", "#", true}, - {"custom delim: single statement terminated by custom delimiter", - "select 1 #", "#", true}, - {"custom delim: unterminated single statement", "select 1", "#", false}, - {"custom delim: multi-char // unterminated", "select 1", "//", false}, - {"custom delim: multi-char // terminated", "select 1 //", "//", true}, - {"custom delim: empty stays complete", "", "#", true}, - } - - for _, tt := range testcases { - t.Run(tt.name, func(t *testing.T) { - got := IsShellInputComplete(tt.input, tt.delimiter) - assert.Equal(t, tt.want, got, "input=%q delimiter=%q", tt.input, tt.delimiter) - }) - } -} diff --git a/go/go.mod b/go/go.mod index 73f1972fa4d..8ea58e1c1a3 100644 --- a/go/go.mod +++ b/go/go.mod @@ -208,6 +208,6 @@ require ( gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect ) -replace github.com/dolthub/ishell => github.com/codeaucafe/ishell v0.0.0-20260607220657-061915e86568 +replace github.com/dolthub/ishell => github.com/codeaucafe/ishell v0.0.0-20260722022957-950b93b95974 go 1.26.2 diff --git a/go/go.sum b/go/go.sum index 434cd091801..5c97e6f56d7 100644 --- a/go/go.sum +++ b/go/go.sum @@ -189,8 +189,8 @@ github.com/cncf/xds/go v0.0.0-20251210132809-ee656c7534f5 h1:6xNmx7iTtyBRev0+D/T 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= -github.com/codeaucafe/ishell v0.0.0-20260607220657-061915e86568/go.mod h1:ehexgi1mPxRTk0Mok/pADALuHbvATulTh6gzr7NzZto= +github.com/codeaucafe/ishell v0.0.0-20260722022957-950b93b95974 h1:vqCdZNC85dov6jyhHJuYrlLUYMXAfiiPpK27QioyYkk= +github.com/codeaucafe/ishell v0.0.0-20260722022957-950b93b95974/go.mod h1:ehexgi1mPxRTk0Mok/pADALuHbvATulTh6gzr7NzZto= github.com/colinmarc/hdfs/v2 v2.1.1/go.mod h1:M3x+k8UKKmxtFu++uAZ0OtDU8jR3jnaZIAc6yK4Ue0c= github.com/coreos/go-systemd/v22 v22.3.3-0.20220203105225-a9a7ef127534/go.mod h1:Y58oyj3AT4RCenI/lSvhwexgC+NSVTIJ3seZv2GcEnc= github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= diff --git a/integration-tests/bats/sql-shell-ctrld-continuation.expect b/integration-tests/bats/sql-shell-ctrld-continuation.expect new file mode 100644 index 00000000000..016b125737e --- /dev/null +++ b/integration-tests/bats/sql-shell-ctrld-continuation.expect @@ -0,0 +1,34 @@ +#!/usr/bin/expect -f +# Ctrl-D at the continuation prompt discards the partial statement instead of +# executing it, then exits (matching the mysql client). Proven by side effect: +# an unterminated INSERT is aborted, so a fresh session sees an empty table. + +set timeout 20 +spawn dolt sql + +expect { "> " {} timeout { send_user "\nFAIL: no initial prompt\n"; exit 1 } } + +send "create table ctrld (i int);\r" +expect { "> " {} timeout { send_user "\nFAIL: create table\n"; exit 1 } } + +# Type an unterminated INSERT to drop into the continuation prompt, then send +# Ctrl-D (0x04). The partial INSERT must be discarded, not run. +send "insert into ctrld values (1)\r" +expect { -exact "-> " {} timeout { send_user "\nFAIL: no continuation prompt\n"; exit 1 } } +send "\x04" +expect { eof {} timeout { send_user "\nFAIL: Ctrl-D did not exit\n"; exit 1 } } + +# Fresh session: the table must be empty because the aborted INSERT never ran. +spawn dolt sql +expect { "> " {} timeout { send_user "\nFAIL: no prompt (2nd session)\n"; exit 1 } } + +send "select count(*) as c from ctrld;\r" +expect { + -re "\\| +0 +\\|" {} + -re "\\| +1 +\\|" { send_user "\nFAIL: partial INSERT was executed\n"; exit 1 } + timeout { send_user "\nFAIL: no count result\n"; exit 1 } +} +expect { "> " {} timeout { send_user "\nFAIL: no prompt after count\n"; exit 1 } } + +send "exit;\r" +expect eof diff --git a/integration-tests/bats/sql-shell-large-input.expect b/integration-tests/bats/sql-shell-large-input.expect new file mode 100644 index 00000000000..1d0f878c66d --- /dev/null +++ b/integration-tests/bats/sql-shell-large-input.expect @@ -0,0 +1,37 @@ +#!/usr/bin/expect -f +# A large multi-line statement parses and executes correctly. This is a +# correctness check; the O(n) single-pass property is guarded separately by a +# Go scaling test on the scanner in the ishell library. + +set timeout 20 +spawn dolt sql + +expect { "> " {} timeout { send_user "\nFAIL: no initial prompt\n"; exit 1 } } + +send "create table big (i int);\r" +expect { "> " {} timeout { send_user "\nFAIL: create table\n"; exit 1 } } + +# Build one INSERT statement whose value rows span 50 physical lines, then send +# it all at once. The continuation prompts are buffered by expect. +set stmt "insert into big values\r" +for {set k 1} {$k < 50} {incr k} { + append stmt "($k),\r" +} +append stmt "(50);\r" +send $stmt +expect { + -re "Query OK" {} + -re "syntax error" { send_user "\nFAIL: syntax error on large insert\n"; exit 1 } + timeout { send_user "\nFAIL: large insert did not complete\n"; exit 1 } +} +expect { "> " {} timeout { send_user "\nFAIL: no prompt after insert\n"; exit 1 } } + +send "select count(*) as c from big;\r" +expect { + -re "\\| +50 +\\|" {} + timeout { send_user "\nFAIL: wrong row count\n"; exit 1 } +} +expect { "> " {} timeout { send_user "\nFAIL: no prompt after count\n"; exit 1 } } + +send "exit;\r" +expect { eof {} timeout { send_user "\nFAIL: did not exit\n"; exit 1 } } diff --git a/integration-tests/bats/sql-shell-quote-continuation.expect b/integration-tests/bats/sql-shell-quote-continuation.expect index 5b55105060a..90b24ddb599 100644 --- a/integration-tests/bats/sql-shell-quote-continuation.expect +++ b/integration-tests/bats/sql-shell-quote-continuation.expect @@ -9,17 +9,22 @@ expect { timeout { send_user "\nFAIL: no initial prompt\n"; exit 1 } } -send "select ';\r" +# The ; is inside an unclosed quote, so the statement must not end here. +send "select length(';\r" expect { -exact "-> " {} -re "syntax error" { send_user "\nFAIL: treated as complete\n"; exit 1 } timeout { send_user "\nFAIL: no continuation prompt\n"; exit 1 } } -send "foo';\r" + +# Close the quote and complete the statement. Assert on the aliased result cell +# (10861) anchored between table pipes, so the terminal echo of the typed input +# (which has no surrounding "| |" delimiters) cannot satisfy the match. +send "x') as n, 10861 as tag;\r" expect { - "foo" {} + -re {\| +10861 +\|} {} -re "syntax error" { send_user "\nFAIL: parse failed\n"; exit 1 } - timeout { send_user "\nFAIL: no result\n"; exit 1 } + timeout { send_user "\nFAIL: no result row\n"; exit 1 } } expect { "> " {} diff --git a/integration-tests/bats/sql-shell.bats b/integration-tests/bats/sql-shell.bats index af2b8b8fd54..d8a487782e0 100644 --- a/integration-tests/bats/sql-shell.bats +++ b/integration-tests/bats/sql-shell.bats @@ -249,6 +249,26 @@ teardown() { [ "$status" -eq 0 ] } +# bats test_tags=no_lambda +@test "sql-shell: large multi-line statement executes correctly" { + skiponwindows "Need to install expect and make this script work on windows." + if [ "$SQL_ENGINE" = "remote-engine" ]; then + skip "Presently sql command will not connect to remote server due to lack of lock file where there are not DBs." + fi + run expect $BATS_TEST_DIRNAME/sql-shell-large-input.expect + [ "$status" -eq 0 ] +} + +# bats test_tags=no_lambda +@test "sql-shell: Ctrl-D at continuation prompt discards partial input" { + skiponwindows "Need to install expect and make this script work on windows." + if [ "$SQL_ENGINE" = "remote-engine" ]; then + skip "Presently sql command will not connect to remote server due to lack of lock file where there are not DBs." + fi + run expect $BATS_TEST_DIRNAME/sql-shell-ctrld-continuation.expect + [ "$status" -eq 0 ] +} + @test "sql-shell: works with ANSI_QUOTES SQL mode" { if [ $SQL_ENGINE = "remote-engine" ]; then skip "Presently sql command will not connect to remote server due to lack of lock file where there are not DBs."