From a78353723b1b37dc5107bd268aba2dd966edb448 Mon Sep 17 00:00:00 2001
From: David Taylor <tinystatemachine@gmail.com>
Date: Tue, 22 Aug 2017 02:02:53 +0000
Subject: [PATCH] sql: don't include quotes in parsed strings

The planner.TypeAsString* helpers, currently only really used in backup/restore/import, were using
parser.AsStringWithFlags to get the string value of the expr they retured. However even in 'bareStrings'
mode, this still prints non-trivial strings *in quotes*. If calling code is planning to use the value of
the string, for instance as a URI or parameter value, mangling it with added quotes is almost certainly
not desired. These helpers are used to read params from a query for processing, not print parsable SQL.
---
 pkg/ccl/acceptanceccl/backup_test.go |  2 +-
 pkg/ccl/sqlccl/backup_test.go        | 10 +++++-----
 pkg/sql/planner.go                   | 19 ++++++++++++++++---
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/pkg/ccl/acceptanceccl/backup_test.go b/pkg/ccl/acceptanceccl/backup_test.go
index f536679d6105..1d09940c5bdc 100644
--- a/pkg/ccl/acceptanceccl/backup_test.go
+++ b/pkg/ccl/acceptanceccl/backup_test.go
@@ -222,7 +222,7 @@ func BenchmarkRestoreBig(b *testing.B) {
 			b.Fatal(err)
 		}
 
-		dbName := fmt.Sprintf("bank%d", b.N)
+		dbName := fmt.Sprintf("bank %d", b.N)
 		r.Exec(fmt.Sprintf("CREATE DATABASE %s", dbName))
 
 		b.ResetTimer()
diff --git a/pkg/ccl/sqlccl/backup_test.go b/pkg/ccl/sqlccl/backup_test.go
index a0a76292f430..f258580d5f03 100644
--- a/pkg/ccl/sqlccl/backup_test.go
+++ b/pkg/ccl/sqlccl/backup_test.go
@@ -1371,7 +1371,7 @@ func TestBackupRestoreIncremental(t *testing.T) {
 		// the greatest key in the diff is less than the previous backups.
 		sqlDB.Exec(`INSERT INTO data.bank VALUES (0, -1, 'final')`)
 		checksums = append(checksums, checksumBankPayload(t, sqlDB))
-		finalBackupDir := filepath.Join(dir, "final")
+		finalBackupDir := filepath.Join(dir, "final layer")
 		sqlDB.Exec(fmt.Sprintf(`BACKUP TABLE data.bank TO '%s' %s`,
 			finalBackupDir, fmt.Sprintf(` INCREMENTAL FROM %s`, strings.Join(backupDirs, `,`)),
 		))
@@ -1837,18 +1837,18 @@ func TestRestoreInto(t *testing.T) {
 
 	sqlDB.Exec(`BACKUP DATABASE data TO $1`, dir)
 
-	restoreStmt := fmt.Sprintf(`RESTORE data.bank FROM '%s' WITH OPTIONS ('into_db'='data2')`, dir)
+	restoreStmt := fmt.Sprintf(`RESTORE data.bank FROM '%s' WITH into_db = 'data 2'`, dir)
 
 	_, err := sqlDB.DB.Exec(restoreStmt)
-	if !testutils.IsError(err, "a database named \"data2\" needs to exist") {
+	if !testutils.IsError(err, "a database named \"data 2\" needs to exist") {
 		t.Fatal(err)
 	}
 
-	sqlDB.Exec(`CREATE DATABASE data2`)
+	sqlDB.Exec(`CREATE DATABASE "data 2"`)
 	sqlDB.Exec(restoreStmt)
 
 	expected := sqlDB.QueryStr(`SELECT * FROM data.bank`)
-	sqlDB.CheckQueryResults(`SELECT * FROM data2.bank`, expected)
+	sqlDB.CheckQueryResults(`SELECT * FROM "data 2".bank`, expected)
 }
 
 func TestBackupRestorePermissions(t *testing.T) {
diff --git a/pkg/sql/planner.go b/pkg/sql/planner.go
index 3afb77ebe776..db22fef1276f 100644
--- a/pkg/sql/planner.go
+++ b/pkg/sql/planner.go
@@ -25,6 +25,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/envutil"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
+	"github.com/pkg/errors"
 	"golang.org/x/net/context"
 )
 
@@ -342,7 +343,11 @@ func (p *planner) TypeAsString(e parser.Expr, op string) (func() (string, error)
 		if err != nil {
 			return "", err
 		}
-		return parser.AsStringWithFlags(d, parser.FmtBareStrings), nil
+		str, ok := d.(*parser.DString)
+		if !ok {
+			return "", errors.Errorf("failed to cast %T to string", d)
+		}
+		return string(*str), nil
 	}
 	return fn, nil
 }
@@ -377,7 +382,11 @@ func (p *planner) TypeAsStringOpts(
 			if err != nil {
 				return nil, err
 			}
-			res[name] = parser.AsStringWithFlags(d, parser.FmtBareStrings)
+			str, ok := d.(*parser.DString)
+			if !ok {
+				return res, errors.Errorf("failed to cast %T to string", d)
+			}
+			res[name] = string(*str)
 		}
 		return res, nil
 	}
@@ -405,7 +414,11 @@ func (p *planner) TypeAsStringArray(
 			if err != nil {
 				return nil, err
 			}
-			strs[i] = parser.AsStringWithFlags(d, parser.FmtBareStrings)
+			str, ok := d.(*parser.DString)
+			if !ok {
+				return strs, errors.Errorf("failed to cast %T to string", d)
+			}
+			strs[i] = string(*str)
 		}
 		return strs, nil
 	}