From 0999ae4f6ede72b1c616cda2429332b456605f7e Mon Sep 17 00:00:00 2001 From: Mike Coutermarsh Date: Fri, 28 Apr 2023 10:57:58 -0400 Subject: [PATCH 1/6] Dump: support invisible columns Fixes: https://github.com/planetscale/cli/issues/652 Originally, the dump command grabbed the column names by running `select *`. This does not work with invisible columns, since `select *` will only return visible columns. This PR updates `dump` to use `SHOW FIELDS` which returns invisible columns. ``` invisible/main> show fields from mike; +-------+------+------+-----+---------+-----------+ | Field | Type | Null | Key | Default | Extra | +-------+------+------+-----+---------+-----------+ | id | int | NO | PRI | NULL | | | test | int | YES | | NULL | INVISIBLE | +-------+------+------+-----+---------+-----------+ ``` We then use that list to generate the dump. --- internal/dumper/dumper.go | 41 +++++++++++++-------------------------- 1 file changed, 14 insertions(+), 27 deletions(-) diff --git a/internal/dumper/dumper.go b/internal/dumper/dumper.go index a333189a..9c084a76 100644 --- a/internal/dumper/dumper.go +++ b/internal/dumper/dumper.go @@ -223,43 +223,28 @@ func (d *Dumper) dumpTable(conn *Connection, database string, table string) erro var where string var selfields []string - isGenerated, err := d.generatedFields(conn, table) - if err != nil { - return err - } - fields := make([]string, 0) { - cursor, err := conn.StreamFetch(fmt.Sprintf("SELECT * FROM `%s`.`%s` LIMIT 1", database, table)) + flds, err := d.dumpableFieldNames(conn, table) if err != nil { return err } - flds := cursor.Fields() for _, fld := range flds { - d.log.Debug("dump", zap.Any("filters", d.cfg.Filters), zap.String("table", table), zap.String("field_name", fld.Name)) - - if _, ok := d.cfg.Filters[table][fld.Name]; ok { - continue - } + d.log.Debug("dump", zap.Any("filters", d.cfg.Filters), zap.String("table", table), zap.String("field_name", fld)) - if isGenerated[fld.Name] { + if _, ok := d.cfg.Filters[table][fld]; ok { continue } - fields = append(fields, fmt.Sprintf("`%s`", fld.Name)) - replacement, ok := d.cfg.Selects[table][fld.Name] + fields = append(fields, fmt.Sprintf("`%s`", fld)) + replacement, ok := d.cfg.Selects[table][fld] if ok { - selfields = append(selfields, fmt.Sprintf("%s AS `%s`", replacement, fld.Name)) + selfields = append(selfields, fmt.Sprintf("%s AS `%s`", replacement, fld)) } else { - selfields = append(selfields, fmt.Sprintf("`%s`", fld.Name)) + selfields = append(selfields, fmt.Sprintf("`%s`", fld)) } } - - err = cursor.Close() - if err != nil { - return err - } } if v, ok := d.cfg.Wheres[table]; ok { @@ -406,16 +391,15 @@ func (d *Dumper) filterDatabases(conn *Connection, filter *regexp.Regexp, invert return databases, nil } -// generatedFields returns a map that contains fields that are virtually -// generated. -func (d *Dumper) generatedFields(conn *Connection, table string) (map[string]bool, error) { +// dumpableFieldNames returns a map that contains valid field names for the dump +func (d *Dumper) dumpableFieldNames(conn *Connection, table string) ([]string, error) { qr, err := conn.Fetch(fmt.Sprintf("SHOW FIELDS FROM `%s`", table)) if err != nil { return nil, err } - fields := map[string]bool{} + fields := make([]string, 0, len(qr.Rows)) for _, t := range qr.Rows { if len(t) != 6 { return nil, fmt.Errorf("error fetching fields, expecting to have 6 columns, have: %d", len(t)) @@ -427,7 +411,10 @@ func (d *Dumper) generatedFields(conn *Connection, table string) (map[string]boo // Can be either "VIRTUAL GENERATED" or "VIRTUAL STORED" // https://dev.mysql.com/doc/refman/8.0/en/show-columns.html if strings.Contains(extra, "VIRTUAL") { - fields[name] = true + // Skip generated columns + continue + } else { + fields = append(fields, name) } } From a64e6e4d1a3fb8c9efd5575d78914f2f5147f7a3 Mon Sep 17 00:00:00 2001 From: Mike Coutermarsh Date: Fri, 28 Apr 2023 11:30:23 -0400 Subject: [PATCH 2/6] Update tests for dumper Now that we use fields to grab all the field names, need to update all the stubbed data. --- internal/dumper/dumper_test.go | 101 +++++++++++++++++---------------- 1 file changed, 53 insertions(+), 48 deletions(-) diff --git a/internal/dumper/dumper_test.go b/internal/dumper/dumper_test.go index 22a364b4..84e406f3 100644 --- a/internal/dumper/dumper_test.go +++ b/internal/dumper/dumper_test.go @@ -14,6 +14,17 @@ import ( qt "github.com/frankban/quicktest" ) +func createFieldRow(name, extra string) []sqltypes.Value { + return []sqltypes.Value{ + sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte(name)), + sqltypes.MakeTrusted(querypb.Type_BLOB, []byte("varchar(255)")), + sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte("YES")), + sqltypes.MakeTrusted(querypb.Type_BINARY, []byte("")), + sqltypes.MakeTrusted(querypb.Type_NULL_TYPE, []byte("NULL")), + sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte(extra)), + } +} + func TestDumper(t *testing.T) { c := qt.New(t) @@ -114,14 +125,13 @@ func TestDumper(t *testing.T) { {Name: "Extra", Type: querypb.Type_VARCHAR}, }, Rows: [][]sqltypes.Value{ - { - sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte("not_deleted")), - sqltypes.MakeTrusted(querypb.Type_BLOB, []byte("int")), - sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte("YES")), - sqltypes.MakeTrusted(querypb.Type_BINARY, []byte("")), - sqltypes.MakeTrusted(querypb.Type_NULL_TYPE, []byte("NULL")), - sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte("VIRTUAL GENERATED")), - }, + createFieldRow("id", ""), + createFieldRow("name", ""), + createFieldRow("namei1", ""), + createFieldRow("null", ""), + createFieldRow("decimal", ""), + createFieldRow("datetime", ""), + createFieldRow("not_deleted", "virtual generated"), }, } @@ -261,14 +271,13 @@ func TestDumperGeneratedFields(t *testing.T) { {Name: "Extra", Type: querypb.Type_VARCHAR}, }, Rows: [][]sqltypes.Value{ - { - sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte("name")), - sqltypes.MakeTrusted(querypb.Type_BLOB, []byte("varchar(255)")), - sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte("YES")), - sqltypes.MakeTrusted(querypb.Type_BINARY, []byte("")), - sqltypes.MakeTrusted(querypb.Type_NULL_TYPE, []byte("NULL")), - sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte("VIRTUAL GENERATED")), - }, + createFieldRow("id", ""), + createFieldRow("name", "VIRTUAL GENERATED"), + createFieldRow("namei1", ""), + createFieldRow("null", ""), + createFieldRow("decimal", ""), + createFieldRow("datetime", ""), + createFieldRow("not_deleted", "VIRTUAL GENERATED"), }, } @@ -442,14 +451,13 @@ func TestDumperAll(t *testing.T) { {Name: "Extra", Type: querypb.Type_VARCHAR}, }, Rows: [][]sqltypes.Value{ - { - sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte("not_deleted")), - sqltypes.MakeTrusted(querypb.Type_BLOB, []byte("int")), - sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte("YES")), - sqltypes.MakeTrusted(querypb.Type_BINARY, []byte("")), - sqltypes.MakeTrusted(querypb.Type_NULL_TYPE, []byte("NULL")), - sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte("VIRTUAL GENERATED")), - }, + createFieldRow("id", ""), + createFieldRow("name", ""), + createFieldRow("namei1", ""), + createFieldRow("null", ""), + createFieldRow("decimal", ""), + createFieldRow("datetime", ""), + createFieldRow("not_deleted", "virtual generated"), }, } @@ -630,14 +638,13 @@ func TestDumperMultiple(t *testing.T) { {Name: "Extra", Type: querypb.Type_VARCHAR}, }, Rows: [][]sqltypes.Value{ - { - sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte("not_deleted")), - sqltypes.MakeTrusted(querypb.Type_BLOB, []byte("int")), - sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte("YES")), - sqltypes.MakeTrusted(querypb.Type_BINARY, []byte("")), - sqltypes.MakeTrusted(querypb.Type_NULL_TYPE, []byte("NULL")), - sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte("VIRTUAL GENERATED")), - }, + createFieldRow("id", ""), + createFieldRow("name", ""), + createFieldRow("namei1", ""), + createFieldRow("null", ""), + createFieldRow("decimal", ""), + createFieldRow("datetime", ""), + createFieldRow("not_deleted", "virtual generated"), }, } @@ -828,14 +835,13 @@ func TestDumperSimpleRegexp(t *testing.T) { {Name: "Extra", Type: querypb.Type_VARCHAR}, }, Rows: [][]sqltypes.Value{ - { - sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte("not_deleted")), - sqltypes.MakeTrusted(querypb.Type_BLOB, []byte("int")), - sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte("YES")), - sqltypes.MakeTrusted(querypb.Type_BINARY, []byte("")), - sqltypes.MakeTrusted(querypb.Type_NULL_TYPE, []byte("NULL")), - sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte("VIRTUAL GENERATED")), - }, + createFieldRow("id", ""), + createFieldRow("name", ""), + createFieldRow("namei1", ""), + createFieldRow("null", ""), + createFieldRow("decimal", ""), + createFieldRow("datetime", ""), + createFieldRow("not_deleted", "virtual generated"), }, } @@ -1026,14 +1032,13 @@ func TestDumperComplexRegexp(t *testing.T) { {Name: "Extra", Type: querypb.Type_VARCHAR}, }, Rows: [][]sqltypes.Value{ - { - sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte("not_deleted")), - sqltypes.MakeTrusted(querypb.Type_BLOB, []byte("int")), - sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte("YES")), - sqltypes.MakeTrusted(querypb.Type_BINARY, []byte("")), - sqltypes.MakeTrusted(querypb.Type_NULL_TYPE, []byte("NULL")), - sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte("VIRTUAL GENERATED")), - }, + createFieldRow("id", ""), + createFieldRow("name", ""), + createFieldRow("namei1", ""), + createFieldRow("null", ""), + createFieldRow("decimal", ""), + createFieldRow("datetime", ""), + createFieldRow("not_deleted", "virtual generated"), }, } From 1a647c6d1b9e59508c4231a6091b16520392fdfa Mon Sep 17 00:00:00 2001 From: Mike Coutermarsh Date: Fri, 28 Apr 2023 13:45:12 -0400 Subject: [PATCH 3/6] Fix generated column check Fixes: https://github.com/planetscale/cli/issues/645 The correct values are VIRTUAL GENERATED or STORED GENERATED --- internal/dumper/dumper.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/dumper/dumper.go b/internal/dumper/dumper.go index 9c084a76..21a99749 100644 --- a/internal/dumper/dumper.go +++ b/internal/dumper/dumper.go @@ -408,9 +408,9 @@ func (d *Dumper) dumpableFieldNames(conn *Connection, table string) ([]string, e name := t[0].String() extra := t[5].String() - // Can be either "VIRTUAL GENERATED" or "VIRTUAL STORED" + // Can be either "VIRTUAL GENERATED" or "STORED GENERATED" // https://dev.mysql.com/doc/refman/8.0/en/show-columns.html - if strings.Contains(extra, "VIRTUAL") { + if strings.Contains(extra, "GENERATED") { // Skip generated columns continue } else { From c15b91f8818b250afbf4430a88888d26d2ca2539 Mon Sep 17 00:00:00 2001 From: Mike Coutermarsh Date: Mon, 1 May 2023 11:00:46 -0400 Subject: [PATCH 4/6] It's a slice not a map Co-authored-by: Fatih Arslan --- internal/dumper/dumper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/dumper/dumper.go b/internal/dumper/dumper.go index 21a99749..8ee60e3f 100644 --- a/internal/dumper/dumper.go +++ b/internal/dumper/dumper.go @@ -391,7 +391,7 @@ func (d *Dumper) filterDatabases(conn *Connection, filter *regexp.Regexp, invert return databases, nil } -// dumpableFieldNames returns a map that contains valid field names for the dump +// dumpableFieldNames returns a slice that contains valid field names for the dump. func (d *Dumper) dumpableFieldNames(conn *Connection, table string) ([]string, error) { qr, err := conn.Fetch(fmt.Sprintf("SHOW FIELDS FROM `%s`", table)) if err != nil { From 15224557721a9f9bd1c7f893aeba23e7b9f32739 Mon Sep 17 00:00:00 2001 From: Mike Coutermarsh Date: Mon, 1 May 2023 11:10:40 -0400 Subject: [PATCH 5/6] test function naming more go-like --- internal/dumper/dumper_test.go | 86 +++++++++++++++++----------------- 1 file changed, 43 insertions(+), 43 deletions(-) diff --git a/internal/dumper/dumper_test.go b/internal/dumper/dumper_test.go index 84e406f3..15bd53aa 100644 --- a/internal/dumper/dumper_test.go +++ b/internal/dumper/dumper_test.go @@ -14,7 +14,7 @@ import ( qt "github.com/frankban/quicktest" ) -func createFieldRow(name, extra string) []sqltypes.Value { +func testRow(name, extra string) []sqltypes.Value { return []sqltypes.Value{ sqltypes.MakeTrusted(querypb.Type_VARCHAR, []byte(name)), sqltypes.MakeTrusted(querypb.Type_BLOB, []byte("varchar(255)")), @@ -125,13 +125,13 @@ func TestDumper(t *testing.T) { {Name: "Extra", Type: querypb.Type_VARCHAR}, }, Rows: [][]sqltypes.Value{ - createFieldRow("id", ""), - createFieldRow("name", ""), - createFieldRow("namei1", ""), - createFieldRow("null", ""), - createFieldRow("decimal", ""), - createFieldRow("datetime", ""), - createFieldRow("not_deleted", "virtual generated"), + testRow("id", ""), + testRow("name", ""), + testRow("namei1", ""), + testRow("null", ""), + testRow("decimal", ""), + testRow("datetime", ""), + testRow("not_deleted", "virtual generated"), }, } @@ -271,13 +271,13 @@ func TestDumperGeneratedFields(t *testing.T) { {Name: "Extra", Type: querypb.Type_VARCHAR}, }, Rows: [][]sqltypes.Value{ - createFieldRow("id", ""), - createFieldRow("name", "VIRTUAL GENERATED"), - createFieldRow("namei1", ""), - createFieldRow("null", ""), - createFieldRow("decimal", ""), - createFieldRow("datetime", ""), - createFieldRow("not_deleted", "VIRTUAL GENERATED"), + testRow("id", ""), + testRow("name", "VIRTUAL GENERATED"), + testRow("namei1", ""), + testRow("null", ""), + testRow("decimal", ""), + testRow("datetime", ""), + testRow("not_deleted", "VIRTUAL GENERATED"), }, } @@ -451,13 +451,13 @@ func TestDumperAll(t *testing.T) { {Name: "Extra", Type: querypb.Type_VARCHAR}, }, Rows: [][]sqltypes.Value{ - createFieldRow("id", ""), - createFieldRow("name", ""), - createFieldRow("namei1", ""), - createFieldRow("null", ""), - createFieldRow("decimal", ""), - createFieldRow("datetime", ""), - createFieldRow("not_deleted", "virtual generated"), + testRow("id", ""), + testRow("name", ""), + testRow("namei1", ""), + testRow("null", ""), + testRow("decimal", ""), + testRow("datetime", ""), + testRow("not_deleted", "virtual generated"), }, } @@ -638,13 +638,13 @@ func TestDumperMultiple(t *testing.T) { {Name: "Extra", Type: querypb.Type_VARCHAR}, }, Rows: [][]sqltypes.Value{ - createFieldRow("id", ""), - createFieldRow("name", ""), - createFieldRow("namei1", ""), - createFieldRow("null", ""), - createFieldRow("decimal", ""), - createFieldRow("datetime", ""), - createFieldRow("not_deleted", "virtual generated"), + testRow("id", ""), + testRow("name", ""), + testRow("namei1", ""), + testRow("null", ""), + testRow("decimal", ""), + testRow("datetime", ""), + testRow("not_deleted", "virtual generated"), }, } @@ -835,13 +835,13 @@ func TestDumperSimpleRegexp(t *testing.T) { {Name: "Extra", Type: querypb.Type_VARCHAR}, }, Rows: [][]sqltypes.Value{ - createFieldRow("id", ""), - createFieldRow("name", ""), - createFieldRow("namei1", ""), - createFieldRow("null", ""), - createFieldRow("decimal", ""), - createFieldRow("datetime", ""), - createFieldRow("not_deleted", "virtual generated"), + testRow("id", ""), + testRow("name", ""), + testRow("namei1", ""), + testRow("null", ""), + testRow("decimal", ""), + testRow("datetime", ""), + testRow("not_deleted", "virtual generated"), }, } @@ -1032,13 +1032,13 @@ func TestDumperComplexRegexp(t *testing.T) { {Name: "Extra", Type: querypb.Type_VARCHAR}, }, Rows: [][]sqltypes.Value{ - createFieldRow("id", ""), - createFieldRow("name", ""), - createFieldRow("namei1", ""), - createFieldRow("null", ""), - createFieldRow("decimal", ""), - createFieldRow("datetime", ""), - createFieldRow("not_deleted", "virtual generated"), + testRow("id", ""), + testRow("name", ""), + testRow("namei1", ""), + testRow("null", ""), + testRow("decimal", ""), + testRow("datetime", ""), + testRow("not_deleted", "virtual generated"), }, } From ba4423cbaab0ee733deef32024aba5a3df3ba1b1 Mon Sep 17 00:00:00 2001 From: Mike Coutermarsh Date: Mon, 1 May 2023 11:12:12 -0400 Subject: [PATCH 6/6] improve naming --- internal/dumper/dumper.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/dumper/dumper.go b/internal/dumper/dumper.go index 8ee60e3f..cfdabee0 100644 --- a/internal/dumper/dumper.go +++ b/internal/dumper/dumper.go @@ -230,19 +230,19 @@ func (d *Dumper) dumpTable(conn *Connection, database string, table string) erro return err } - for _, fld := range flds { - d.log.Debug("dump", zap.Any("filters", d.cfg.Filters), zap.String("table", table), zap.String("field_name", fld)) + for _, name := range flds { + d.log.Debug("dump", zap.Any("filters", d.cfg.Filters), zap.String("table", table), zap.String("field_name", name)) - if _, ok := d.cfg.Filters[table][fld]; ok { + if _, ok := d.cfg.Filters[table][name]; ok { continue } - fields = append(fields, fmt.Sprintf("`%s`", fld)) - replacement, ok := d.cfg.Selects[table][fld] + fields = append(fields, fmt.Sprintf("`%s`", name)) + replacement, ok := d.cfg.Selects[table][name] if ok { - selfields = append(selfields, fmt.Sprintf("%s AS `%s`", replacement, fld)) + selfields = append(selfields, fmt.Sprintf("%s AS `%s`", replacement, name)) } else { - selfields = append(selfields, fmt.Sprintf("`%s`", fld)) + selfields = append(selfields, fmt.Sprintf("`%s`", name)) } } }