Merge pull request #395 from liquidata-inc/aaron/sql-close-rowiters

go/cmd/dolt/commands/sql.go: Always close row iters. Improves robustness when SQL returns an error.
dolthub · Feb 17, 2020 · 22b4e97 · 22b4e97
2 parents 49963a6 + c7e5ce6
commit 22b4e97
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 0 deletions.
diff --git a/bats/sql-works-after-failing-query.expect 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
@@ -384,3 +384,8 @@ teardown() {
     [ "${#lines[@]}" -eq 5 ]
     [[ "${lines[3]}" =~ " <NULL> " ]] || false
 }
+
+@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
+}
diff --git 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 {