From af213366501775ee6392e02a93421907a136e962 Mon Sep 17 00:00:00 2001 From: Aaron Son Date: Thu, 13 Feb 2020 14:20:32 -0800 Subject: [PATCH 1/3] go/cmd/dolt/commands/sql.go: Always close row iters. Improves robustness when SQL returns an error. --- go/cmd/dolt/commands/sql.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/go/cmd/dolt/commands/sql.go b/go/cmd/dolt/commands/sql.go index 71f3c42efe4..316f76a513e 100644 --- a/go/cmd/dolt/commands/sql.go +++ b/go/cmd/dolt/commands/sql.go @@ -533,6 +533,7 @@ func processQuery(ctx context.Context, query string, se *sqlEngine) error { case *sqlparser.Select, *sqlparser.Insert, *sqlparser.Update, *sqlparser.OtherRead, *sqlparser.Show, *sqlparser.Explain: sqlSch, rowIter, err := se.query(ctx, query) if err == nil { + defer rowIter.Close() err = se.prettyPrintResults(ctx, se.ddb.ValueReadWriter().Format(), sqlSch, rowIter) } return err @@ -543,6 +544,7 @@ func processQuery(ctx context.Context, query string, se *sqlEngine) error { } sqlSch, rowIter, err := se.query(ctx, query) if err == nil { + defer rowIter.Close() err = se.prettyPrintResults(ctx, se.ddb.Format(), sqlSch, rowIter) } return err @@ -585,6 +587,7 @@ func processBatchQuery(ctx context.Context, query string, se *sqlEngine) error { if err != nil { return fmt.Errorf("Error inserting rows: %v", err.Error()) } + defer rowIter.Close() err = mergeInsertResultIntoStats(rowIter, &batchEditStats) if err != nil { From acae58005a69fe5cf944a37a9c54d942094e3aa5 Mon Sep 17 00:00:00 2001 From: Aaron Son Date: Thu, 13 Feb 2020 14:40:44 -0800 Subject: [PATCH 2/3] bats/sql.bats: Add a test case to ensure compliance. --- bats/sql-works-after-failing-query.expect | 17 +++++++++++++++++ bats/sql.bats | 4 ++++ 2 files changed, 21 insertions(+) create mode 100755 bats/sql-works-after-failing-query.expect diff --git a/bats/sql-works-after-failing-query.expect b/bats/sql-works-after-failing-query.expect new file mode 100755 index 00000000000..cc206124ac8 --- /dev/null +++ b/bats/sql-works-after-failing-query.expect @@ -0,0 +1,17 @@ +#!/usr/bin/expect + +set timeout 1 +spawn dolt sql +expect { + "doltsql> " { send "select json_unquote('\\\\');\r"; } +} +expect { + "error processing results: Missing a closing quotation mark in string" +} +expect { + "doltsql> " { send "select 1;\r"; } +} +expect { + "pid 0 is already in use" { exit 1 } + " 1 " { exit 0 } +} diff --git a/bats/sql.bats b/bats/sql.bats index 94ffc3eb51b..45360dafd03 100755 --- a/bats/sql.bats +++ b/bats/sql.bats @@ -384,3 +384,7 @@ teardown() { [ "${#lines[@]}" -eq 5 ] [[ "${lines[3]}" =~ " " ]] || false } + +@test "sql shell works after failing query" { + $BATS_TEST_DIRNAME/sql-works-after-failing-query.expect +} From c7e5ce670b755ff5dda7bdd2c71c299674a0f19d Mon Sep 17 00:00:00 2001 From: Aaron Son Date: Thu, 13 Feb 2020 15:21:32 -0800 Subject: [PATCH 3/3] bats/sql.bats: Skip new bats tests on Windows. --- bats/sql.bats | 1 + 1 file changed, 1 insertion(+) diff --git a/bats/sql.bats b/bats/sql.bats index 45360dafd03..763701973bb 100755 --- a/bats/sql.bats +++ b/bats/sql.bats @@ -386,5 +386,6 @@ teardown() { } @test "sql shell works after failing query" { + skiponwindows "Need to install expect and make this script work on windows." $BATS_TEST_DIRNAME/sql-works-after-failing-query.expect }