Skip to content

Commit

Permalink
Merge #26032 #26043
Browse files Browse the repository at this point in the history 
26032: importccl: append file/row notes to IMPORT error messages r=mjibson a=mjibson

Fixes #25532

Release note (sql change): improve IMPORT error messages

26043: sql: Add test showing manual column type change r=bobvawter a=bobvawter

This test verifies that it's possible to effect a non-trivial column type
change using the current state of the system and verifies the planned strategy
for ALTER COLUMN TYPE.

Release note: None

Co-authored-by: Matt Jibson <[email protected]>
Co-authored-by: Bob Vawter <[email protected]>
  • Loading branch information
@maddyblue @bobvawter
3 people committed May 24, 2018
3 parents 92f687c + b7ff7ed + 7be5d91 commit fa750da
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 25 deletions.
70 changes: 52 additions & 18 deletions pkg/ccl/importccl/import_stmt_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import (
"github.com/pkg/errors"
)

func TestLoadCSV(t *testing.T) {
func TestImportData(t *testing.T) {
defer leaktest.AfterTest(t)()

s, db, _ := serverutils.StartServer(t, base.TestServerArgs{})
Expand All @@ -60,7 +60,8 @@ func TestLoadCSV(t *testing.T) {
name string
create string
with string
csv string
typ string
data string
err string
query map[string][][]string
}{
Expand All @@ -71,7 +72,8 @@ func TestLoadCSV(t *testing.T) {
i int,
unique index idx_f (i)
`,
csv: `1,1
typ: "CSV",
data: `1,1
2,2
3,3
4,3
Expand All @@ -83,7 +85,8 @@ func TestLoadCSV(t *testing.T) {
create: `
i int primary key
`,
csv: `1
typ: "CSV",
data: `1
2
3
3
Expand All @@ -95,7 +98,8 @@ func TestLoadCSV(t *testing.T) {
create: `
s string collate en_u_ks_level1 primary key
`,
csv: `a
typ: "CSV",
data: `a
B
c
D
Expand All @@ -110,7 +114,8 @@ d
s string
`,
with: `WITH sstsize = '10B'`,
csv: `1,0000000000
typ: "CSV",
data: `1,0000000000
1,0000000000`,
err: "duplicate key",
},
Expand All @@ -127,15 +132,17 @@ d
family (s, c)
`,
with: `WITH sstsize = '1B'`,
csv: `5,STRING,7,9`,
typ: "CSV",
data: `5,STRING,7,9`,
query: map[string][][]string{
`SELECT count(*) from d.t`: {{"1"}},
},
},
{
name: "good bytes encoding",
create: `b bytes`,
csv: `\x0143
typ: "CSV",
data: `\x0143
0143`,
query: map[string][][]string{
`SELECT * from d.t`: {{"\x01C"}, {"0143"}},
Expand All @@ -144,34 +151,61 @@ d
{
name: "invalid byte",
create: `b bytes`,
csv: `\x0g`,
typ: "CSV",
data: `\x0g`,
err: "invalid byte",
},
{
name: "bad bytes length",
create: `b bytes`,
csv: `\x0`,
typ: "CSV",
data: `\x0`,
err: "odd length hex string",
},

// MySQL OUTFILE
{
name: "unexpected number of columns",
create: `i int`,
typ: "MYSQLOUTFILE",
data: "1\t2\n",
err: "row 1: expected 1 columns, got 2",
},
{
name: "unmatched field enclosure",
create: `i int`,
with: `WITH fields_enclosed_by = '"'`,
typ: "MYSQLOUTFILE",
data: "\"1",
err: "row 1: unmatched field enclosure",
},
{
name: "unmatched literal",
create: `i int`,
with: `WITH fields_escaped_by = '\'`,
typ: "MYSQLOUTFILE",
data: `\`,
err: "row 1: unmatched literal",
},
}

var csvString string
var dataString string
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method == "GET" {
fmt.Fprint(w, csvString)
fmt.Fprint(w, dataString)
}
}))
defer srv.Close()

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
t.Run(fmt.Sprintf("%s: %s", tc.typ, tc.name), func(t *testing.T) {
sqlDB.Exec(t, `DROP TABLE IF EXISTS d.t`)
q := fmt.Sprintf(`IMPORT TABLE d.t (%s) CSV DATA ($1) %s`, tc.create, tc.with)
q := fmt.Sprintf(`IMPORT TABLE d.t (%s) %s DATA ($1) %s`, tc.create, tc.typ, tc.with)
t.Log(q)
csvString = tc.csv
dataString = tc.data
_, err := db.Exec(q, srv.URL)
if !testutils.IsError(err, tc.err) {
t.Fatalf("unexpected: %v", err)
t.Errorf("unexpected: %v", err)
}
for query, res := range tc.query {
sqlDB.CheckQueryResults(t, query, res)
Expand Down Expand Up @@ -590,10 +624,10 @@ func TestImportCSVStmt(t *testing.T) {
)

data = ",5,e,,,"
if _, err := conn.Exec(query, srv.URL); !testutils.IsError(err, `could not parse "" as type int`) {
if _, err := conn.Exec(query, srv.URL); !testutils.IsError(err, `row 1: parse "a" as INT: could not parse ""`) {
t.Fatalf("unexpected: %v", err)
}
if _, err := conn.Exec(query+nullif, srv.URL); !testutils.IsError(err, `"a" violates not-null constraint`) {
if _, err := conn.Exec(query+nullif, srv.URL); !testutils.IsError(err, `row 1: generate insert row: null value in column "a" violates not-null constraint`) {
t.Fatalf("unexpected: %v", err)
}
data = "2,,e,,,"
Expand Down
4 changes: 2 additions & 2 deletions pkg/ccl/importccl/read_import_csv.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,12 @@ func (c *csvInputReader) convertRecord(ctx context.Context) error {
var err error
conv.datums[i], err = tree.ParseDatumStringAs(col.Type.ToDatumType(), v, &conv.evalCtx)
if err != nil {
return errors.Wrapf(err, "%s: row %d: parse %q as %s", batch.file, rowNum, col.Name, col.Type.SQLString())
return makeRowErr(batch.file, rowNum, "parse %q as %s: %s:", col.Name, col.Type.SQLString(), err)
}
}
}
if err := conv.row(ctx, batch.fileIndex, rowNum); err != nil {
return errors.Wrapf(err, "converting row: %s: row %d", batch.file, rowNum)
return makeRowErr(batch.file, rowNum, "%s", err)
}
}
}
Expand Down
8 changes: 3 additions & 5 deletions pkg/ccl/importccl/read_import_mysqlout.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import (
"io"
"unicode"

"github.com/pkg/errors"

"github.com/cockroachdb/cockroach/pkg/roachpb"
"github.com/cockroachdb/cockroach/pkg/sql/sqlbase"
)
Expand Down Expand Up @@ -74,10 +72,10 @@ func (d *mysqloutfileReader) readFile(
// First check that if we're done and everything looks good.
if finished {
if nextLiteral {
return errors.New("unmatched literal")
return makeRowErr(inputName, int64(count)+1, "unmatched literal")
}
if readingField {
return errors.New("unmatched field enclosure")
return makeRowErr(inputName, int64(count)+1, "unmatched field enclosure")
}
// flush the last row if we have one.
if len(row) > 0 {
Expand Down Expand Up @@ -160,7 +158,7 @@ func (d *mysqloutfileReader) readFile(

if c == d.opts.RowSeparator {
if expected := d.csvInputReader.expectedCols; expected != len(row) {
return errors.Errorf("row %d: expected %d columns, go %d: %#v", count+1, expected, len(row), row)
return makeRowErr(inputName, int64(count)+1, "expected %d columns, got %d: %#v", expected, len(row), row)
}
d.csvInputReader.batch.r = append(d.csvInputReader.batch.r, row)
count++
Expand Down
4 changes: 4 additions & 0 deletions pkg/ccl/importccl/read_import_proc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,10 @@ func sampleAll(kv roachpb.KeyValue) bool {
return true
}

func makeRowErr(file string, row int64, format string, args ...interface{}) error {
return errors.Errorf("%q: row %d: "+format, append([]interface{}{file, row}, args...)...)
}

func init() {
distsqlrun.NewReadImportDataProcessor = newReadImportDataProcessor
}
59 changes: 59 additions & 0 deletions pkg/sql/logictest/testdata/logic_test/alter_column_type
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,65 @@ statement ok
DROP TABLE t


# Demonstrate manual flow for non-trivial column change
subtest ManualGeneralChange

statement ok
CREATE TABLE t (a INT PRIMARY KEY, b STRING)

statement ok
CREATE INDEX idx ON t (b)

statement ok
INSERT INTO t VALUES (1, '01'), (2, '002'), (3, '0003')

query IT colnames
SELECT * from t ORDER BY b DESC
----
a b
1 01
2 002
3 0003

statement ok
ALTER TABLE t ADD COLUMN i INT as (b::INT) STORED

statement ok
CREATE INDEX idx2 ON t (i)

statement ok
ALTER TABLE t ALTER COLUMN i DROP STORED, DROP COLUMN b CASCADE

query TT colnames
show create table t
----
Table CreateTable
t CREATE TABLE t (
a INT NOT NULL,
i INT NULL,
CONSTRAINT "primary" PRIMARY KEY (a ASC),
INDEX idx2 (i ASC),
FAMILY "primary" (a, i)
)

statement ok
ALTER TABLE t RENAME COLUMN i TO b

statement ok
ALTER INDEX idx2 RENAME TO idx

query II colnames
SELECT * from t ORDER BY b DESC
----
a b
3 3
2 2
1 1

statement ok
DROP TABLE t CASCADE


# Demonstrate that we can change to an alias of a type
subtest ChangeVisibleColumnType

Expand Down

0 comments on commit fa750da

Please sign in to comment.