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 e142b84f9ef..73f1972fa4d 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 47ee1deb73b..434cd091801 100644 --- a/go/go.sum +++ b/go/go.sum @@ -189,6 +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/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= @@ -218,8 +220,6 @@ github.com/dolthub/go-mysql-server v0.20.1-0.20260612173244-a42b55aad6b4 h1:cy8U github.com/dolthub/go-mysql-server v0.20.1-0.20260612173244-a42b55aad6b4/go.mod h1:Q6+pUrFqO9ElSVu02xmSEzdcToXLVqRU+hZYcAa04wU= 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 5004d18a874..af2b8b8fd54 100644 --- a/integration-tests/bats/sql-shell.bats +++ b/integration-tests/bats/sql-shell.bats @@ -209,6 +209,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."