From 88b1a9b16d144c4ea1657ed817694dca259fcc9b Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 1 Aug 2023 10:20:28 -0700 Subject: [PATCH 01/28] Initial support for storing ANSI QUOTES mode info in schema fragments table --- go/libraries/doltcore/doltdb/system_table.go | 6 ++-- go/libraries/doltcore/schema/reserved_tags.go | 1 + go/libraries/doltcore/sqle/database.go | 18 +++++++++-- .../sqle/enginetest/dolt_engine_test.go | 8 +++++ .../doltcore/sqle/index/prolly_fields.go | 6 ++++ go/libraries/doltcore/sqle/schema_table.go | 30 +++++++++++++++---- 6 files changed, 58 insertions(+), 11 deletions(-) diff --git a/go/libraries/doltcore/doltdb/system_table.go b/go/libraries/doltcore/doltdb/system_table.go index c4e07d4162..be4d8f2b50 100644 --- a/go/libraries/doltcore/doltdb/system_table.go +++ b/go/libraries/doltcore/doltdb/system_table.go @@ -244,8 +244,10 @@ const ( // SchemasTablesExtraCol The name of the column that stores extra information about a schema element in the // dolt_schemas table SchemasTablesExtraCol = "extra" - // - SchemasTablesIndexName = "fragment_name" + // SchemasTablesAnsiQuotesCol is the name of the column that stores whether this fragment needs to be parsed + // in ANSI_QUOTES mode to treat double quote chars as identifier quotes, instead of string literal quotes. + SchemasTablesAnsiQuotesCol = "ansi_quotes" + SchemasTablesIndexName = "fragment_name" ) const ( diff --git a/go/libraries/doltcore/schema/reserved_tags.go b/go/libraries/doltcore/schema/reserved_tags.go index 5d314c7a0a..623c71afc7 100644 --- a/go/libraries/doltcore/schema/reserved_tags.go +++ b/go/libraries/doltcore/schema/reserved_tags.go @@ -79,6 +79,7 @@ const ( DoltSchemasNameTag DoltSchemasFragmentTag DoltSchemasExtraTag + DoltSchemasAnsiQuotesTag ) // Tags for hidden columns in keyless rows diff --git a/go/libraries/doltcore/sqle/database.go b/go/libraries/doltcore/sqle/database.go index 46b84c228c..1f680016cd 100644 --- a/go/libraries/doltcore/sqle/database.go +++ b/go/libraries/doltcore/sqle/database.go @@ -26,6 +26,7 @@ import ( "github.com/dolthub/go-mysql-server/sql/parse" "github.com/dolthub/go-mysql-server/sql/plan" "github.com/dolthub/go-mysql-server/sql/types" + "github.com/dolthub/vitess/go/vt/sqlparser" "gopkg.in/src-d/go-errors.v1" "github.com/dolthub/dolt/go/libraries/doltcore/branch_control" @@ -1124,14 +1125,16 @@ func getViewDefinitionFromSchemaFragmentsOfView(ctx *sql.Context, tbl *WritableD var viewDef sql.ViewDefinition var views = make([]sql.ViewDefinition, len(fragments)) for i, fragment := range fragments { - cv, err := parse.Parse(ctx, fragments[i].fragment) + cv, err := parse.ParseWithOptions(ctx, fragments[i].fragment, + sqlparser.ParserOptions{AnsiQuotes: fragment.ansiQuotes}) if err != nil { return nil, sql.ViewDefinition{}, false, err } createView, ok := cv.(*plan.CreateView) if ok { - views[i] = sql.ViewDefinition{Name: fragments[i].name, TextDefinition: createView.Definition.TextDefinition, CreateViewStatement: fragments[i].fragment} + views[i] = sql.ViewDefinition{Name: fragments[i].name, TextDefinition: createView.Definition.TextDefinition, + CreateViewStatement: fragments[i].fragment, AnsiQuotes: fragment.ansiQuotes} } else { views[i] = sql.ViewDefinition{Name: fragments[i].name, TextDefinition: fragments[i].fragment, CreateViewStatement: fmt.Sprintf("CREATE VIEW %s AS %s", fragments[i].name, fragments[i].fragment)} } @@ -1200,6 +1203,7 @@ func (db Database) GetTriggers(ctx *sql.Context) ([]sql.TriggerDefinition, error Name: frag.name, CreateStatement: frag.fragment, CreatedAt: frag.created, + AnsiQuotes: frag.ansiQuotes, }) } if err != nil { @@ -1241,12 +1245,14 @@ func (db Database) GetEvent(ctx *sql.Context, name string) (sql.EventDefinition, return sql.EventDefinition{}, false, err } + // TODO: Some duplication with GetEvents here... would be nice to clean up for _, frag := range frags { if strings.ToLower(frag.name) == strings.ToLower(name) { return sql.EventDefinition{ Name: frag.name, CreateStatement: frag.fragment, CreatedAt: frag.created, + AnsiQuotes: frag.ansiQuotes, }, true, nil } } @@ -1274,6 +1280,7 @@ func (db Database) GetEvents(ctx *sql.Context) ([]sql.EventDefinition, error) { Name: frag.name, CreateStatement: frag.fragment, CreatedAt: frag.created, + AnsiQuotes: frag.ansiQuotes, }) } return events, nil @@ -1372,7 +1379,12 @@ func (db Database) addFragToSchemasTable(ctx *sql.Context, fragType, name, defin return err } - return inserter.Insert(ctx, sql.Row{fragType, name, definition, extraJSON}) + sqlMode, err := sql.LoadSqlMode(ctx) + if err != nil { + return err + } + + return inserter.Insert(ctx, sql.Row{fragType, name, definition, extraJSON, sqlMode.AnsiQuotes()}) } func (db Database) dropFragFromSchemasTable(ctx *sql.Context, fragType, name string, missingErr error) error { diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go index 9df940ae26..5a1bff6d20 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go @@ -336,6 +336,14 @@ func TestVersionedQueries(t *testing.T) { enginetest.TestVersionedQueries(t, h) } +func TestAnsiQuotesSqlMode(t *testing.T) { + enginetest.TestAnsiQuotesSqlMode(t, newDoltHarness(t)) +} + +func TestAnsiQuotesSqlModePrepared(t *testing.T) { + enginetest.TestAnsiQuotesSqlModePrepared(t, newDoltHarness(t)) +} + // Tests of choosing the correct execution plan independent of result correctness. Mostly useful for confirming that // the right indexes are being used for joining tables. func TestQueryPlans(t *testing.T) { diff --git a/go/libraries/doltcore/sqle/index/prolly_fields.go b/go/libraries/doltcore/sqle/index/prolly_fields.go index 3b3ee5385e..d19ef27f93 100644 --- a/go/libraries/doltcore/sqle/index/prolly_fields.go +++ b/go/libraries/doltcore/sqle/index/prolly_fields.go @@ -229,6 +229,12 @@ func PutField(ctx context.Context, ns tree.NodeStore, tb *val.TupleBuilder, i in func convInt(v interface{}) int { switch i := v.(type) { + case bool: + if i { + return 1 + } else { + return 0 + } case int: return i case int8: diff --git a/go/libraries/doltcore/sqle/schema_table.go b/go/libraries/doltcore/sqle/schema_table.go index 7029c68722..6ff08564e4 100644 --- a/go/libraries/doltcore/sqle/schema_table.go +++ b/go/libraries/doltcore/sqle/schema_table.go @@ -61,6 +61,7 @@ var schemasTableCols = schema.NewColCollection( mustNewColWithTypeInfo(doltdb.SchemasTablesNameCol, schema.DoltSchemasNameTag, typeinfo.CreateVarStringTypeFromSqlType(mustCreateStringType(query.Type_VARCHAR, 64, sql.Collation_utf8mb4_0900_ai_ci)), true, "", false, ""), mustNewColWithTypeInfo(doltdb.SchemasTablesFragmentCol, schema.DoltSchemasFragmentTag, typeinfo.CreateVarStringTypeFromSqlType(gmstypes.LongText), false, "", false, ""), mustNewColWithTypeInfo(doltdb.SchemasTablesExtraCol, schema.DoltSchemasExtraTag, typeinfo.JSONType, false, "", false, ""), + mustNewColWithTypeInfo(doltdb.SchemasTablesAnsiQuotesCol, schema.DoltSchemasAnsiQuotesTag, typeinfo.BoolType, false, "false", false, ""), ) var schemaTableSchema = schema.MustSchemaFromCols(schemasTableCols) @@ -117,6 +118,7 @@ func migrateOldSchemasTableToNew(ctx *sql.Context, db Database, schemasTable *Wr typeIdx := schemasTable.sqlSchema().IndexOfColName(doltdb.SchemasTablesTypeCol) fragmentIdx := schemasTable.sqlSchema().IndexOfColName(doltdb.SchemasTablesFragmentCol) extraIdx := schemasTable.sqlSchema().IndexOfColName(doltdb.SchemasTablesExtraCol) + ansiQuotesIdx := schemasTable.sqlSchema().IndexOfColName(doltdb.SchemasTablesAnsiQuotesCol) defer func(iter sql.RowIter, ctx *sql.Context) { err := iter.Close(ctx) @@ -142,6 +144,9 @@ func migrateOldSchemasTableToNew(ctx *sql.Context, db Database, schemasTable *Wr if extraIdx >= 0 { newRow[3] = sqlRow[extraIdx] } + if ansiQuotesIdx >= 0 { + newRow[4] = sqlRow[ansiQuotesIdx] + } newRows = append(newRows, newRow) } @@ -229,6 +234,9 @@ type schemaFragment struct { name string fragment string created time.Time + // ansiQuotes indicates if this fragment must be parsed with ANSI_QUOTES mode, meaning it uses double quote + // characters as identifier quotes, instead of string literal quotes. + ansiQuotes bool } func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType string) (sf []schemaFragment, rerr error) { @@ -243,6 +251,7 @@ func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType typeIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesTypeCol) fragmentIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesFragmentCol) extraIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesExtraCol) + ansiQuotesIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesAnsiQuotesCol) defer func(iter sql.RowIter, ctx *sql.Context) { err := iter.Close(ctx) @@ -265,12 +274,20 @@ func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType continue } + ansiQuotes := false + if ansiQuotesIdx >= 0 { + if sqlRow[ansiQuotesIdx].(int8) > 0 { + ansiQuotes = true + } + } + // For older tables, use 1 as the trigger creation time if extraIdx < 0 || sqlRow[extraIdx] == nil { frags = append(frags, schemaFragment{ - name: sqlRow[nameIdx].(string), - fragment: sqlRow[fragmentIdx].(string), - created: time.Unix(1, 0).UTC(), // TablePlus editor thinks 0 is out of range + name: sqlRow[nameIdx].(string), + fragment: sqlRow[fragmentIdx].(string), + created: time.Unix(1, 0).UTC(), // TablePlus editor thinks 0 is out of range + ansiQuotes: ansiQuotes, }) continue } @@ -279,9 +296,10 @@ func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType createdTime, err := getCreatedTime(ctx, sqlRow[extraIdx].(gmstypes.JSONValue)) frags = append(frags, schemaFragment{ - name: sqlRow[nameIdx].(string), - fragment: sqlRow[fragmentIdx].(string), - created: time.Unix(createdTime, 0).UTC(), + name: sqlRow[nameIdx].(string), + fragment: sqlRow[fragmentIdx].(string), + created: time.Unix(createdTime, 0).UTC(), + ansiQuotes: ansiQuotes, }) } From c2e17d3807b80da4afe2e4c886ad636268caf17d Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 1 Aug 2023 12:43:41 -0700 Subject: [PATCH 02/28] stored procedure support for ANSI_QUOTES --- go/libraries/doltcore/doltdb/system_table.go | 3 +++ go/libraries/doltcore/schema/reserved_tags.go | 1 + .../doltcore/sqle/procedures_table.go | 19 ++++++++++++++++++- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/go/libraries/doltcore/doltdb/system_table.go b/go/libraries/doltcore/doltdb/system_table.go index 3baecae964..c0e6252861 100644 --- a/go/libraries/doltcore/doltdb/system_table.go +++ b/go/libraries/doltcore/doltdb/system_table.go @@ -332,4 +332,7 @@ const ( ProceduresTableCreatedAtCol = "created_at" // ProceduresTableModifiedAtCol is the time that the stored procedure was last modified, in UTC. ProceduresTableModifiedAtCol = "modified_at" + // ProceduresTableAnsiQuotesCol is whether this stored procedure was defined in ANSI_QUOTES mode, indicating + // that double quotes should be treated as identifier quotes, instead of string literal quotes. + ProceduresTableAnsiQuotesCol = "ansi_quotes" ) diff --git a/go/libraries/doltcore/schema/reserved_tags.go b/go/libraries/doltcore/schema/reserved_tags.go index 623c71afc7..43cfa4efcd 100644 --- a/go/libraries/doltcore/schema/reserved_tags.go +++ b/go/libraries/doltcore/schema/reserved_tags.go @@ -94,6 +94,7 @@ const ( DoltProceduresCreateStmtTag DoltProceduresCreatedAtTag DoltProceduresModifiedAtTag + DoltProceduresAnsiQuotesTag ) const ( diff --git a/go/libraries/doltcore/sqle/procedures_table.go b/go/libraries/doltcore/sqle/procedures_table.go index 4f3d2c723a..e05a72718d 100644 --- a/go/libraries/doltcore/sqle/procedures_table.go +++ b/go/libraries/doltcore/sqle/procedures_table.go @@ -59,6 +59,7 @@ func ProceduresTableSchema() schema.Schema { schema.NewColumn(doltdb.ProceduresTableCreateStmtCol, schema.DoltProceduresCreateStmtTag, types.StringKind, false), schema.NewColumn(doltdb.ProceduresTableCreatedAtCol, schema.DoltProceduresCreatedAtTag, types.TimestampKind, false), schema.NewColumn(doltdb.ProceduresTableModifiedAtCol, schema.DoltProceduresModifiedAtTag, types.TimestampKind, false), + schema.NewColumn(doltdb.ProceduresTableAnsiQuotesCol, schema.DoltProceduresAnsiQuotesTag, types.BoolKind, false), ) return schema.MustSchemaFromCols(colColl) } @@ -71,6 +72,18 @@ func DoltProceduresGetOrCreateTable(ctx *sql.Context, db Database) (*WritableDol return nil, err } if found { + // If the table we found doesn't have the same number of fields in its schema, + // then add any schema fields from the end that are missing. + alterableDoltTable := tbl.(*AlterableDoltTable) + targetSchema := ProceduresTableSqlSchema().Schema + for i := len(tbl.Schema()); i < len(targetSchema); i++ { + columnOrder := sql.ColumnOrder{} + err := alterableDoltTable.AddColumn(ctx, targetSchema[i], &columnOrder) + if err != nil { + return nil, err + } + } + return tbl.(*WritableDoltTable), nil } @@ -179,6 +192,9 @@ func DoltProceduresGetAll(ctx *sql.Context, db Database, procedureName string) ( if d.ModifiedAt, ok = sqlRow[3].(time.Time); !ok { return nil, missingValue.New(doltdb.ProceduresTableModifiedAtCol, sqlRow) } + if sqlRow[4].(int8) > 0 { + d.AnsiQuotes = true + } details = append(details, d) } return details, nil @@ -210,6 +226,7 @@ func DoltProceduresAddProcedure(ctx *sql.Context, db Database, spd sql.StoredPro spd.CreateStatement, spd.CreatedAt.UTC(), spd.ModifiedAt.UTC(), + spd.AnsiQuotes, }) } @@ -276,7 +293,7 @@ func DoltProceduresGetDetails(ctx *sql.Context, tbl *WritableDoltTable, name str sqlRow, err := rowIter.Next(ctx) if err == nil { - if len(sqlRow) != 4 { + if len(sqlRow) != 5 { return sql.StoredProcedureDetails{}, false, fmt.Errorf("unexpected row in dolt_procedures:\n%v", sqlRow) } return sql.StoredProcedureDetails{ From 2367808a9c01fc8da3c301360d25e1d1cd183539 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 1 Aug 2023 16:30:09 -0700 Subject: [PATCH 03/28] `dolt sql` support for ANSI_QUOTES --- go/cmd/dolt/commands/sql.go | 8 ++++- integration-tests/bats/sql-shell.bats | 45 +++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/go/cmd/dolt/commands/sql.go b/go/cmd/dolt/commands/sql.go index 20416f6bcf..96791f8411 100644 --- a/go/cmd/dolt/commands/sql.go +++ b/go/cmd/dolt/commands/sql.go @@ -607,7 +607,13 @@ func execBatchMode(ctx *sql.Context, qryist cli.Queryist, input io.Reader, conti if len(query) == 0 || query == "\n" { continue } - sqlStatement, err := sqlparser.Parse(query) + + sqlMode, err := sql.LoadSqlMode(ctx) + if err != nil { + return err + } + + sqlStatement, err := sqlparser.ParseWithOptions(query, sqlMode.ParserOptions()) if err == sqlparser.ErrEmpty { continue } else if err != nil { diff --git a/integration-tests/bats/sql-shell.bats b/integration-tests/bats/sql-shell.bats index a4e7e08a80..6eb00b6597 100644 --- a/integration-tests/bats/sql-shell.bats +++ b/integration-tests/bats/sql-shell.bats @@ -83,6 +83,51 @@ teardown() { $BATS_TEST_DIRNAME/sql-shell-empty-prompt.expect } +@test "sql-shell: works with ANSI_QUOTES SQL mode" { + if [ "$SQL_ENGINE" = "remote-engine" ]; then + skip "Presently sql command will not connect to remote server due to lack of lock file where there are not DBs." + fi + + mkdir doltsql + cd doltsql + dolt init + + dolt sql << SQL +SET @@SQL_MODE=ANSI_QUOTES; +CREATE TABLE "table1"("pk" int primary key, "col1" int DEFAULT ("pk")); +CREATE TRIGGER trigger1 BEFORE INSERT ON "table1" FOR EACH ROW SET NEW."pk" = NEW."pk" + 1; +INSERT INTO "table1" ("pk") VALUES (1); +CREATE VIEW "view1" AS select "pk", "col1" from "table1"; +CREATE PROCEDURE procedure1() SELECT "pk", "col1" from "table1"; +SQL + + # In a new session, with SQL_MODE set back to the default modes, assert that + # we can still use the entities we created with ANSI_QUOTES. + run dolt sql -q "INSERT INTO table1 (pk) VALUES (111);" + [ $status -eq "0" ] + [[ $output =~ "Query OK" ]] || false + + run dolt sql -r csv -q "SELECT * from table1;" + [ $status -eq "0" ] + [[ $output =~ "2,1" ]] || false + [[ $output =~ "112,111" ]] || false + + run dolt sql -q "show tables;" + [ $status -eq "0" ] + [[ $output =~ "table1" ]] || false + [[ $output =~ "view1" ]] || false + + run dolt sql -r csv -q "SELECT * from view1;" + [ $status -eq "0" ] + [[ $output =~ "2,1" ]] || false + [[ $output =~ "112,111" ]] || false + + run dolt sql -r csv -q "call procedure1;" + [ $status -eq "0" ] + [[ $output =~ "2,1" ]] || false + [[ $output =~ "112,111" ]] || false +} + @test "sql-shell: delimiter" { skiponwindows "Need to install expect and make this script work on windows." mkdir doltsql From 9106f29cfdc5343472da013cf02430017456f59d Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 1 Aug 2023 16:31:33 -0700 Subject: [PATCH 04/28] First (messy) pass at `dolt dump` support for ANSI_QUOTES --- go/cmd/dolt/commands/dump.go | 59 +++++++++++++++++++++++++++++++- integration-tests/bats/dump.bats | 27 ++++++++++++++- 2 files changed, 84 insertions(+), 2 deletions(-) diff --git a/go/cmd/dolt/commands/dump.go b/go/cmd/dolt/commands/dump.go index ed6f578467..9976ef8ff6 100644 --- a/go/cmd/dolt/commands/dump.go +++ b/go/cmd/dolt/commands/dump.go @@ -41,6 +41,7 @@ import ( "github.com/dolthub/dolt/go/libraries/utils/argparser" "github.com/dolthub/dolt/go/libraries/utils/filesys" "github.com/dolthub/dolt/go/libraries/utils/iohelp" + "github.com/dolthub/vitess/go/vt/sqlparser" ) const ( @@ -269,6 +270,7 @@ func dumpProcedures(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb. } stmtColIdx := sch.IndexOfColName(doltdb.ProceduresTableCreateStmtCol) + ansiQuotesIdx := sch.IndexOfColName(doltdb.ProceduresTableAnsiQuotesCol) defer func(iter sql.RowIter, context *sql.Context) { err := iter.Close(context) @@ -285,6 +287,20 @@ func dumpProcedures(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb. return err } + ansiQuotesMode := false + if len(row) >= ansiQuotesIdx { + if row[ansiQuotesIdx].(int8) > 0 { + ansiQuotesMode = true + } + } + + if ansiQuotesMode { + err = iohelp.WriteLine(writer, "SET @@SQL_MODE=CONCAT(@@SQL_MODE, ',ANSI_QUOTES');") + if err != nil { + return err + } + } + err = iohelp.WriteLine(writer, "delimiter END_PROCEDURE") if err != nil { return err @@ -321,6 +337,7 @@ func dumpTriggers(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb.Ro typeColIdx := sch.IndexOfColName(doltdb.SchemasTablesTypeCol) fragColIdx := sch.IndexOfColName(doltdb.SchemasTablesFragmentCol) + ansiQuotesIdx := sch.IndexOfColName(doltdb.SchemasTablesAnsiQuotesCol) defer func(iter sql.RowIter, context *sql.Context) { err := iter.Close(context) @@ -341,6 +358,20 @@ func dumpTriggers(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb.Ro continue } + ansiQuotesMode := false + if row[ansiQuotesIdx].(int8) > 0 { + ansiQuotesMode = true + } + + // TODO: helper functions? + // TODO: Need to clean up how we set SQL_MODE in the output dump file + if ansiQuotesMode { + err = iohelp.WriteLine(writer, "SET @@SQL_MODE=CONCAT(@@SQL_MODE, ',ANSI_QUOTES');") + if err != nil { + return err + } + } + err = iohelp.WriteLine(writer, fmt.Sprintf("%s;", row[fragColIdx])) if err != nil { return err @@ -368,6 +399,7 @@ func dumpViews(ctx *sql.Context, engine *engine.SqlEngine, root *doltdb.RootValu typeColIdx := sch.IndexOfColName(doltdb.SchemasTablesTypeCol) fragColIdx := sch.IndexOfColName(doltdb.SchemasTablesFragmentCol) nameColIdx := sch.IndexOfColName(doltdb.SchemasTablesNameCol) + ansiQuotesColIdx := sch.IndexOfColName(doltdb.SchemasTablesAnsiQuotesCol) defer func(iter sql.RowIter, context *sql.Context) { err := iter.Close(context) @@ -388,12 +420,30 @@ func dumpViews(ctx *sql.Context, engine *engine.SqlEngine, root *doltdb.RootValu continue } + ansiQuotesMode := false + if row[ansiQuotesColIdx].(int8) > 0 { + ansiQuotesMode = true + } + // We used to store just the SELECT part of a view, but now we store the entire CREATE VIEW statement - cv, err := parse.Parse(ctx, row[fragColIdx].(string)) + cv, err := parse.ParseWithOptions(ctx, row[fragColIdx].(string), sqlparser.ParserOptions{ + AnsiQuotes: ansiQuotesMode, + }) if err != nil { return err } + if ansiQuotesMode { + err := iohelp.WriteLine(writer, "SET @previousSqlMode=@@SQL_MODE;") + if err != nil { + return err + } + err = iohelp.WriteLine(writer, "SET @@SQL_MODE=CONCAT(@@SQL_MODE, ',ANSI_QUOTES');") + if err != nil { + return err + } + } + _, ok := cv.(*plan.CreateView) if ok { err := iohelp.WriteLine(writer, fmt.Sprintf("%s;", row[fragColIdx])) @@ -406,6 +456,13 @@ func dumpViews(ctx *sql.Context, engine *engine.SqlEngine, root *doltdb.RootValu return err } } + + if ansiQuotesMode { + err := iohelp.WriteLine(writer, "SET @@SQL_MODE=@previousSqlMode;") + if err != nil { + return err + } + } } return nil diff --git a/integration-tests/bats/dump.bats b/integration-tests/bats/dump.bats index e6ecb500d3..850a3d8c69 100644 --- a/integration-tests/bats/dump.bats +++ b/integration-tests/bats/dump.bats @@ -364,7 +364,6 @@ SQL } @test "dump: SQL type - with keyless tables" { - dolt sql -q "CREATE TABLE new_table(pk int primary key);" dolt sql -q "INSERT INTO new_table VALUES (1);" dolt sql -q "CREATE TABLE warehouse(warehouse_id int primary key, warehouse_name longtext);" @@ -865,6 +864,32 @@ SQL [ "${#lines[@]}" -eq 2 ] } +# Assert that we can create data in ANSI_QUOTES mode, and then correctly dump it +# out after disabling ANSI_QUOTES mode. +@test "dump: ANSI_QUOTES data" { + dolt sql << SQL +SET @@SQL_MODE=ANSI_QUOTES; +CREATE TABLE "table1"("pk" int primary key, "col1" int DEFAULT ("pk")); +CREATE TRIGGER trigger1 BEFORE INSERT ON "table1" FOR EACH ROW SET NEW."pk" = NEW."pk" + 1; +INSERT INTO "table1" ("pk") VALUES (1); +CREATE VIEW "view1" AS select "pk", "col1" from "table1"; +CREATE PROCEDURE procedure1() SELECT "pk", "col1" from "table1"; +SQL + + run dolt dump + [ $status -eq 0 ] + [[ $output =~ "Successfully exported data." ]] || false + [ -f doltdump.sql ] + cp doltdump.sql ~/doltdump.sql + + # TODO: Would be good to make sure we can roundtrip it, but we won't be able + # to without SQL_MODE set to ANSI_QUOTES + # TODO: Need to clean up setting SQL_MODE in dump file; + # How does MySQL do this? If we could just normalize the query + # or fragment to non-ANSI_QUOTES, then everything would be so + # much easier! +} + @test "dump: round trip dolt dump with all data types" { skip "export table in bit, binary and blob types not working" # table with all data types with one all null values row and one all non-null values row From d1fd6419f95a644071884e5f9ba18f4a676a0b20 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 1 Aug 2023 16:31:33 -0700 Subject: [PATCH 05/28] First (messy) pass at `dolt dump` support for ANSI_QUOTES --- go/cmd/dolt/commands/dump.go | 59 ++++++++++++++++++++++++++- integration-tests/bats/dump.bats | 27 +++++++++++- integration-tests/bats/sql-shell.bats | 2 +- 3 files changed, 85 insertions(+), 3 deletions(-) diff --git a/go/cmd/dolt/commands/dump.go b/go/cmd/dolt/commands/dump.go index ed6f578467..9976ef8ff6 100644 --- a/go/cmd/dolt/commands/dump.go +++ b/go/cmd/dolt/commands/dump.go @@ -41,6 +41,7 @@ import ( "github.com/dolthub/dolt/go/libraries/utils/argparser" "github.com/dolthub/dolt/go/libraries/utils/filesys" "github.com/dolthub/dolt/go/libraries/utils/iohelp" + "github.com/dolthub/vitess/go/vt/sqlparser" ) const ( @@ -269,6 +270,7 @@ func dumpProcedures(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb. } stmtColIdx := sch.IndexOfColName(doltdb.ProceduresTableCreateStmtCol) + ansiQuotesIdx := sch.IndexOfColName(doltdb.ProceduresTableAnsiQuotesCol) defer func(iter sql.RowIter, context *sql.Context) { err := iter.Close(context) @@ -285,6 +287,20 @@ func dumpProcedures(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb. return err } + ansiQuotesMode := false + if len(row) >= ansiQuotesIdx { + if row[ansiQuotesIdx].(int8) > 0 { + ansiQuotesMode = true + } + } + + if ansiQuotesMode { + err = iohelp.WriteLine(writer, "SET @@SQL_MODE=CONCAT(@@SQL_MODE, ',ANSI_QUOTES');") + if err != nil { + return err + } + } + err = iohelp.WriteLine(writer, "delimiter END_PROCEDURE") if err != nil { return err @@ -321,6 +337,7 @@ func dumpTriggers(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb.Ro typeColIdx := sch.IndexOfColName(doltdb.SchemasTablesTypeCol) fragColIdx := sch.IndexOfColName(doltdb.SchemasTablesFragmentCol) + ansiQuotesIdx := sch.IndexOfColName(doltdb.SchemasTablesAnsiQuotesCol) defer func(iter sql.RowIter, context *sql.Context) { err := iter.Close(context) @@ -341,6 +358,20 @@ func dumpTriggers(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb.Ro continue } + ansiQuotesMode := false + if row[ansiQuotesIdx].(int8) > 0 { + ansiQuotesMode = true + } + + // TODO: helper functions? + // TODO: Need to clean up how we set SQL_MODE in the output dump file + if ansiQuotesMode { + err = iohelp.WriteLine(writer, "SET @@SQL_MODE=CONCAT(@@SQL_MODE, ',ANSI_QUOTES');") + if err != nil { + return err + } + } + err = iohelp.WriteLine(writer, fmt.Sprintf("%s;", row[fragColIdx])) if err != nil { return err @@ -368,6 +399,7 @@ func dumpViews(ctx *sql.Context, engine *engine.SqlEngine, root *doltdb.RootValu typeColIdx := sch.IndexOfColName(doltdb.SchemasTablesTypeCol) fragColIdx := sch.IndexOfColName(doltdb.SchemasTablesFragmentCol) nameColIdx := sch.IndexOfColName(doltdb.SchemasTablesNameCol) + ansiQuotesColIdx := sch.IndexOfColName(doltdb.SchemasTablesAnsiQuotesCol) defer func(iter sql.RowIter, context *sql.Context) { err := iter.Close(context) @@ -388,12 +420,30 @@ func dumpViews(ctx *sql.Context, engine *engine.SqlEngine, root *doltdb.RootValu continue } + ansiQuotesMode := false + if row[ansiQuotesColIdx].(int8) > 0 { + ansiQuotesMode = true + } + // We used to store just the SELECT part of a view, but now we store the entire CREATE VIEW statement - cv, err := parse.Parse(ctx, row[fragColIdx].(string)) + cv, err := parse.ParseWithOptions(ctx, row[fragColIdx].(string), sqlparser.ParserOptions{ + AnsiQuotes: ansiQuotesMode, + }) if err != nil { return err } + if ansiQuotesMode { + err := iohelp.WriteLine(writer, "SET @previousSqlMode=@@SQL_MODE;") + if err != nil { + return err + } + err = iohelp.WriteLine(writer, "SET @@SQL_MODE=CONCAT(@@SQL_MODE, ',ANSI_QUOTES');") + if err != nil { + return err + } + } + _, ok := cv.(*plan.CreateView) if ok { err := iohelp.WriteLine(writer, fmt.Sprintf("%s;", row[fragColIdx])) @@ -406,6 +456,13 @@ func dumpViews(ctx *sql.Context, engine *engine.SqlEngine, root *doltdb.RootValu return err } } + + if ansiQuotesMode { + err := iohelp.WriteLine(writer, "SET @@SQL_MODE=@previousSqlMode;") + if err != nil { + return err + } + } } return nil diff --git a/integration-tests/bats/dump.bats b/integration-tests/bats/dump.bats index e6ecb500d3..850a3d8c69 100644 --- a/integration-tests/bats/dump.bats +++ b/integration-tests/bats/dump.bats @@ -364,7 +364,6 @@ SQL } @test "dump: SQL type - with keyless tables" { - dolt sql -q "CREATE TABLE new_table(pk int primary key);" dolt sql -q "INSERT INTO new_table VALUES (1);" dolt sql -q "CREATE TABLE warehouse(warehouse_id int primary key, warehouse_name longtext);" @@ -865,6 +864,32 @@ SQL [ "${#lines[@]}" -eq 2 ] } +# Assert that we can create data in ANSI_QUOTES mode, and then correctly dump it +# out after disabling ANSI_QUOTES mode. +@test "dump: ANSI_QUOTES data" { + dolt sql << SQL +SET @@SQL_MODE=ANSI_QUOTES; +CREATE TABLE "table1"("pk" int primary key, "col1" int DEFAULT ("pk")); +CREATE TRIGGER trigger1 BEFORE INSERT ON "table1" FOR EACH ROW SET NEW."pk" = NEW."pk" + 1; +INSERT INTO "table1" ("pk") VALUES (1); +CREATE VIEW "view1" AS select "pk", "col1" from "table1"; +CREATE PROCEDURE procedure1() SELECT "pk", "col1" from "table1"; +SQL + + run dolt dump + [ $status -eq 0 ] + [[ $output =~ "Successfully exported data." ]] || false + [ -f doltdump.sql ] + cp doltdump.sql ~/doltdump.sql + + # TODO: Would be good to make sure we can roundtrip it, but we won't be able + # to without SQL_MODE set to ANSI_QUOTES + # TODO: Need to clean up setting SQL_MODE in dump file; + # How does MySQL do this? If we could just normalize the query + # or fragment to non-ANSI_QUOTES, then everything would be so + # much easier! +} + @test "dump: round trip dolt dump with all data types" { skip "export table in bit, binary and blob types not working" # table with all data types with one all null values row and one all non-null values row diff --git a/integration-tests/bats/sql-shell.bats b/integration-tests/bats/sql-shell.bats index 6eb00b6597..c6788ff10b 100644 --- a/integration-tests/bats/sql-shell.bats +++ b/integration-tests/bats/sql-shell.bats @@ -84,7 +84,7 @@ teardown() { } @test "sql-shell: works with ANSI_QUOTES SQL mode" { - if [ "$SQL_ENGINE" = "remote-engine" ]; then + if [ $SQL_ENGINE = "remote-engine" ]; then skip "Presently sql command will not connect to remote server due to lack of lock file where there are not DBs." fi From d4908183e1d577ed8f276d0ad9b3821ac7e7a3ef Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Mon, 7 Aug 2023 14:08:30 -0700 Subject: [PATCH 06/28] Switching from tracking ANSI_QUOTES, specifically, to SQL_MODE, more generally --- go/cmd/dolt/commands/dump.go | 45 ++++---- go/libraries/doltcore/doltdb/system_table.go | 14 +-- go/libraries/doltcore/schema/reserved_tags.go | 4 +- go/libraries/doltcore/sqle/database.go | 12 +- .../doltcore/sqle/procedures_table.go | 106 +++++++++++++++--- go/libraries/doltcore/sqle/schema_table.go | 40 +++---- 6 files changed, 146 insertions(+), 75 deletions(-) diff --git a/go/cmd/dolt/commands/dump.go b/go/cmd/dolt/commands/dump.go index e7752baf78..b90c658a87 100644 --- a/go/cmd/dolt/commands/dump.go +++ b/go/cmd/dolt/commands/dump.go @@ -41,7 +41,6 @@ import ( "github.com/dolthub/dolt/go/libraries/utils/argparser" "github.com/dolthub/dolt/go/libraries/utils/filesys" "github.com/dolthub/dolt/go/libraries/utils/iohelp" - "github.com/dolthub/vitess/go/vt/sqlparser" ) const ( @@ -270,7 +269,7 @@ func dumpProcedures(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb. } stmtColIdx := sch.IndexOfColName(doltdb.ProceduresTableCreateStmtCol) - ansiQuotesIdx := sch.IndexOfColName(doltdb.ProceduresTableAnsiQuotesCol) + sqlModeIdx := sch.IndexOfColName(doltdb.ProceduresTableSqlModeCol) defer func(iter sql.RowIter, context *sql.Context) { err := iter.Close(context) @@ -287,15 +286,15 @@ func dumpProcedures(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb. return err } - ansiQuotesMode := false - if len(row) >= ansiQuotesIdx { - if row[ansiQuotesIdx].(int8) > 0 { - ansiQuotesMode = true + sqlMode := "" + if len(row) >= sqlModeIdx { + if s, ok := row[sqlModeIdx].(string); ok { + sqlMode = s } } - if ansiQuotesMode { - err = iohelp.WriteLine(writer, "SET @@SQL_MODE=CONCAT(@@SQL_MODE, ',ANSI_QUOTES');") + if sqlMode != "" { + err = iohelp.WriteLine(writer, fmt.Sprintf("SET @@SQL_MODE='%s;", sqlMode)) if err != nil { return err } @@ -337,7 +336,7 @@ func dumpTriggers(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb.Ro typeColIdx := sch.IndexOfColName(doltdb.SchemasTablesTypeCol) fragColIdx := sch.IndexOfColName(doltdb.SchemasTablesFragmentCol) - ansiQuotesIdx := sch.IndexOfColName(doltdb.SchemasTablesAnsiQuotesCol) + sqlModeIdx := sch.IndexOfColName(doltdb.SchemasTablesSqlModeCol) defer func(iter sql.RowIter, context *sql.Context) { err := iter.Close(context) @@ -358,15 +357,15 @@ func dumpTriggers(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb.Ro continue } - ansiQuotesMode := false - if row[ansiQuotesIdx].(int8) > 0 { - ansiQuotesMode = true + sqlMode := "" + if s, ok := row[sqlModeIdx].(string); ok { + sqlMode = s } // TODO: helper functions? // TODO: Need to clean up how we set SQL_MODE in the output dump file - if ansiQuotesMode { - err = iohelp.WriteLine(writer, "SET @@SQL_MODE=CONCAT(@@SQL_MODE, ',ANSI_QUOTES');") + if sqlMode != "" { + err = iohelp.WriteLine(writer, fmt.Sprintf("SET @@SQL_MODE='%s';", sqlMode)) if err != nil { return err } @@ -399,7 +398,7 @@ func dumpViews(ctx *sql.Context, engine *engine.SqlEngine, root *doltdb.RootValu typeColIdx := sch.IndexOfColName(doltdb.SchemasTablesTypeCol) fragColIdx := sch.IndexOfColName(doltdb.SchemasTablesFragmentCol) nameColIdx := sch.IndexOfColName(doltdb.SchemasTablesNameCol) - ansiQuotesColIdx := sch.IndexOfColName(doltdb.SchemasTablesAnsiQuotesCol) + sqlModeIdx := sch.IndexOfColName(doltdb.SchemasTablesSqlModeCol) defer func(iter sql.RowIter, context *sql.Context) { err := iter.Close(context) @@ -420,25 +419,23 @@ func dumpViews(ctx *sql.Context, engine *engine.SqlEngine, root *doltdb.RootValu continue } - ansiQuotesMode := false - if row[ansiQuotesColIdx].(int8) > 0 { - ansiQuotesMode = true + sqlMode := "" + if s, ok := row[sqlModeIdx].(string); ok { + sqlMode = s } // We used to store just the SELECT part of a view, but now we store the entire CREATE VIEW statement - cv, err := parse.ParseWithOptions(ctx, row[fragColIdx].(string), sqlparser.ParserOptions{ - AnsiQuotes: ansiQuotesMode, - }) + cv, err := parse.ParseWithOptions(ctx, row[fragColIdx].(string), sql.NewSqlModeFromString(sqlMode).ParserOptions()) if err != nil { return err } - if ansiQuotesMode { + if sqlMode != "" { err := iohelp.WriteLine(writer, "SET @previousSqlMode=@@SQL_MODE;") if err != nil { return err } - err = iohelp.WriteLine(writer, "SET @@SQL_MODE=CONCAT(@@SQL_MODE, ',ANSI_QUOTES');") + err = iohelp.WriteLine(writer, fmt.Sprintf("SET @@SQL_MODE='%s';", sqlMode)) if err != nil { return err } @@ -457,7 +454,7 @@ func dumpViews(ctx *sql.Context, engine *engine.SqlEngine, root *doltdb.RootValu } } - if ansiQuotesMode { + if sqlMode != "" { err := iohelp.WriteLine(writer, "SET @@SQL_MODE=@previousSqlMode;") if err != nil { return err diff --git a/go/libraries/doltcore/doltdb/system_table.go b/go/libraries/doltcore/doltdb/system_table.go index c0e6252861..3dc35f7eb9 100644 --- a/go/libraries/doltcore/doltdb/system_table.go +++ b/go/libraries/doltcore/doltdb/system_table.go @@ -254,10 +254,10 @@ const ( // SchemasTablesExtraCol The name of the column that stores extra information about a schema element in the // dolt_schemas table SchemasTablesExtraCol = "extra" - // SchemasTablesAnsiQuotesCol is the name of the column that stores whether this fragment needs to be parsed - // in ANSI_QUOTES mode to treat double quote chars as identifier quotes, instead of string literal quotes. - SchemasTablesAnsiQuotesCol = "ansi_quotes" - SchemasTablesIndexName = "fragment_name" + // SchemasTablesSqlModeCol is the name of the column that stores the SQL_MODE string used when this fragment + // was originally defined. Mode settings, such as ANSI_QUOTES, are needed to correctly parse the fragment. + SchemasTablesSqlModeCol = "sql_mode" + SchemasTablesIndexName = "fragment_name" ) const ( @@ -332,7 +332,7 @@ const ( ProceduresTableCreatedAtCol = "created_at" // ProceduresTableModifiedAtCol is the time that the stored procedure was last modified, in UTC. ProceduresTableModifiedAtCol = "modified_at" - // ProceduresTableAnsiQuotesCol is whether this stored procedure was defined in ANSI_QUOTES mode, indicating - // that double quotes should be treated as identifier quotes, instead of string literal quotes. - ProceduresTableAnsiQuotesCol = "ansi_quotes" + // ProceduresTableSqlModeCol is the name of the column that stores the SQL_MODE string used when this fragment + // was originally defined. Mode settings, such as ANSI_QUOTES, are needed to correctly parse the fragment. + ProceduresTableSqlModeCol = "sql_mode" ) diff --git a/go/libraries/doltcore/schema/reserved_tags.go b/go/libraries/doltcore/schema/reserved_tags.go index 43cfa4efcd..92a5a0ebd1 100644 --- a/go/libraries/doltcore/schema/reserved_tags.go +++ b/go/libraries/doltcore/schema/reserved_tags.go @@ -79,7 +79,7 @@ const ( DoltSchemasNameTag DoltSchemasFragmentTag DoltSchemasExtraTag - DoltSchemasAnsiQuotesTag + DoltSchemasSqlModeTag ) // Tags for hidden columns in keyless rows @@ -94,7 +94,7 @@ const ( DoltProceduresCreateStmtTag DoltProceduresCreatedAtTag DoltProceduresModifiedAtTag - DoltProceduresAnsiQuotesTag + DoltProceduresSqlModeTag ) const ( diff --git a/go/libraries/doltcore/sqle/database.go b/go/libraries/doltcore/sqle/database.go index d6251aa30b..8c2594e4ec 100644 --- a/go/libraries/doltcore/sqle/database.go +++ b/go/libraries/doltcore/sqle/database.go @@ -27,7 +27,6 @@ import ( "github.com/dolthub/go-mysql-server/sql/parse" "github.com/dolthub/go-mysql-server/sql/plan" "github.com/dolthub/go-mysql-server/sql/types" - "github.com/dolthub/vitess/go/vt/sqlparser" "gopkg.in/src-d/go-errors.v1" "github.com/dolthub/dolt/go/libraries/doltcore/branch_control" @@ -1154,7 +1153,7 @@ func getViewDefinitionFromSchemaFragmentsOfView(ctx *sql.Context, tbl *WritableD var views = make([]sql.ViewDefinition, len(fragments)) for i, fragment := range fragments { cv, err := parse.ParseWithOptions(ctx, fragments[i].fragment, - sqlparser.ParserOptions{AnsiQuotes: fragment.ansiQuotes}) + sql.NewSqlModeFromString(fragment.sqlMode).ParserOptions()) if err != nil { return nil, sql.ViewDefinition{}, false, err } @@ -1162,7 +1161,7 @@ func getViewDefinitionFromSchemaFragmentsOfView(ctx *sql.Context, tbl *WritableD createView, ok := cv.(*plan.CreateView) if ok { views[i] = sql.ViewDefinition{Name: fragments[i].name, TextDefinition: createView.Definition.TextDefinition, - CreateViewStatement: fragments[i].fragment, AnsiQuotes: fragment.ansiQuotes} + CreateViewStatement: fragments[i].fragment, SqlMode: fragment.sqlMode} } else { views[i] = sql.ViewDefinition{Name: fragments[i].name, TextDefinition: fragments[i].fragment, CreateViewStatement: fmt.Sprintf("CREATE VIEW %s AS %s", fragments[i].name, fragments[i].fragment)} } @@ -1231,7 +1230,7 @@ func (db Database) GetTriggers(ctx *sql.Context) ([]sql.TriggerDefinition, error Name: frag.name, CreateStatement: frag.fragment, CreatedAt: frag.created, - AnsiQuotes: frag.ansiQuotes, + SqlMode: frag.sqlMode, }) } if err != nil { @@ -1273,14 +1272,13 @@ func (db Database) GetEvent(ctx *sql.Context, name string) (sql.EventDefinition, return sql.EventDefinition{}, false, err } - // TODO: Some duplication with GetEvents here... would be nice to clean up for _, frag := range frags { if strings.ToLower(frag.name) == strings.ToLower(name) { return sql.EventDefinition{ Name: frag.name, CreateStatement: frag.fragment, CreatedAt: frag.created, - AnsiQuotes: frag.ansiQuotes, + SqlMode: frag.sqlMode, }, true, nil } } @@ -1308,7 +1306,7 @@ func (db Database) GetEvents(ctx *sql.Context) ([]sql.EventDefinition, error) { Name: frag.name, CreateStatement: frag.fragment, CreatedAt: frag.created, - AnsiQuotes: frag.ansiQuotes, + SqlMode: frag.sqlMode, }) } return events, nil diff --git a/go/libraries/doltcore/sqle/procedures_table.go b/go/libraries/doltcore/sqle/procedures_table.go index e05a72718d..e81cf3e37c 100644 --- a/go/libraries/doltcore/sqle/procedures_table.go +++ b/go/libraries/doltcore/sqle/procedures_table.go @@ -59,7 +59,7 @@ func ProceduresTableSchema() schema.Schema { schema.NewColumn(doltdb.ProceduresTableCreateStmtCol, schema.DoltProceduresCreateStmtTag, types.StringKind, false), schema.NewColumn(doltdb.ProceduresTableCreatedAtCol, schema.DoltProceduresCreatedAtTag, types.TimestampKind, false), schema.NewColumn(doltdb.ProceduresTableModifiedAtCol, schema.DoltProceduresModifiedAtTag, types.TimestampKind, false), - schema.NewColumn(doltdb.ProceduresTableAnsiQuotesCol, schema.DoltProceduresAnsiQuotesTag, types.BoolKind, false), + schema.NewColumn(doltdb.ProceduresTableSqlModeCol, schema.DoltProceduresSqlModeTag, types.BoolKind, false), ) return schema.MustSchemaFromCols(colColl) } @@ -72,19 +72,13 @@ func DoltProceduresGetOrCreateTable(ctx *sql.Context, db Database) (*WritableDol return nil, err } if found { - // If the table we found doesn't have the same number of fields in its schema, - // then add any schema fields from the end that are missing. - alterableDoltTable := tbl.(*AlterableDoltTable) + // Make sure the schema is up to date + writableDoltTable := tbl.(*WritableDoltTable) targetSchema := ProceduresTableSqlSchema().Schema - for i := len(tbl.Schema()); i < len(targetSchema); i++ { - columnOrder := sql.ColumnOrder{} - err := alterableDoltTable.AddColumn(ctx, targetSchema[i], &columnOrder) - if err != nil { - return nil, err - } + if len(tbl.Schema()) != len(targetSchema) { + return migrateDoltProceduresSchema(ctx, db, writableDoltTable) } - - return tbl.(*WritableDoltTable), nil + return writableDoltTable, nil } root, err := db.GetRoot(ctx) @@ -107,6 +101,88 @@ func DoltProceduresGetOrCreateTable(ctx *sql.Context, db Database) (*WritableDol return tbl.(*WritableDoltTable), nil } +// TODO: godocs +// TODO: what's the best way to test this? backwards compat test suite? +func migrateDoltProceduresSchema(ctx *sql.Context, db Database, oldTable *WritableDoltTable) (newTable *WritableDoltTable, rerr error) { + // Copy all the old data + iter, err := SqlTableToRowIter(ctx, oldTable.DoltTable, nil) + if err != nil { + return nil, err + } + + nameIdx := oldTable.sqlSchema().IndexOfColName(doltdb.ProceduresTableNameCol) + createStatementIdx := oldTable.sqlSchema().IndexOfColName(doltdb.ProceduresTableCreateStmtCol) + createdAtIdx := oldTable.sqlSchema().IndexOfColName(doltdb.ProceduresTableCreatedAtCol) + modifiedAtIdx := oldTable.sqlSchema().IndexOfColName(doltdb.ProceduresTableModifiedAtCol) + sqlModeIdx := oldTable.sqlSchema().IndexOfColName(doltdb.ProceduresTableSqlModeCol) + + defer func(iter sql.RowIter, ctx *sql.Context) { + err := iter.Close(ctx) + if err != nil && rerr == nil { + rerr = err + } + }(iter, ctx) + + var newRows []sql.Row + for { + sqlRow, err := iter.Next(ctx) + if err == io.EOF { + break + } + if err != nil { + return nil, err + } + + newRow := make(sql.Row, ProceduresTableSchema().GetAllCols().Size()) + newRow[0] = sqlRow[nameIdx] + newRow[1] = sqlRow[createStatementIdx] + newRow[2] = sqlRow[createdAtIdx] + newRow[3] = sqlRow[modifiedAtIdx] + if sqlModeIdx >= 0 { + newRow[4] = sqlRow[sqlModeIdx] + } + newRows = append(newRows, newRow) + } + + err = db.dropTable(ctx, doltdb.ProceduresTableName) + if err != nil { + return nil, err + } + + root, err := db.GetRoot(ctx) + if err != nil { + return nil, err + } + + err = db.createDoltTable(ctx, doltdb.ProceduresTableName, root, ProceduresTableSchema()) + if err != nil { + return nil, err + } + + tbl, found, err := db.GetTableInsensitive(ctx, doltdb.ProceduresTableName) + if err != nil { + return nil, err + } + if !found { + return nil, sql.ErrTableNotFound.New(doltdb.ProceduresTableName) + } + + inserter := tbl.(*WritableDoltTable).Inserter(ctx) + for _, row := range newRows { + err = inserter.Insert(ctx, row) + if err != nil { + return nil, err + } + } + + err = inserter.Close(ctx) + if err != nil { + return nil, err + } + + return tbl.(*WritableDoltTable), nil +} + // DoltProceduresGetTable returns the `dolt_procedures` table from the given db, or nil if the table doesn't exist func DoltProceduresGetTable(ctx *sql.Context, db Database) (*WritableDoltTable, error) { tbl, found, err := db.GetTableInsensitive(ctx, doltdb.ProceduresTableName) @@ -192,8 +268,8 @@ func DoltProceduresGetAll(ctx *sql.Context, db Database, procedureName string) ( if d.ModifiedAt, ok = sqlRow[3].(time.Time); !ok { return nil, missingValue.New(doltdb.ProceduresTableModifiedAtCol, sqlRow) } - if sqlRow[4].(int8) > 0 { - d.AnsiQuotes = true + if s, ok := sqlRow[4].(string); ok { + d.SqlMode = s } details = append(details, d) } @@ -226,7 +302,7 @@ func DoltProceduresAddProcedure(ctx *sql.Context, db Database, spd sql.StoredPro spd.CreateStatement, spd.CreatedAt.UTC(), spd.ModifiedAt.UTC(), - spd.AnsiQuotes, + spd.SqlMode, }) } diff --git a/go/libraries/doltcore/sqle/schema_table.go b/go/libraries/doltcore/sqle/schema_table.go index 6ff08564e4..dd06fe07c2 100644 --- a/go/libraries/doltcore/sqle/schema_table.go +++ b/go/libraries/doltcore/sqle/schema_table.go @@ -61,7 +61,7 @@ var schemasTableCols = schema.NewColCollection( mustNewColWithTypeInfo(doltdb.SchemasTablesNameCol, schema.DoltSchemasNameTag, typeinfo.CreateVarStringTypeFromSqlType(mustCreateStringType(query.Type_VARCHAR, 64, sql.Collation_utf8mb4_0900_ai_ci)), true, "", false, ""), mustNewColWithTypeInfo(doltdb.SchemasTablesFragmentCol, schema.DoltSchemasFragmentTag, typeinfo.CreateVarStringTypeFromSqlType(gmstypes.LongText), false, "", false, ""), mustNewColWithTypeInfo(doltdb.SchemasTablesExtraCol, schema.DoltSchemasExtraTag, typeinfo.JSONType, false, "", false, ""), - mustNewColWithTypeInfo(doltdb.SchemasTablesAnsiQuotesCol, schema.DoltSchemasAnsiQuotesTag, typeinfo.BoolType, false, "false", false, ""), + mustNewColWithTypeInfo(doltdb.SchemasTablesSqlModeCol, schema.DoltSchemasSqlModeTag, typeinfo.BoolType, false, "false", false, ""), ) var schemaTableSchema = schema.MustSchemaFromCols(schemasTableCols) @@ -118,7 +118,7 @@ func migrateOldSchemasTableToNew(ctx *sql.Context, db Database, schemasTable *Wr typeIdx := schemasTable.sqlSchema().IndexOfColName(doltdb.SchemasTablesTypeCol) fragmentIdx := schemasTable.sqlSchema().IndexOfColName(doltdb.SchemasTablesFragmentCol) extraIdx := schemasTable.sqlSchema().IndexOfColName(doltdb.SchemasTablesExtraCol) - ansiQuotesIdx := schemasTable.sqlSchema().IndexOfColName(doltdb.SchemasTablesAnsiQuotesCol) + sqlModeIdx := schemasTable.sqlSchema().IndexOfColName(doltdb.SchemasTablesSqlModeCol) defer func(iter sql.RowIter, ctx *sql.Context) { err := iter.Close(ctx) @@ -144,8 +144,8 @@ func migrateOldSchemasTableToNew(ctx *sql.Context, db Database, schemasTable *Wr if extraIdx >= 0 { newRow[3] = sqlRow[extraIdx] } - if ansiQuotesIdx >= 0 { - newRow[4] = sqlRow[ansiQuotesIdx] + if sqlModeIdx >= 0 { + newRow[4] = sqlRow[sqlModeIdx] } newRows = append(newRows, newRow) @@ -234,9 +234,9 @@ type schemaFragment struct { name string fragment string created time.Time - // ansiQuotes indicates if this fragment must be parsed with ANSI_QUOTES mode, meaning it uses double quote - // characters as identifier quotes, instead of string literal quotes. - ansiQuotes bool + // sqlMode indicates the SQL_MODE that was used when this schema fragment was initially parsed. SQL_MODE settings + // such as ANSI_QUOTES control customized parsing behavior needed for some schema fragments. + sqlMode string } func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType string) (sf []schemaFragment, rerr error) { @@ -251,7 +251,7 @@ func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType typeIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesTypeCol) fragmentIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesFragmentCol) extraIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesExtraCol) - ansiQuotesIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesAnsiQuotesCol) + sqlModeIdx := tbl.sqlSchema().IndexOfColName(doltdb.SchemasTablesSqlModeCol) defer func(iter sql.RowIter, ctx *sql.Context) { err := iter.Close(ctx) @@ -274,20 +274,20 @@ func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType continue } - ansiQuotes := false - if ansiQuotesIdx >= 0 { - if sqlRow[ansiQuotesIdx].(int8) > 0 { - ansiQuotes = true + sqlModeString := "" + if sqlModeIdx >= 0 { + if s, ok := sqlRow[sqlModeIdx].(string); ok { + sqlModeString = s } } // For older tables, use 1 as the trigger creation time if extraIdx < 0 || sqlRow[extraIdx] == nil { frags = append(frags, schemaFragment{ - name: sqlRow[nameIdx].(string), - fragment: sqlRow[fragmentIdx].(string), - created: time.Unix(1, 0).UTC(), // TablePlus editor thinks 0 is out of range - ansiQuotes: ansiQuotes, + name: sqlRow[nameIdx].(string), + fragment: sqlRow[fragmentIdx].(string), + created: time.Unix(1, 0).UTC(), // TablePlus editor thinks 0 is out of range + sqlMode: sqlModeString, }) continue } @@ -296,10 +296,10 @@ func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType createdTime, err := getCreatedTime(ctx, sqlRow[extraIdx].(gmstypes.JSONValue)) frags = append(frags, schemaFragment{ - name: sqlRow[nameIdx].(string), - fragment: sqlRow[fragmentIdx].(string), - created: time.Unix(createdTime, 0).UTC(), - ansiQuotes: ansiQuotes, + name: sqlRow[nameIdx].(string), + fragment: sqlRow[fragmentIdx].(string), + created: time.Unix(createdTime, 0).UTC(), + sqlMode: sqlModeString, }) } From 1bf620a9a8ea566216f1ca840712414f9e5aa690 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Mon, 7 Aug 2023 17:29:54 -0700 Subject: [PATCH 07/28] Bumping GMS version to latest from fulghum/ansi_quotes dev branch --- go/go.mod | 2 +- go/go.sum | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/go/go.mod b/go/go.mod index 8a38e80f36..e08f107e41 100644 --- a/go/go.mod +++ b/go/go.mod @@ -59,7 +59,7 @@ require ( github.com/cespare/xxhash v1.1.0 github.com/creasty/defaults v1.6.0 github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 - github.com/dolthub/go-mysql-server v0.16.1-0.20230804191758-46bed6efacdc + github.com/dolthub/go-mysql-server v0.16.1-0.20230807211358-5b6031f113ba github.com/dolthub/swiss v0.1.0 github.com/goccy/go-json v0.10.2 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 diff --git a/go/go.sum b/go/go.sum index df0d177501..86f20cd952 100644 --- a/go/go.sum +++ b/go/go.sum @@ -182,6 +182,8 @@ github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e h1:kPsT4a47cw github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e/go.mod h1:KPUcpx070QOfJK1gNe0zx4pA5sicIK1GMikIGLKC168= github.com/dolthub/go-mysql-server v0.16.1-0.20230804191758-46bed6efacdc h1:XAfRHQoOoF2hYEgG0ADYJgjf7tgcUlGIhM7FpY9a1sI= github.com/dolthub/go-mysql-server v0.16.1-0.20230804191758-46bed6efacdc/go.mod h1:nY1J1sV2kuGJbAZ6bcZARw4dF8TD3KpEfYVVs/HK/JY= +github.com/dolthub/go-mysql-server v0.16.1-0.20230807211358-5b6031f113ba h1:ImvGNMKqAA+KPH5rXTR1xmOVQYv9XC7p6AjX8RhkWMU= +github.com/dolthub/go-mysql-server v0.16.1-0.20230807211358-5b6031f113ba/go.mod h1:nY1J1sV2kuGJbAZ6bcZARw4dF8TD3KpEfYVVs/HK/JY= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488 h1:0HHu0GWJH0N6a6keStrHhUAK5/o9LVfkh44pvsV4514= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488/go.mod h1:ehexgi1mPxRTk0Mok/pADALuHbvATulTh6gzr7NzZto= github.com/dolthub/jsonpath v0.0.2-0.20230525180605-8dc13778fd72 h1:NfWmngMi1CYUWU4Ix8wM+USEhjc+mhPlT9JUR/anvbQ= From 8e51bdc91bf663d76e51ffe72790e7b2e7463e25 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 8 Aug 2023 08:15:06 -0700 Subject: [PATCH 08/28] Reverting bool support that isn't needed any more --- go/libraries/doltcore/sqle/index/prolly_fields.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/go/libraries/doltcore/sqle/index/prolly_fields.go b/go/libraries/doltcore/sqle/index/prolly_fields.go index d19ef27f93..3b3ee5385e 100644 --- a/go/libraries/doltcore/sqle/index/prolly_fields.go +++ b/go/libraries/doltcore/sqle/index/prolly_fields.go @@ -229,12 +229,6 @@ func PutField(ctx context.Context, ns tree.NodeStore, tb *val.TupleBuilder, i in func convInt(v interface{}) int { switch i := v.(type) { - case bool: - if i { - return 1 - } else { - return 0 - } case int: return i case int8: From 8302cdeb9f71a6f99df705e4f54d87127ac282d2 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 8 Aug 2023 08:21:53 -0700 Subject: [PATCH 09/28] Fixing type for sql_mode columns --- go/libraries/doltcore/sqle/procedures_table.go | 2 +- go/libraries/doltcore/sqle/schema_table.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/go/libraries/doltcore/sqle/procedures_table.go b/go/libraries/doltcore/sqle/procedures_table.go index e81cf3e37c..bc50dfeba6 100644 --- a/go/libraries/doltcore/sqle/procedures_table.go +++ b/go/libraries/doltcore/sqle/procedures_table.go @@ -59,7 +59,7 @@ func ProceduresTableSchema() schema.Schema { schema.NewColumn(doltdb.ProceduresTableCreateStmtCol, schema.DoltProceduresCreateStmtTag, types.StringKind, false), schema.NewColumn(doltdb.ProceduresTableCreatedAtCol, schema.DoltProceduresCreatedAtTag, types.TimestampKind, false), schema.NewColumn(doltdb.ProceduresTableModifiedAtCol, schema.DoltProceduresModifiedAtTag, types.TimestampKind, false), - schema.NewColumn(doltdb.ProceduresTableSqlModeCol, schema.DoltProceduresSqlModeTag, types.BoolKind, false), + schema.NewColumn(doltdb.ProceduresTableSqlModeCol, schema.DoltProceduresSqlModeTag, types.StringKind, false), ) return schema.MustSchemaFromCols(colColl) } diff --git a/go/libraries/doltcore/sqle/schema_table.go b/go/libraries/doltcore/sqle/schema_table.go index dd06fe07c2..123dc16f4a 100644 --- a/go/libraries/doltcore/sqle/schema_table.go +++ b/go/libraries/doltcore/sqle/schema_table.go @@ -61,7 +61,7 @@ var schemasTableCols = schema.NewColCollection( mustNewColWithTypeInfo(doltdb.SchemasTablesNameCol, schema.DoltSchemasNameTag, typeinfo.CreateVarStringTypeFromSqlType(mustCreateStringType(query.Type_VARCHAR, 64, sql.Collation_utf8mb4_0900_ai_ci)), true, "", false, ""), mustNewColWithTypeInfo(doltdb.SchemasTablesFragmentCol, schema.DoltSchemasFragmentTag, typeinfo.CreateVarStringTypeFromSqlType(gmstypes.LongText), false, "", false, ""), mustNewColWithTypeInfo(doltdb.SchemasTablesExtraCol, schema.DoltSchemasExtraTag, typeinfo.JSONType, false, "", false, ""), - mustNewColWithTypeInfo(doltdb.SchemasTablesSqlModeCol, schema.DoltSchemasSqlModeTag, typeinfo.BoolType, false, "false", false, ""), + mustNewColWithTypeInfo(doltdb.SchemasTablesSqlModeCol, schema.DoltSchemasSqlModeTag, typeinfo.CreateVarStringTypeFromSqlType(mustCreateStringType(query.Type_VARCHAR, 256, sql.Collation_utf8mb4_0900_ai_ci)), false, "false", false, ""), ) var schemaTableSchema = schema.MustSchemaFromCols(schemasTableCols) From 72d46b9b9549e63ba871dc3c425dd6614ea3c8ee Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 8 Aug 2023 08:26:15 -0700 Subject: [PATCH 10/28] Bug fix from converting AnsiQuotes:bool -> SqlMode:String --- go/libraries/doltcore/sqle/database.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/libraries/doltcore/sqle/database.go b/go/libraries/doltcore/sqle/database.go index 8c2594e4ec..635d468e26 100644 --- a/go/libraries/doltcore/sqle/database.go +++ b/go/libraries/doltcore/sqle/database.go @@ -1410,7 +1410,7 @@ func (db Database) addFragToSchemasTable(ctx *sql.Context, fragType, name, defin return err } - return inserter.Insert(ctx, sql.Row{fragType, name, definition, extraJSON, sqlMode.AnsiQuotes()}) + return inserter.Insert(ctx, sql.Row{fragType, name, definition, extraJSON, sqlMode.String()}) } func (db Database) dropFragFromSchemasTable(ctx *sql.Context, fragType, name string, missingErr error) error { From 75b80fd476a7b3d94fd33a627e6f7ae5ecd61ddd Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 8 Aug 2023 09:23:54 -0700 Subject: [PATCH 11/28] Fixing more tests --- go/libraries/doltcore/sqle/schema_table.go | 2 +- integration-tests/bats/triggers.bats | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/go/libraries/doltcore/sqle/schema_table.go b/go/libraries/doltcore/sqle/schema_table.go index 123dc16f4a..1b1ad15713 100644 --- a/go/libraries/doltcore/sqle/schema_table.go +++ b/go/libraries/doltcore/sqle/schema_table.go @@ -61,7 +61,7 @@ var schemasTableCols = schema.NewColCollection( mustNewColWithTypeInfo(doltdb.SchemasTablesNameCol, schema.DoltSchemasNameTag, typeinfo.CreateVarStringTypeFromSqlType(mustCreateStringType(query.Type_VARCHAR, 64, sql.Collation_utf8mb4_0900_ai_ci)), true, "", false, ""), mustNewColWithTypeInfo(doltdb.SchemasTablesFragmentCol, schema.DoltSchemasFragmentTag, typeinfo.CreateVarStringTypeFromSqlType(gmstypes.LongText), false, "", false, ""), mustNewColWithTypeInfo(doltdb.SchemasTablesExtraCol, schema.DoltSchemasExtraTag, typeinfo.JSONType, false, "", false, ""), - mustNewColWithTypeInfo(doltdb.SchemasTablesSqlModeCol, schema.DoltSchemasSqlModeTag, typeinfo.CreateVarStringTypeFromSqlType(mustCreateStringType(query.Type_VARCHAR, 256, sql.Collation_utf8mb4_0900_ai_ci)), false, "false", false, ""), + mustNewColWithTypeInfo(doltdb.SchemasTablesSqlModeCol, schema.DoltSchemasSqlModeTag, typeinfo.CreateVarStringTypeFromSqlType(mustCreateStringType(query.Type_VARCHAR, 256, sql.Collation_utf8mb4_0900_ai_ci)), false, "", false, ""), ) var schemaTableSchema = schema.MustSchemaFromCols(schemasTableCols) diff --git a/integration-tests/bats/triggers.bats b/integration-tests/bats/triggers.bats index fe91d16d25..e337706f13 100644 --- a/integration-tests/bats/triggers.bats +++ b/integration-tests/bats/triggers.bats @@ -122,7 +122,7 @@ SQL @test "triggers: Writing directly into dolt_schemas" { dolt sql -q "CREATE TABLE test(pk BIGINT PRIMARY KEY, v1 BIGINT);" dolt sql -q "CREATE VIEW view1 AS SELECT v1 FROM test;" - dolt sql -q "INSERT INTO dolt_schemas VALUES ('trigger', 'trigger1', 'CREATE TRIGGER trigger1 BEFORE INSERT ON test FOR EACH ROW SET new.v1 = -new.v1;', json_object('CreatedAt', 1));" + dolt sql -q "INSERT INTO dolt_schemas VALUES ('trigger', 'trigger1', 'CREATE TRIGGER trigger1 BEFORE INSERT ON test FOR EACH ROW SET new.v1 = -new.v1;', json_object('CreatedAt', 1), NULL);" dolt sql -q "INSERT INTO test VALUES (1, 1);" run dolt sql -q "SELECT * FROM test" -r=csv [ "$status" -eq "0" ] From 0d640b26706d16b10e0193f7336e067222344f55 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 8 Aug 2023 09:37:35 -0700 Subject: [PATCH 12/28] Fixing more tests --- go/libraries/doltcore/sqle/schema_table_test.go | 8 ++++---- go/libraries/doltcore/sqle/sqlreplace_test.go | 6 +++--- go/libraries/doltcore/sqle/sqlselect_test.go | 4 ++-- go/libraries/doltcore/sqle/sqlupdate_test.go | 4 ++-- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/go/libraries/doltcore/sqle/schema_table_test.go b/go/libraries/doltcore/sqle/schema_table_test.go index 034cd752b8..9ba38c76b7 100644 --- a/go/libraries/doltcore/sqle/schema_table_test.go +++ b/go/libraries/doltcore/sqle/schema_table_test.go @@ -80,8 +80,8 @@ func TestSchemaTableMigrationOriginal(t *testing.T) { require.NoError(t, iter.Close(ctx)) expectedRows := []sql.Row{ - {"view", "view1", "SELECT v1 FROM test;", nil}, - {"view", "view2", "SELECT v2 FROM test;", nil}, + {"view", "view1", "SELECT v1 FROM test;", nil, nil}, + {"view", "view2", "SELECT v2 FROM test;", nil, nil}, } assert.Equal(t, expectedRows, rows) @@ -159,8 +159,8 @@ func TestSchemaTableMigrationV1(t *testing.T) { require.NoError(t, iter.Close(ctx)) expectedRows := []sql.Row{ - {"view", "view1", "SELECT v1 FROM test;", `{"extra":"data"}`}, - {"view", "view2", "SELECT v2 FROM test;", nil}, + {"view", "view1", "SELECT v1 FROM test;", `{"extra":"data"}`, nil}, + {"view", "view2", "SELECT v2 FROM test;", nil, nil}, } assert.Equal(t, expectedRows, rows) diff --git a/go/libraries/doltcore/sqle/sqlreplace_test.go b/go/libraries/doltcore/sqle/sqlreplace_test.go index 93207e43df..95e60de214 100644 --- a/go/libraries/doltcore/sqle/sqlreplace_test.go +++ b/go/libraries/doltcore/sqle/sqlreplace_test.go @@ -273,10 +273,10 @@ var systemTableReplaceTests = []ReplaceTest{ { Name: "replace into dolt_schemas", AdditionalSetup: CreateTableFn(doltdb.SchemasTableName, schemaTableSchema, - "INSERT INTO dolt_schemas VALUES ('view', 'name', 'create view name as select 2+2 from dual', NULL)"), + "INSERT INTO dolt_schemas VALUES ('view', 'name', 'create view name as select 2+2 from dual', NULL, NULL)"), ReplaceQuery: "replace into dolt_schemas (type, name, fragment) values ('view', 'name', 'create view name as select 1+1 from dual')", - SelectQuery: "select type, name, fragment, extra from dolt_schemas", - ExpectedRows: []sql.Row{{"view", "name", "create view name as select 1+1 from dual", nil}}, + SelectQuery: "select type, name, fragment, extra, sql_mode from dolt_schemas", + ExpectedRows: []sql.Row{{"view", "name", "create view name as select 1+1 from dual", nil, nil}}, ExpectedSchema: CompressSchema(schemaTableSchema), }, } diff --git a/go/libraries/doltcore/sqle/sqlselect_test.go b/go/libraries/doltcore/sqle/sqlselect_test.go index 9ba8fb3ea4..5df18881a4 100644 --- a/go/libraries/doltcore/sqle/sqlselect_test.go +++ b/go/libraries/doltcore/sqle/sqlselect_test.go @@ -1301,9 +1301,9 @@ var systemTableSelectTests = []SelectTest{ { Name: "select from dolt_schemas", AdditionalSetup: CreateTableFn(doltdb.SchemasTableName, schemaTableSchema, - `INSERT INTO dolt_schemas VALUES ('view', 'name', 'create view name as select 2+2 from dual', NULL)`), + `INSERT INTO dolt_schemas VALUES ('view', 'name', 'create view name as select 2+2 from dual', NULL, NULL)`), Query: "select * from dolt_schemas", - ExpectedRows: []sql.Row{{"view", "name", "create view name as select 2+2 from dual", nil}}, + ExpectedRows: []sql.Row{{"view", "name", "create view name as select 2+2 from dual", nil, nil}}, ExpectedSchema: CompressSchema(schemaTableSchema), }, } diff --git a/go/libraries/doltcore/sqle/sqlupdate_test.go b/go/libraries/doltcore/sqle/sqlupdate_test.go index 66f3fcca13..d8b4a764a6 100644 --- a/go/libraries/doltcore/sqle/sqlupdate_test.go +++ b/go/libraries/doltcore/sqle/sqlupdate_test.go @@ -378,10 +378,10 @@ var systemTableUpdateTests = []UpdateTest{ { Name: "update dolt_schemas", AdditionalSetup: CreateTableFn(doltdb.SchemasTableName, schemaTableSchema, - `INSERT INTO dolt_schemas VALUES ('view', 'name', 'create view name as select 2+2 from dual', NULL)`), + `INSERT INTO dolt_schemas VALUES ('view', 'name', 'create view name as select 2+2 from dual', NULL, NULL)`), UpdateQuery: "update dolt_schemas set type = 'not a view'", SelectQuery: "select * from dolt_schemas", - ExpectedRows: []sql.Row{{"not a view", "name", "create view name as select 2+2 from dual", nil}}, + ExpectedRows: []sql.Row{{"not a view", "name", "create view name as select 2+2 from dual", nil, nil}}, ExpectedSchema: CompressSchema(schemaTableSchema), }, } From d9447f1b316ed43200c1e39064668a7db749bab1 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 8 Aug 2023 11:31:12 -0700 Subject: [PATCH 13/28] Bug fix for missing single quote --- go/cmd/dolt/commands/dump.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/cmd/dolt/commands/dump.go b/go/cmd/dolt/commands/dump.go index b90c658a87..702d1c6008 100644 --- a/go/cmd/dolt/commands/dump.go +++ b/go/cmd/dolt/commands/dump.go @@ -294,7 +294,7 @@ func dumpProcedures(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb. } if sqlMode != "" { - err = iohelp.WriteLine(writer, fmt.Sprintf("SET @@SQL_MODE='%s;", sqlMode)) + err = iohelp.WriteLine(writer, fmt.Sprintf("SET @@SQL_MODE='%s';", sqlMode)) if err != nil { return err } From 13ef7c4f3425162e790b9abe72bb93b17d0c0843 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Wed, 9 Aug 2023 10:22:12 -0700 Subject: [PATCH 14/28] Cleaning up dump.go --- go/cmd/dolt/commands/dump.go | 64 +++++++++++++++++++++--------------- 1 file changed, 38 insertions(+), 26 deletions(-) diff --git a/go/cmd/dolt/commands/dump.go b/go/cmd/dolt/commands/dump.go index 702d1c6008..8ee5f8d8a7 100644 --- a/go/cmd/dolt/commands/dump.go +++ b/go/cmd/dolt/commands/dump.go @@ -293,11 +293,8 @@ func dumpProcedures(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb. } } - if sqlMode != "" { - err = iohelp.WriteLine(writer, fmt.Sprintf("SET @@SQL_MODE='%s';", sqlMode)) - if err != nil { - return err - } + if err := changeSqlMode(writer, sqlMode); err != nil { + return err } err = iohelp.WriteLine(writer, "delimiter END_PROCEDURE") @@ -314,6 +311,10 @@ func dumpProcedures(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb. if err != nil { return err } + + if err := resetSqlMode(writer); err != nil { + return err + } } return nil @@ -362,19 +363,18 @@ func dumpTriggers(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb.Ro sqlMode = s } - // TODO: helper functions? - // TODO: Need to clean up how we set SQL_MODE in the output dump file - if sqlMode != "" { - err = iohelp.WriteLine(writer, fmt.Sprintf("SET @@SQL_MODE='%s';", sqlMode)) - if err != nil { - return err - } + if err := changeSqlMode(writer, sqlMode); err != nil { + return err } err = iohelp.WriteLine(writer, fmt.Sprintf("%s;", row[fragColIdx])) if err != nil { return err } + + if err := resetSqlMode(writer); err != nil { + return err + } } return nil @@ -430,15 +430,8 @@ func dumpViews(ctx *sql.Context, engine *engine.SqlEngine, root *doltdb.RootValu return err } - if sqlMode != "" { - err := iohelp.WriteLine(writer, "SET @previousSqlMode=@@SQL_MODE;") - if err != nil { - return err - } - err = iohelp.WriteLine(writer, fmt.Sprintf("SET @@SQL_MODE='%s';", sqlMode)) - if err != nil { - return err - } + if err := changeSqlMode(writer, sqlMode); err != nil { + return err } _, ok := cv.(*plan.CreateView) @@ -454,17 +447,36 @@ func dumpViews(ctx *sql.Context, engine *engine.SqlEngine, root *doltdb.RootValu } } - if sqlMode != "" { - err := iohelp.WriteLine(writer, "SET @@SQL_MODE=@previousSqlMode;") - if err != nil { - return err - } + if err := resetSqlMode(writer); err != nil { + return err } } return nil } +// changeSqlMode outputs a SQL statement to |writer| to save the current @@SQL_MODE to a +// variable and then outputs a SQL statement to set the @@SQL_MODE to |sqlMode|. +func changeSqlMode(writer io.WriteCloser, sqlMode string) error { + if sqlMode == "" { + return nil + } + + err := iohelp.WriteLine(writer, "SET @previousSqlMode=@@SQL_MODE;") + if err != nil { + return err + } + + err = iohelp.WriteLine(writer, fmt.Sprintf("SET @@SQL_MODE='%s';", sqlMode)) + return err +} + +// resetSqlMode outputs a SQL statement to |writer| to reset @@SQL_MODE back to the +// previous value stored by the last call to changeSqlMode. +func resetSqlMode(writer io.WriteCloser) error { + return iohelp.WriteLine(writer, "SET @@SQL_MODE=@previousSqlMode;") +} + type dumpOptions struct { format string schemaOnly bool From e6036fefe7f93ced809d85ed15d091ac45a5bf57 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Wed, 9 Aug 2023 11:40:20 -0700 Subject: [PATCH 15/28] Tidying up dump output so that SQL_MODE is only set (and then reverted) if the new SQL_MODE is different from the default SQL_MODE. --- go/cmd/dolt/commands/dump.go | 70 +++++++++++++++++++++++++----------- 1 file changed, 50 insertions(+), 20 deletions(-) diff --git a/go/cmd/dolt/commands/dump.go b/go/cmd/dolt/commands/dump.go index 8ee5f8d8a7..5dcded8c59 100644 --- a/go/cmd/dolt/commands/dump.go +++ b/go/cmd/dolt/commands/dump.go @@ -293,7 +293,8 @@ func dumpProcedures(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb. } } - if err := changeSqlMode(writer, sqlMode); err != nil { + modeChanged, err := changeSqlMode(sqlCtx, writer, sqlMode) + if err != nil { return err } @@ -312,8 +313,10 @@ func dumpProcedures(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb. return err } - if err := resetSqlMode(writer); err != nil { - return err + if modeChanged { + if err := resetSqlMode(writer); err != nil { + return err + } } } @@ -363,7 +366,8 @@ func dumpTriggers(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb.Ro sqlMode = s } - if err := changeSqlMode(writer, sqlMode); err != nil { + modeChanged, err := changeSqlMode(sqlCtx, writer, sqlMode) + if err != nil { return err } @@ -372,8 +376,10 @@ func dumpTriggers(sqlCtx *sql.Context, engine *engine.SqlEngine, root *doltdb.Ro return err } - if err := resetSqlMode(writer); err != nil { - return err + if modeChanged { + if err := resetSqlMode(writer); err != nil { + return err + } } } @@ -430,7 +436,8 @@ func dumpViews(ctx *sql.Context, engine *engine.SqlEngine, root *doltdb.RootValu return err } - if err := changeSqlMode(writer, sqlMode); err != nil { + modeChanged, err := changeSqlMode(ctx, writer, sqlMode) + if err != nil { return err } @@ -447,32 +454,55 @@ func dumpViews(ctx *sql.Context, engine *engine.SqlEngine, root *doltdb.RootValu } } - if err := resetSqlMode(writer); err != nil { - return err + if modeChanged { + if err := resetSqlMode(writer); err != nil { + return err + } } } return nil } -// changeSqlMode outputs a SQL statement to |writer| to save the current @@SQL_MODE to a -// variable and then outputs a SQL statement to set the @@SQL_MODE to |sqlMode|. -func changeSqlMode(writer io.WriteCloser, sqlMode string) error { - if sqlMode == "" { - return nil +// changeSqlMode checks if the current SQL session's @@SQL_MODE is different from the requested |newSqlMode| and if so, +// outputs a SQL statement to |writer| to save the current @@SQL_MODE to the @previousSqlMode variable and then outputs +// a SQL statement to set the @@SQL_MODE to |sqlMode|. If |newSqlMode| is the identical to the current session's +// SQL_MODE (the default, global @@SQL_MODE), then no statements are written to |writer|. The boolean return code +// indicates if any statements were written. +func changeSqlMode(ctx *sql.Context, writer io.WriteCloser, newSqlMode string) (bool, error) { + if newSqlMode == "" { + return false, nil } - err := iohelp.WriteLine(writer, "SET @previousSqlMode=@@SQL_MODE;") + variable, err := ctx.Session.GetSessionVariable(ctx, "SQL_MODE") if err != nil { - return err + return false, err + } + currentSqlMode, ok := variable.(string) + if !ok { + return false, fmt.Errorf("unable to read @@SQL_MODE system variable from value '%v'", currentSqlMode) + } + + if currentSqlMode == newSqlMode { + return false, nil + } + + err = iohelp.WriteLine(writer, "SET @previousSqlMode=@@SQL_MODE;") + if err != nil { + return false, err + } + + err = iohelp.WriteLine(writer, fmt.Sprintf("SET @@SQL_MODE='%s';", newSqlMode)) + if err != nil { + return false, err } - err = iohelp.WriteLine(writer, fmt.Sprintf("SET @@SQL_MODE='%s';", sqlMode)) - return err + return true, nil } -// resetSqlMode outputs a SQL statement to |writer| to reset @@SQL_MODE back to the -// previous value stored by the last call to changeSqlMode. +// resetSqlMode outputs a SQL statement to |writer| to reset @@SQL_MODE back to the previous value stored +// by the last call to changeSqlMode. This function should only be called after changeSqlMode, otherwise the +// @previousSqlMode variable will not be set correctly. func resetSqlMode(writer io.WriteCloser) error { return iohelp.WriteLine(writer, "SET @@SQL_MODE=@previousSqlMode;") } From 53a59a74ed8b40c18cdfec3c7dc4343c729a0fdd Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Wed, 9 Aug 2023 11:49:14 -0700 Subject: [PATCH 16/28] Tidying up --- go/libraries/doltcore/sqle/procedures_table.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/go/libraries/doltcore/sqle/procedures_table.go b/go/libraries/doltcore/sqle/procedures_table.go index bc50dfeba6..19f7b3e5c9 100644 --- a/go/libraries/doltcore/sqle/procedures_table.go +++ b/go/libraries/doltcore/sqle/procedures_table.go @@ -101,9 +101,10 @@ func DoltProceduresGetOrCreateTable(ctx *sql.Context, db Database) (*WritableDol return tbl.(*WritableDoltTable), nil } -// TODO: godocs -// TODO: what's the best way to test this? backwards compat test suite? +// migrateDoltProceduresSchema migrates the dolt_procedures system table from a previous schema version to the current +// schema version by adding any columns that do not exist. func migrateDoltProceduresSchema(ctx *sql.Context, db Database, oldTable *WritableDoltTable) (newTable *WritableDoltTable, rerr error) { + // TODO: what's the best way to test this? backwards compat test suite? // Copy all the old data iter, err := SqlTableToRowIter(ctx, oldTable.DoltTable, nil) if err != nil { From 6f03849f56880edbd6863c70710bfdc8077ad554 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Wed, 9 Aug 2023 12:30:05 -0700 Subject: [PATCH 17/28] Testing that we can round trip a dolt dump with ANSI_QUOTES mode used --- integration-tests/bats/dump.bats | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/integration-tests/bats/dump.bats b/integration-tests/bats/dump.bats index 850a3d8c69..2fe7fe8872 100644 --- a/integration-tests/bats/dump.bats +++ b/integration-tests/bats/dump.bats @@ -882,12 +882,22 @@ SQL [ -f doltdump.sql ] cp doltdump.sql ~/doltdump.sql - # TODO: Would be good to make sure we can roundtrip it, but we won't be able - # to without SQL_MODE set to ANSI_QUOTES - # TODO: Need to clean up setting SQL_MODE in dump file; - # How does MySQL do this? If we could just normalize the query - # or fragment to non-ANSI_QUOTES, then everything would be so - # much easier! + mkdir roundtrip + cd roundtrip + dolt init + + dolt sql < ../doltdump.sql + [ $status -eq 0 ] + + run dolt sql -q "USE dolt_repo_$$; SHOW TABLES;" + [ $status -eq 0 ] + [[ $output =~ "table1" ]] || false + [[ $output =~ "view1" ]] || false + + run dolt sql -r csv -q "USE dolt_repo_$$; CALL procedure1;" + [ $status -eq 0 ] + [[ $output =~ "pk,col1" ]] || false + [[ $output =~ "2,1" ]] || false } @test "dump: round trip dolt dump with all data types" { From 8b52c90c1ff5b0cc1449cde80d0988e4df97a635 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Wed, 9 Aug 2023 14:23:48 -0700 Subject: [PATCH 18/28] Added expected sql_mode column --- .../mysql-client-tests/node/workbenchTests/views.js | 1 + 1 file changed, 1 insertion(+) diff --git a/integration-tests/mysql-client-tests/node/workbenchTests/views.js b/integration-tests/mysql-client-tests/node/workbenchTests/views.js index 984db581b8..49370d4cdb 100644 --- a/integration-tests/mysql-client-tests/node/workbenchTests/views.js +++ b/integration-tests/mysql-client-tests/node/workbenchTests/views.js @@ -34,6 +34,7 @@ export const viewsTests = [ name: "myview", fragment: "CREATE VIEW `myview` AS SELECT * FROM test", extra: { CreatedAt: 0 }, + sql_mode: 'STRICT_TRANS_TABLES,NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY', }, ], }, From 030b7db81943a465ae1cea2ab572bbca044d55de Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Wed, 9 Aug 2023 15:21:12 -0700 Subject: [PATCH 19/28] Fixing up tests from dolt_schemas schema change --- .../mysql-client-tests/node/workbenchTests/diffs.js | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/integration-tests/mysql-client-tests/node/workbenchTests/diffs.js b/integration-tests/mysql-client-tests/node/workbenchTests/diffs.js index 840f9bcb0a..087855c59d 100644 --- a/integration-tests/mysql-client-tests/node/workbenchTests/diffs.js +++ b/integration-tests/mysql-client-tests/node/workbenchTests/diffs.js @@ -74,13 +74,13 @@ export const diffTests = [ rows_added: 1, rows_deleted: 0, rows_modified: 0, - cells_added: 4, + cells_added: 5, cells_deleted: 0, cells_modified: 0, old_row_count: 0, new_row_count: 1, old_cell_count: 0, - new_cell_count: 4, + new_cell_count: 5, }, { table_name: "test", @@ -197,12 +197,14 @@ export const diffTests = [ to_name: "myview", to_fragment: "CREATE VIEW `myview` AS SELECT * FROM test", to_extra: { CreatedAt: 0 }, + to_sql_mode: 'STRICT_TRANS_TABLES,NO_ENGINE_SUBSTITUTION,ONLY_FULL_GROUP_BY', to_commit: "WORKING", to_commit_date: "2023-03-09T07:56:29.035Z", from_type: null, from_name: null, from_fragment: null, from_extra: null, + from_sql_mode: null, from_commit: "HEAD", from_commit_date: "2023-03-09T07:56:28.841Z", diff_type: "added", @@ -234,6 +236,7 @@ export const diffTests = [ " `name` varchar(64) COLLATE utf8mb4_0900_ai_ci NOT NULL,\n" + " `fragment` longtext,\n" + " `extra` json,\n" + + " `sql_mode` varchar(256) COLLATE utf8mb4_0900_ai_ci,\n" + " PRIMARY KEY (`type`,`name`)\n" + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin;", }, @@ -283,6 +286,7 @@ export const diffTests = [ " `name` varchar(64) COLLATE utf8mb4_0900_ai_ci NOT NULL,\n" + " `fragment` longtext,\n" + " `extra` json,\n" + + " `sql_mode` varchar(256) COLLATE utf8mb4_0900_ai_ci,\n" + " PRIMARY KEY (`type`,`name`)\n" + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin;", }, @@ -365,13 +369,13 @@ export const diffTests = [ rows_added: 1, rows_deleted: 0, rows_modified: 0, - cells_added: 4, + cells_added: 5, cells_deleted: 0, cells_modified: 0, old_row_count: 0, new_row_count: 1, old_cell_count: 0, - new_cell_count: 4, + new_cell_count: 5, }, { table_name: "test", @@ -447,6 +451,7 @@ export const diffTests = [ " `name` varchar(64) COLLATE utf8mb4_0900_ai_ci NOT NULL,\n" + " `fragment` longtext,\n" + " `extra` json,\n" + + " `sql_mode` varchar(256) COLLATE utf8mb4_0900_ai_ci,\n" + " PRIMARY KEY (`type`,`name`)\n" + ") ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_0900_bin;", }, From 5f5cb5bda023f08792d77f98d83011538e20dd2d Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Wed, 9 Aug 2023 16:10:01 -0700 Subject: [PATCH 20/28] Adding a test for the dolt_procedures table schema migration code path --- .../doltcore/sqle/procedures_table.go | 1 - .../doltcore/sqle/procedures_table_test.go | 94 +++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 go/libraries/doltcore/sqle/procedures_table_test.go diff --git a/go/libraries/doltcore/sqle/procedures_table.go b/go/libraries/doltcore/sqle/procedures_table.go index 19f7b3e5c9..22064eb55e 100644 --- a/go/libraries/doltcore/sqle/procedures_table.go +++ b/go/libraries/doltcore/sqle/procedures_table.go @@ -104,7 +104,6 @@ func DoltProceduresGetOrCreateTable(ctx *sql.Context, db Database) (*WritableDol // migrateDoltProceduresSchema migrates the dolt_procedures system table from a previous schema version to the current // schema version by adding any columns that do not exist. func migrateDoltProceduresSchema(ctx *sql.Context, db Database, oldTable *WritableDoltTable) (newTable *WritableDoltTable, rerr error) { - // TODO: what's the best way to test this? backwards compat test suite? // Copy all the old data iter, err := SqlTableToRowIter(ctx, oldTable.DoltTable, nil) if err != nil { diff --git a/go/libraries/doltcore/sqle/procedures_table_test.go b/go/libraries/doltcore/sqle/procedures_table_test.go new file mode 100644 index 0000000000..4149d2de1d --- /dev/null +++ b/go/libraries/doltcore/sqle/procedures_table_test.go @@ -0,0 +1,94 @@ +// Copyright 2023 Dolthub, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package sqle + +import ( + "context" + "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" + "github.com/dolthub/dolt/go/libraries/doltcore/dtestutils" + "github.com/dolthub/dolt/go/libraries/doltcore/table/editor" + "github.com/dolthub/go-mysql-server/sql" + gmstypes "github.com/dolthub/go-mysql-server/sql/types" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "io" + "testing" + "time" +) + +// Tests the codepath for migrating the dolt_procedures system table from an older schema +// to the latest schema +func TestProceduresMigration(t *testing.T) { + dEnv := dtestutils.CreateTestEnv() + tmpDir, err := dEnv.TempTableFilesDir() + require.NoError(t, err) + opts := editor.Options{Deaf: dEnv.DbEaFactory(), Tempdir: tmpDir} + db, err := NewDatabase(context.Background(), "dolt", dEnv.DbData(), opts) + require.NoError(t, err) + + _, ctx, err := NewTestEngine(dEnv, context.Background(), db) + require.NoError(t, err) + + // Create the dolt_procedures table with its original schema + err = db.createSqlTable(ctx, doltdb.ProceduresTableName, sql.NewPrimaryKeySchema(sql.Schema{ + {Name: doltdb.ProceduresTableNameCol, Type: gmstypes.Text, Source: doltdb.ProceduresTableName, PrimaryKey: true}, + {Name: doltdb.ProceduresTableCreateStmtCol, Type: gmstypes.Text, Source: doltdb.ProceduresTableName, PrimaryKey: false}, + {Name: doltdb.ProceduresTableCreatedAtCol, Type: gmstypes.Timestamp, Source: doltdb.ProceduresTableName, PrimaryKey: false}, + {Name: doltdb.ProceduresTableModifiedAtCol, Type: gmstypes.Timestamp, Source: doltdb.ProceduresTableName, PrimaryKey: false}, + }), sql.Collation_Default) + require.NoError(t, err) + + sqlTbl, found, err := db.GetTableInsensitive(ctx, doltdb.ProceduresTableName) + require.NoError(t, err) + require.True(t, found) + + // Insert some test data for procedures + inserter := sqlTbl.(*WritableDoltTable).Inserter(ctx) + timestamp := time.Now().UTC() + require.NoError(t, inserter.Insert(ctx, sql.Row{"proc1", "create procedure proc1() SELECT 42 as pk from dual;", timestamp, timestamp})) + require.NoError(t, inserter.Insert(ctx, sql.Row{"proc2", "create procedure proc2() SELECT 'HELLO' as greeting from dual;", timestamp, timestamp})) + require.NoError(t, inserter.Close(ctx)) + + // Call the logic to migrate it to the latest schema + tbl, err := DoltProceduresGetOrCreateTable(ctx, db) + require.NoError(t, err) + + // Assert that the data was migrated correctly + rows := readAllRows(ctx, t, tbl) + expectedRows := []sql.Row{ + {"proc1", "create procedure proc1() SELECT 42 as pk from dual;", timestamp, timestamp, nil}, + {"proc2", "create procedure proc2() SELECT 'HELLO' as greeting from dual;", timestamp, timestamp, nil}, + } + assert.Equal(t, expectedRows, rows) +} + +func readAllRows(ctx *sql.Context, t *testing.T, tbl *WritableDoltTable) []sql.Row { + iter, err := SqlTableToRowIter(ctx, tbl.DoltTable, nil) + require.NoError(t, err) + + var rows []sql.Row + for { + row, err := iter.Next(ctx) + if err == io.EOF { + break + } + + require.NoError(t, err) + rows = append(rows, row) + } + require.NoError(t, iter.Close(ctx)) + + return rows +} From b1e754b2670026feca9337785e8c578c7dc7562a Mon Sep 17 00:00:00 2001 From: fulghum Date: Wed, 9 Aug 2023 23:27:16 +0000 Subject: [PATCH 21/28] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/go.sum | 2 -- .../doltcore/sqle/procedures_table_test.go | 14 ++++++++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/go/go.sum b/go/go.sum index 86f20cd952..b6f5193c40 100644 --- a/go/go.sum +++ b/go/go.sum @@ -180,8 +180,6 @@ github.com/dolthub/fslock v0.0.3 h1:iLMpUIvJKMKm92+N1fmHVdxJP5NdyDK5bK7z7Ba2s2U= github.com/dolthub/fslock v0.0.3/go.mod h1:QWql+P17oAAMLnL4HGB5tiovtDuAjdDTPbuqx7bYfa0= github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e h1:kPsT4a47cw1+y/N5SSCkma7FhAPw7KeGmD6c9PBZW9Y= github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e/go.mod h1:KPUcpx070QOfJK1gNe0zx4pA5sicIK1GMikIGLKC168= -github.com/dolthub/go-mysql-server v0.16.1-0.20230804191758-46bed6efacdc h1:XAfRHQoOoF2hYEgG0ADYJgjf7tgcUlGIhM7FpY9a1sI= -github.com/dolthub/go-mysql-server v0.16.1-0.20230804191758-46bed6efacdc/go.mod h1:nY1J1sV2kuGJbAZ6bcZARw4dF8TD3KpEfYVVs/HK/JY= github.com/dolthub/go-mysql-server v0.16.1-0.20230807211358-5b6031f113ba h1:ImvGNMKqAA+KPH5rXTR1xmOVQYv9XC7p6AjX8RhkWMU= github.com/dolthub/go-mysql-server v0.16.1-0.20230807211358-5b6031f113ba/go.mod h1:nY1J1sV2kuGJbAZ6bcZARw4dF8TD3KpEfYVVs/HK/JY= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488 h1:0HHu0GWJH0N6a6keStrHhUAK5/o9LVfkh44pvsV4514= diff --git a/go/libraries/doltcore/sqle/procedures_table_test.go b/go/libraries/doltcore/sqle/procedures_table_test.go index 4149d2de1d..554bb7cc85 100644 --- a/go/libraries/doltcore/sqle/procedures_table_test.go +++ b/go/libraries/doltcore/sqle/procedures_table_test.go @@ -16,16 +16,18 @@ package sqle import ( "context" - "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" - "github.com/dolthub/dolt/go/libraries/doltcore/dtestutils" - "github.com/dolthub/dolt/go/libraries/doltcore/table/editor" + "io" + "testing" + "time" + "github.com/dolthub/go-mysql-server/sql" gmstypes "github.com/dolthub/go-mysql-server/sql/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "io" - "testing" - "time" + + "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" + "github.com/dolthub/dolt/go/libraries/doltcore/dtestutils" + "github.com/dolthub/dolt/go/libraries/doltcore/table/editor" ) // Tests the codepath for migrating the dolt_procedures system table from an older schema From bc5e7ec936662b8c2c2eedb4cadc91eaf5e269ad Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Wed, 9 Aug 2023 17:52:14 -0700 Subject: [PATCH 22/28] Truncating test time value to deal with test failures on Ubuntu where time seems truncated to milliseconds --- go/libraries/doltcore/sqle/procedures_table_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/libraries/doltcore/sqle/procedures_table_test.go b/go/libraries/doltcore/sqle/procedures_table_test.go index 4149d2de1d..8a347ac2f1 100644 --- a/go/libraries/doltcore/sqle/procedures_table_test.go +++ b/go/libraries/doltcore/sqle/procedures_table_test.go @@ -56,7 +56,7 @@ func TestProceduresMigration(t *testing.T) { // Insert some test data for procedures inserter := sqlTbl.(*WritableDoltTable).Inserter(ctx) - timestamp := time.Now().UTC() + timestamp := time.Now().Truncate(time.Minute).UTC() require.NoError(t, inserter.Insert(ctx, sql.Row{"proc1", "create procedure proc1() SELECT 42 as pk from dual;", timestamp, timestamp})) require.NoError(t, inserter.Insert(ctx, sql.Row{"proc2", "create procedure proc2() SELECT 'HELLO' as greeting from dual;", timestamp, timestamp})) require.NoError(t, inserter.Close(ctx)) From 5580f3d85197f5dd5fb980f5b2250ef303d2bccd Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 15 Aug 2023 14:53:52 -0700 Subject: [PATCH 23/28] Bumping DoltFeatureVersion --- go/libraries/doltcore/doltdb/root_val.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/libraries/doltcore/doltdb/root_val.go b/go/libraries/doltcore/doltdb/root_val.go index dea4a4f286..e61eb15e26 100644 --- a/go/libraries/doltcore/doltdb/root_val.go +++ b/go/libraries/doltcore/doltdb/root_val.go @@ -50,7 +50,7 @@ type FeatureVersion int64 // DoltFeatureVersion is described in feature_version.md. // only variable for testing. -var DoltFeatureVersion FeatureVersion = 3 // last bumped when storing creation time for triggers +var DoltFeatureVersion FeatureVersion = 4 // last bumped when adding sql_mode column to dolt_schemas // RootValue is the value of the Database and is the committed value in every Dolt commit. type RootValue struct { From 488bd40b112b37ab3ff817d9ef198dc6967b8187 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 15 Aug 2023 15:38:39 -0700 Subject: [PATCH 24/28] PR feedback --- go/libraries/doltcore/doltdb/system_table.go | 1 - .../doltcore/sqle/procedures_table.go | 6 ++++++ go/libraries/doltcore/sqle/schema_table.go | 20 +++++++++++++++++++ 3 files changed, 26 insertions(+), 1 deletion(-) diff --git a/go/libraries/doltcore/doltdb/system_table.go b/go/libraries/doltcore/doltdb/system_table.go index 3dc35f7eb9..9b3b212f8a 100644 --- a/go/libraries/doltcore/doltdb/system_table.go +++ b/go/libraries/doltcore/doltdb/system_table.go @@ -257,7 +257,6 @@ const ( // SchemasTablesSqlModeCol is the name of the column that stores the SQL_MODE string used when this fragment // was originally defined. Mode settings, such as ANSI_QUOTES, are needed to correctly parse the fragment. SchemasTablesSqlModeCol = "sql_mode" - SchemasTablesIndexName = "fragment_name" ) const ( diff --git a/go/libraries/doltcore/sqle/procedures_table.go b/go/libraries/doltcore/sqle/procedures_table.go index 22064eb55e..c1b398fa27 100644 --- a/go/libraries/doltcore/sqle/procedures_table.go +++ b/go/libraries/doltcore/sqle/procedures_table.go @@ -270,6 +270,12 @@ func DoltProceduresGetAll(ctx *sql.Context, db Database, procedureName string) ( } if s, ok := sqlRow[4].(string); ok { d.SqlMode = s + } else { + defaultSqlMode, err := loadDefaultSqlMode() + if err != nil { + return nil, err + } + d.SqlMode = defaultSqlMode } details = append(details, d) } diff --git a/go/libraries/doltcore/sqle/schema_table.go b/go/libraries/doltcore/sqle/schema_table.go index 1b1ad15713..0531568840 100644 --- a/go/libraries/doltcore/sqle/schema_table.go +++ b/go/libraries/doltcore/sqle/schema_table.go @@ -279,6 +279,12 @@ func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType if s, ok := sqlRow[sqlModeIdx].(string); ok { sqlModeString = s } + } else { + defaultSqlMode, err := loadDefaultSqlMode() + if err != nil { + return nil, err + } + sqlModeString = defaultSqlMode } // For older tables, use 1 as the trigger creation time @@ -306,6 +312,20 @@ func getSchemaFragmentsOfType(ctx *sql.Context, tbl *WritableDoltTable, fragType return frags, nil } +// loadDefaultSqlMode loads the default value for the @@SQL_MODE system variable and returns it, along +// with any unexpected errors encountered while reading the default value. +func loadDefaultSqlMode() (string, error) { + global, _, ok := sql.SystemVariables.GetGlobal("SQL_MODE") + if !ok { + return "", fmt.Errorf("unable to load default @@SQL_MODE") + } + s, ok := global.Default.(string) + if !ok { + return "", fmt.Errorf("unexpected type for @@SQL_MODE default value: %T", global.Default) + } + return s, nil +} + func getCreatedTime(ctx *sql.Context, extraCol gmstypes.JSONValue) (int64, error) { doc, err := extraCol.Unmarshall(ctx) if err != nil { From c4f73a03b4d48c743c51583b989d45782313305c Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 15 Aug 2023 16:30:46 -0700 Subject: [PATCH 25/28] Bumping to latest dev builds of go-mysql-server from fulghum/ansi_quotes branch --- go/go.mod | 4 ++-- go/go.sum | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/go/go.mod b/go/go.mod index 5262c12a61..4d35661b07 100644 --- a/go/go.mod +++ b/go/go.mod @@ -15,7 +15,7 @@ require ( github.com/dolthub/fslock v0.0.3 github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488 github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81 - github.com/dolthub/vitess v0.0.0-20230803210631-6a8ca1536779 + github.com/dolthub/vitess v0.0.0-20230815183531-5f606d83f408 github.com/dustin/go-humanize v1.0.0 github.com/fatih/color v1.13.0 github.com/flynn-archive/go-shlex v0.0.0-20150515145356-3f9db97f8568 @@ -59,7 +59,7 @@ require ( github.com/cespare/xxhash v1.1.0 github.com/creasty/defaults v1.6.0 github.com/dolthub/flatbuffers/v23 v23.3.3-dh.2 - github.com/dolthub/go-mysql-server v0.16.1-0.20230807211358-5b6031f113ba + github.com/dolthub/go-mysql-server v0.16.1-0.20230815232241-704a2c0709d3 github.com/dolthub/swiss v0.1.0 github.com/goccy/go-json v0.10.2 github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510 diff --git a/go/go.sum b/go/go.sum index b6f5193c40..b5ca41e5b5 100644 --- a/go/go.sum +++ b/go/go.sum @@ -182,6 +182,8 @@ github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e h1:kPsT4a47cw github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e/go.mod h1:KPUcpx070QOfJK1gNe0zx4pA5sicIK1GMikIGLKC168= github.com/dolthub/go-mysql-server v0.16.1-0.20230807211358-5b6031f113ba h1:ImvGNMKqAA+KPH5rXTR1xmOVQYv9XC7p6AjX8RhkWMU= github.com/dolthub/go-mysql-server v0.16.1-0.20230807211358-5b6031f113ba/go.mod h1:nY1J1sV2kuGJbAZ6bcZARw4dF8TD3KpEfYVVs/HK/JY= +github.com/dolthub/go-mysql-server v0.16.1-0.20230815232241-704a2c0709d3 h1:2puC5sgRHTdWapYMn4WrSH06/qq8Qfw1wW84eWHvC1Q= +github.com/dolthub/go-mysql-server v0.16.1-0.20230815232241-704a2c0709d3/go.mod h1:wK9co8FTJEstDv35ITVgCjYVqqBoN5MlDyVMsNCoWaw= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488 h1:0HHu0GWJH0N6a6keStrHhUAK5/o9LVfkh44pvsV4514= github.com/dolthub/ishell v0.0.0-20221214210346-d7db0b066488/go.mod h1:ehexgi1mPxRTk0Mok/pADALuHbvATulTh6gzr7NzZto= github.com/dolthub/jsonpath v0.0.2-0.20230525180605-8dc13778fd72 h1:NfWmngMi1CYUWU4Ix8wM+USEhjc+mhPlT9JUR/anvbQ= @@ -194,6 +196,8 @@ github.com/dolthub/swiss v0.1.0 h1:EaGQct3AqeP/MjASHLiH6i4TAmgbG/c4rA6a1bzCOPc= github.com/dolthub/swiss v0.1.0/go.mod h1:BeucyB08Vb1G9tumVN3Vp/pyY4AMUnr9p7Rz7wJ7kAQ= github.com/dolthub/vitess v0.0.0-20230803210631-6a8ca1536779 h1:/ru4ji1X6Fj350rnYhKVZE1qClbKNS6hqYQNgiNanQk= github.com/dolthub/vitess v0.0.0-20230803210631-6a8ca1536779/go.mod h1:IyoysiiOzrIs7QsEHC+yVF0yRQ6W70GXyCXqtI2vVTs= +github.com/dolthub/vitess v0.0.0-20230815183531-5f606d83f408 h1:0MhiNzFzHybo3FR3kJBGadx+Z9u30R30YfUOKQ3fyJQ= +github.com/dolthub/vitess v0.0.0-20230815183531-5f606d83f408/go.mod h1:IyoysiiOzrIs7QsEHC+yVF0yRQ6W70GXyCXqtI2vVTs= github.com/dustin/go-humanize v0.0.0-20171111073723-bb3d318650d4/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= github.com/dustin/go-humanize v1.0.0 h1:VSnTsYCnlFHaM2/igO1h6X3HA71jcobQuxemgkq4zYo= github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk= From daa149891cabf8ac136b1b08305a7b175963a5d0 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Tue, 15 Aug 2023 17:09:42 -0700 Subject: [PATCH 26/28] Updating head commit value for tests, from changing FeatureVersion from 3 to 4. --- go/libraries/doltcore/sqle/sqlselect_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/libraries/doltcore/sqle/sqlselect_test.go b/go/libraries/doltcore/sqle/sqlselect_test.go index 5df18881a4..3c000c984b 100644 --- a/go/libraries/doltcore/sqle/sqlselect_test.go +++ b/go/libraries/doltcore/sqle/sqlselect_test.go @@ -91,7 +91,7 @@ func BasicSelectTests() []SelectTest { var headCommitHash string switch types.Format_Default { case types.Format_DOLT: - headCommitHash = "a0gt4vif0b0bf19g89k87gs55qqlqpod" + headCommitHash = "m1gkfp9ii4hiqhpmgcfet5sojvopo4da" case types.Format_LD_1: headCommitHash = "73hc2robs4v0kt9taoe3m5hd49dmrgun" } From 3ee51bb0eb9a2a61d1dff9589662bca42269bd9c Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Wed, 16 Aug 2023 09:00:40 -0700 Subject: [PATCH 27/28] Updating BATS for feature version bump --- integration-tests/bats/helper/common.bash | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/bats/helper/common.bash b/integration-tests/bats/helper/common.bash index c9036d3ebf..94dada4cb6 100644 --- a/integration-tests/bats/helper/common.bash +++ b/integration-tests/bats/helper/common.bash @@ -77,7 +77,7 @@ assert_feature_version() { # Tests that don't end in a valid dolt dir will fail the above # command, don't check its output in that case if [ "$status" -eq 0 ]; then - [[ "$output" =~ "feature version: 3" ]] || exit 1 + [[ "$output" =~ "feature version: 4" ]] || exit 1 else # Clear status to avoid BATS failing if this is the last run command status=0 From 09f1c00c4d44113cf82d46e277fca1557bfba691 Mon Sep 17 00:00:00 2001 From: Jason Fulghum Date: Wed, 16 Aug 2023 09:57:36 -0700 Subject: [PATCH 28/28] Updating tests from bumping feature version --- integration-tests/bats/status.bats | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/integration-tests/bats/status.bats b/integration-tests/bats/status.bats index 687a144e94..9327e699a1 100644 --- a/integration-tests/bats/status.bats +++ b/integration-tests/bats/status.bats @@ -19,7 +19,7 @@ get_head_commit() { run dolt version --feature [ "$status" -eq 0 ] [[ "$output" =~ "dolt version" ]] || false - [[ "$output" =~ "feature version: 3" ]] || false + [[ "$output" =~ "feature version: 4" ]] || false } @test "status: no changes" {