From 94541c890e6865d3fd0094e30777c282df0182c9 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Thu, 26 Oct 2023 10:19:04 -0700 Subject: [PATCH 01/26] Added new fields to schema.Column. Killed all un-named column struct constructions --- go/libraries/doltcore/schema/col_coll_test.go | 30 +++++++-------- go/libraries/doltcore/schema/column.go | 37 ++++++++++--------- go/libraries/doltcore/schema/schema_test.go | 26 +++++-------- go/serial/schema.fbs | 2 +- 4 files changed, 45 insertions(+), 50 deletions(-) diff --git a/go/libraries/doltcore/schema/col_coll_test.go b/go/libraries/doltcore/schema/col_coll_test.go index 597a9f45d1..d39e21c671 100644 --- a/go/libraries/doltcore/schema/col_coll_test.go +++ b/go/libraries/doltcore/schema/col_coll_test.go @@ -21,14 +21,14 @@ import ( "github.com/stretchr/testify/assert" - "github.com/dolthub/dolt/go/libraries/doltcore/schema/typeinfo" + typeinfo "github.com/dolthub/dolt/go/libraries/doltcore/schema/typeinfo" "github.com/dolthub/dolt/go/store/types" ) -var firstNameCol = Column{"first", 0, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil} -var lastNameCol = Column{"last", 1, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil} -var firstNameCapsCol = Column{"FiRsT", 2, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil} -var lastNameCapsCol = Column{"LAST", 3, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil} +var firstNameCol = Column{Name: "first", Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType} +var lastNameCol = Column{Name: "last", Tag: 1, Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType} +var firstNameCapsCol = Column{Name: "FiRsT", Tag: 2, Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType} +var lastNameCapsCol = Column{Name: "LAST", Tag: 3, Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType} func TestGetByNameAndTag(t *testing.T) { cols := []Column{firstNameCol, lastNameCol, firstNameCapsCol, lastNameCapsCol} @@ -102,18 +102,18 @@ func TestGetByNameCaseInsensitive(t *testing.T) { func TestAppendAndItrInSortOrder(t *testing.T) { cols := []Column{ - {"0", 0, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil}, - {"2", 2, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil}, - {"4", 4, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil}, - {"3", 3, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil}, - {"1", 1, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil}, + {Name: "0", Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType}, + {Name: "2", Tag: 2, Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType}, + {Name: "4", Tag: 4, Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType}, + {Name: "3", Tag: 3, Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType}, + {Name: "1", Tag: 1, Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType}, } cols2 := []Column{ - {"7", 7, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil}, - {"9", 9, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil}, - {"5", 5, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil}, - {"8", 8, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil}, - {"6", 6, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil}, + {Name: "7", Tag: 7, Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType}, + {Name: "9", Tag: 9, Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType}, + {Name: "5", Tag: 5, Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType}, + {Name: "8", Tag: 8, Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType}, + {Name: "6", Tag: 6, Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType}, } colColl := NewColCollection(cols...) diff --git a/go/libraries/doltcore/schema/column.go b/go/libraries/doltcore/schema/column.go index 3b94faad95..9fa6eb8496 100644 --- a/go/libraries/doltcore/schema/column.go +++ b/go/libraries/doltcore/schema/column.go @@ -37,15 +37,10 @@ var ( var ( // InvalidCol is a Column instance that is returned when there is nothing to return and can be tested against. InvalidCol = Column{ - "invalid", - InvalidTag, - types.NullKind, - false, - typeinfo.UnknownType, - "", - false, - "", - nil, + Name: "invalid", + Tag: InvalidTag, + Kind: types.NullKind, + TypeInfo: typeinfo.UnknownType, } ) @@ -75,6 +70,12 @@ type Column struct { // Default is the default value of this column. This is the string representation of a sql.Expression. Default string + + // Generated is the generated value of this column. This is the string representation of a sql.Expression. + Generated string + + // Virtual is true if this is a virtual column. + Virtual bool // AutoIncrement says whether this column auto increments. AutoIncrement bool @@ -109,15 +110,15 @@ func NewColumnWithTypeInfo(name string, tag uint64, typeInfo typeinfo.TypeInfo, } return Column{ - name, - tag, - typeInfo.NomsKind(), - partOfPK, - typeInfo, - defaultVal, - autoIncrement, - comment, - constraints, + Name: name, + Tag: tag, + Kind: typeInfo.NomsKind(), + IsPartOfPK: partOfPK, + TypeInfo: typeInfo, + Default: defaultVal, + AutoIncrement: autoIncrement, + Comment: comment, + Constraints: constraints, }, nil } diff --git a/go/libraries/doltcore/schema/schema_test.go b/go/libraries/doltcore/schema/schema_test.go index e457b8a00b..2182fd8efa 100644 --- a/go/libraries/doltcore/schema/schema_test.go +++ b/go/libraries/doltcore/schema/schema_test.go @@ -17,7 +17,7 @@ package schema import ( "fmt" "reflect" - "strings" + strings "strings" "testing" "github.com/stretchr/testify/assert" @@ -42,21 +42,15 @@ const ( reservedColTag = 50 ) -var lnVal = types.String("astley") -var fnVal = types.String("rick") -var addrVal = types.String("123 Fake St") -var ageVal = types.Uint(53) -var titleVal = types.NullValue - var pkCols = []Column{ - {lnColName, lnColTag, types.StringKind, true, typeinfo.StringDefaultType, "", false, "", nil}, - {fnColName, fnColTag, types.StringKind, true, typeinfo.StringDefaultType, "", false, "", nil}, + {Name: lnColName, Tag: lnColTag, Kind: types.StringKind, IsPartOfPK: true, TypeInfo: typeinfo.StringDefaultType}, + {Name: fnColName, Tag: fnColTag, Kind: types.StringKind, IsPartOfPK: true, TypeInfo: typeinfo.StringDefaultType}, } var nonPkCols = []Column{ - {addrColName, addrColTag, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil}, - {ageColName, ageColTag, types.UintKind, false, typeinfo.FromKind(types.UintKind), "", false, "", nil}, - {titleColName, titleColTag, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil}, - {reservedColName, reservedColTag, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil}, + {Name: addrColName, Tag: addrColTag, Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType}, + {Name: ageColName, Tag: ageColTag, Kind: types.UintKind, TypeInfo: typeinfo.FromKind(types.UintKind)}, + {Name: titleColName, Tag: titleColTag, Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType}, + {Name: reservedColName, Tag: reservedColTag, Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType}, } var allCols = append(append([]Column(nil), pkCols...), nonPkCols...) @@ -204,7 +198,7 @@ func TestValidateForInsert(t *testing.T) { }) t.Run("Name collision", func(t *testing.T) { - cols := append(allCols, Column{titleColName, 100, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil}) + cols := append(allCols, Column{Name: titleColName, Tag: 100, Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType}) colColl := NewColCollection(cols...) err := ValidateForInsert(colColl) @@ -213,7 +207,7 @@ func TestValidateForInsert(t *testing.T) { }) t.Run("Case insensitive collision", func(t *testing.T) { - cols := append(allCols, Column{strings.ToUpper(titleColName), 100, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil}) + cols := append(allCols, Column{Name: strings.ToUpper(titleColName), Tag: 100, Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType}) colColl := NewColCollection(cols...) err := ValidateForInsert(colColl) @@ -222,7 +216,7 @@ func TestValidateForInsert(t *testing.T) { }) t.Run("Tag collision", func(t *testing.T) { - cols := append(allCols, Column{"newCol", lnColTag, types.StringKind, false, typeinfo.StringDefaultType, "", false, "", nil}) + cols := append(allCols, Column{Name: "newCol", Tag: lnColTag, Kind: types.StringKind, TypeInfo: typeinfo.StringDefaultType}) colColl := NewColCollection(cols...) err := ValidateForInsert(colColl) diff --git a/go/serial/schema.fbs b/go/serial/schema.fbs index 659a2c24cb..d51be4e0de 100644 --- a/go/serial/schema.fbs +++ b/go/serial/schema.fbs @@ -33,7 +33,7 @@ table Column { // sql column type sql_type:string; - // sql default value + // sql default value. For generated columns, this is the generated expression rather than the default. default_value:string; // sql comment From 0e0a5ee13730ea9d92d1691e1c79a00b5daab9f6 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Thu, 26 Oct 2023 12:50:41 -0700 Subject: [PATCH 02/26] Added generated and virtual markers to schema --- go/cmd/dolt/commands/diff.go | 10 +--------- go/libraries/doltcore/doltdb/system_table.go | 2 +- go/libraries/doltcore/doltdb/table.go | 4 ++-- .../doltcore/merge/schema_integration_test.go | 2 +- go/libraries/doltcore/migrate/integration_test.go | 6 +++--- go/libraries/doltcore/migrate/transform.go | 12 +++--------- go/libraries/doltcore/rowconv/joiner.go | 2 +- go/libraries/doltcore/schema/column.go | 15 +++++++++++++-- .../doltcore/schema/encoding/schema_marshaling.go | 2 +- .../schema/encoding/schema_marshaling_test.go | 7 +++---- .../doltcore/schema/encoding/serialization.go | 14 +++----------- go/libraries/doltcore/sqle/alterschema.go | 4 ++-- go/libraries/doltcore/sqle/alterschema_test.go | 2 +- go/libraries/doltcore/sqle/common_test.go | 2 +- .../sqle/dtables/conflicts_tables_prolly.go | 4 ++-- go/libraries/doltcore/sqle/dtables/diff_table.go | 4 ++-- .../doltcore/sqle/index/noms_index_iter.go | 2 +- go/libraries/doltcore/sqle/schema_table.go | 2 +- go/libraries/doltcore/sqle/sqlutil/convert.go | 2 +- go/libraries/doltcore/sqle/testdata.go | 2 +- 20 files changed, 44 insertions(+), 56 deletions(-) diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index fa4335f329..41100e2b9a 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -900,15 +900,7 @@ func schemaFromCreateTableStmt(createTableStmt string) (schema.Schema, error) { if col.Type.Comment != nil { comment = col.Type.Comment.String() } - sCol, err := schema.NewColumnWithTypeInfo( - col.Name.String(), - 0, - typeInfo, - primaryCols[col.Name.Lowered()], - defBuf.String(), - col.Type.Autoincrement == true, - comment, - ) + sCol, err := schema.NewColumnWithTypeInfo(col.Name.String(), 0, typeInfo, primaryCols[col.Name.Lowered()], defBuf.String(), "", false, col.Type.Autoincrement == true, comment) cols = append(cols, sCol) } diff --git a/go/libraries/doltcore/doltdb/system_table.go b/go/libraries/doltcore/doltdb/system_table.go index 69ec018206..2122e7d393 100644 --- a/go/libraries/doltcore/doltdb/system_table.go +++ b/go/libraries/doltcore/doltdb/system_table.go @@ -43,7 +43,7 @@ var OldDocsSchema = schema.MustSchemaFromCols(schema.NewColCollection( var DocsSchema schema.Schema func init() { - docTextCol, err := schema.NewColumnWithTypeInfo(DocTextColumnName, schema.DocTextTag, typeinfo.LongTextType, false, "", false, "") + docTextCol, err := schema.NewColumnWithTypeInfo(DocTextColumnName, schema.DocTextTag, typeinfo.LongTextType, false, "", "", false, false, "") if err != nil { panic(err) } diff --git a/go/libraries/doltcore/doltdb/table.go b/go/libraries/doltcore/doltdb/table.go index d94229ea1d..2b5d9c10e5 100644 --- a/go/libraries/doltcore/doltdb/table.go +++ b/go/libraries/doltcore/doltdb/table.go @@ -343,11 +343,11 @@ func (t *Table) GetConstraintViolationsSchema(ctx context.Context) (schema.Schem if err != nil { return nil, err } - typeCol, err := schema.NewColumnWithTypeInfo("violation_type", schema.DoltConstraintViolationsTypeTag, typeType, true, "", false, "") + typeCol, err := schema.NewColumnWithTypeInfo("violation_type", schema.DoltConstraintViolationsTypeTag, typeType, true, "", "", false, false, "") if err != nil { return nil, err } - infoCol, err := schema.NewColumnWithTypeInfo("violation_info", schema.DoltConstraintViolationsInfoTag, typeinfo.JSONType, false, "", false, "") + infoCol, err := schema.NewColumnWithTypeInfo("violation_info", schema.DoltConstraintViolationsInfoTag, typeinfo.JSONType, false, "", "", false, false, "") if err != nil { return nil, err } diff --git a/go/libraries/doltcore/merge/schema_integration_test.go b/go/libraries/doltcore/merge/schema_integration_test.go index 64cf18bc5a..a3f55bc9ff 100644 --- a/go/libraries/doltcore/merge/schema_integration_test.go +++ b/go/libraries/doltcore/merge/schema_integration_test.go @@ -538,7 +538,7 @@ func schemaFromColsAndIdxs(allCols *schema.ColCollection, indexes ...schema.Inde } func newColTypeInfo(name string, tag uint64, typeInfo typeinfo.TypeInfo, partOfPK bool, constraints ...schema.ColConstraint) schema.Column { - c, err := schema.NewColumnWithTypeInfo(name, tag, typeInfo, partOfPK, "", false, "", constraints...) + c, err := schema.NewColumnWithTypeInfo(name, tag, typeInfo, partOfPK, "", "", false, false, "", constraints...) if err != nil { panic("could not create column") } diff --git a/go/libraries/doltcore/migrate/integration_test.go b/go/libraries/doltcore/migrate/integration_test.go index 4ab378dc85..bf86a85d71 100644 --- a/go/libraries/doltcore/migrate/integration_test.go +++ b/go/libraries/doltcore/migrate/integration_test.go @@ -182,9 +182,9 @@ func setupMigrationTest(t *testing.T, ctx context.Context, test migrationTest) * } func SetupHookRefKeys(ctx context.Context, dEnv *env.DoltEnv) (*env.DoltEnv, error) { - pk, _ := schema.NewColumnWithTypeInfo("pk", 1, typeinfo.TextType, true, "", false, "", schema.NotNullConstraint{}) - c0, _ := schema.NewColumnWithTypeInfo("c0", 2, typeinfo.Int32Type, false, "", false, "") - c1, _ := schema.NewColumnWithTypeInfo("c1", 3, typeinfo.BlobType, false, "", false, "") + pk, _ := schema.NewColumnWithTypeInfo("pk", 1, typeinfo.TextType, true, "", "", false, false, "", schema.NotNullConstraint{}) + c0, _ := schema.NewColumnWithTypeInfo("c0", 2, typeinfo.Int32Type, false, "", "", false, false, "") + c1, _ := schema.NewColumnWithTypeInfo("c1", 3, typeinfo.BlobType, false, "", "", false, false, "") sch, err := schema.SchemaFromCols(schema.NewColCollection(pk, c0, c1)) if err != nil { diff --git a/go/libraries/doltcore/migrate/transform.go b/go/libraries/doltcore/migrate/transform.go index 5101475c31..9ac80681e7 100644 --- a/go/libraries/doltcore/migrate/transform.go +++ b/go/libraries/doltcore/migrate/transform.go @@ -484,15 +484,11 @@ func migrateSchema(ctx context.Context, tableName string, existing schema.Schema case query.Type_TEXT: patched = true info := typeinfo.StringDefaultType - cols[i], err = schema.NewColumnWithTypeInfo( - c.Name, c.Tag, info, c.IsPartOfPK, c.Default, - c.AutoIncrement, c.Comment, c.Constraints...) + cols[i], err = schema.NewColumnWithTypeInfo(c.Name, c.Tag, info, c.IsPartOfPK, c.Default, "", false, c.AutoIncrement, c.Comment, c.Constraints...) case query.Type_BLOB: patched = true info := typeinfo.VarbinaryDefaultType - cols[i], err = schema.NewColumnWithTypeInfo( - c.Name, c.Tag, info, c.IsPartOfPK, c.Default, - c.AutoIncrement, c.Comment, c.Constraints...) + cols[i], err = schema.NewColumnWithTypeInfo(c.Name, c.Tag, info, c.IsPartOfPK, c.Default, "", false, c.AutoIncrement, c.Comment, c.Constraints...) } if err != nil { return nil, err @@ -525,9 +521,7 @@ func migrateSchema(ctx context.Context, tableName string, existing schema.Schema return nil, err } - cols[i], err = schema.NewColumnWithTypeInfo( - c.Name, c.Tag, info, c.IsPartOfPK, c.Default, - c.AutoIncrement, c.Comment, c.Constraints...) + cols[i], err = schema.NewColumnWithTypeInfo(c.Name, c.Tag, info, c.IsPartOfPK, c.Default, "", false, c.AutoIncrement, c.Comment, c.Constraints...) if err != nil { return nil, err } diff --git a/go/libraries/doltcore/rowconv/joiner.go b/go/libraries/doltcore/rowconv/joiner.go index fea8b34a5b..77f3683297 100644 --- a/go/libraries/doltcore/rowconv/joiner.go +++ b/go/libraries/doltcore/rowconv/joiner.go @@ -73,7 +73,7 @@ func NewJoiner(namedSchemas []NamedSchema, namers map[string]ColNamingFunc) (*Jo allCols.IterInSortedOrder(func(srcTag uint64, col schema.Column) (stop bool) { newColName := namer(col.Name) var newCol schema.Column - newCol, err = schema.NewColumnWithTypeInfo(newColName, destTag, col.TypeInfo, false, col.Default, false, col.Comment) + newCol, err = schema.NewColumnWithTypeInfo(newColName, destTag, col.TypeInfo, false, col.Default, "", false, false, col.Comment) if err != nil { return true } diff --git a/go/libraries/doltcore/schema/column.go b/go/libraries/doltcore/schema/column.go index 9fa6eb8496..7ba0a48e3e 100644 --- a/go/libraries/doltcore/schema/column.go +++ b/go/libraries/doltcore/schema/column.go @@ -90,7 +90,7 @@ type Column struct { // NewColumn creates a Column instance with the default type info for the NomsKind func NewColumn(name string, tag uint64, kind types.NomsKind, partOfPK bool, constraints ...ColConstraint) Column { typeInfo := typeinfo.FromKind(kind) - col, err := NewColumnWithTypeInfo(name, tag, typeInfo, partOfPK, "", false, "", constraints...) + col, err := NewColumnWithTypeInfo(name, tag, typeInfo, partOfPK, "", "", false, false, "", constraints...) if err != nil { panic(err) } @@ -98,7 +98,16 @@ func NewColumn(name string, tag uint64, kind types.NomsKind, partOfPK bool, cons } // NewColumnWithTypeInfo creates a Column instance with the given type info. -func NewColumnWithTypeInfo(name string, tag uint64, typeInfo typeinfo.TypeInfo, partOfPK bool, defaultVal string, autoIncrement bool, comment string, constraints ...ColConstraint) (Column, error) { +func NewColumnWithTypeInfo( + name string, + tag uint64, + typeInfo typeinfo.TypeInfo, + partOfPK bool, + defaultVal, generatedVal string, + virtual, autoIncrement bool, + comment string, + constraints ...ColConstraint, +) (Column, error) { for _, c := range constraints { if c == nil { return Column{}, errors.New("nil passed as a constraint") @@ -116,6 +125,8 @@ func NewColumnWithTypeInfo(name string, tag uint64, typeInfo typeinfo.TypeInfo, IsPartOfPK: partOfPK, TypeInfo: typeInfo, Default: defaultVal, + Generated: generatedVal, + Virtual: virtual, AutoIncrement: autoIncrement, Comment: comment, Constraints: constraints, diff --git a/go/libraries/doltcore/schema/encoding/schema_marshaling.go b/go/libraries/doltcore/schema/encoding/schema_marshaling.go index e891b85ba1..e31bef7d15 100644 --- a/go/libraries/doltcore/schema/encoding/schema_marshaling.go +++ b/go/libraries/doltcore/schema/encoding/schema_marshaling.go @@ -117,7 +117,7 @@ func (nfd encodedColumn) decodeColumn() (schema.Column, error) { return schema.Column{}, errors.New("cannot decode column due to unknown schema format") } colConstraints := decodeAllColConstraint(nfd.Constraints) - return schema.NewColumnWithTypeInfo(nfd.Name, nfd.Tag, typeInfo, nfd.IsPartOfPK, nfd.Default, nfd.AutoIncrement, nfd.Comment, colConstraints...) + return schema.NewColumnWithTypeInfo(nfd.Name, nfd.Tag, typeInfo, nfd.IsPartOfPK, nfd.Default, "", false, nfd.AutoIncrement, nfd.Comment, colConstraints...) } type encodedConstraint struct { diff --git a/go/libraries/doltcore/schema/encoding/schema_marshaling_test.go b/go/libraries/doltcore/schema/encoding/schema_marshaling_test.go index 8b9250251e..3a4fc95053 100644 --- a/go/libraries/doltcore/schema/encoding/schema_marshaling_test.go +++ b/go/libraries/doltcore/schema/encoding/schema_marshaling_test.go @@ -137,7 +137,7 @@ func TestTypeInfoMarshalling(t *testing.T) { t.Run(sqlType.String(), func(t *testing.T) { ti, err := typeinfo.FromSqlType(sqlType) require.NoError(t, err) - col, err := schema.NewColumnWithTypeInfo("pk", 1, ti, true, "", false, "", schema.NotNullConstraint{}) + col, err := schema.NewColumnWithTypeInfo("pk", 1, ti, true, "", "", false, false, "", schema.NotNullConstraint{}) require.NoError(t, err) colColl := schema.NewColCollection(col) originalSch, err := schema.SchemaFromCols(colColl) @@ -232,7 +232,7 @@ func (tec testEncodedColumn) decodeColumn() (schema.Column, error) { return schema.Column{}, errors.New("cannot decode column due to unknown schema format") } colConstraints := decodeAllColConstraint(tec.Constraints) - return schema.NewColumnWithTypeInfo(tec.Name, tec.Tag, typeInfo, tec.IsPartOfPK, tec.Default, tec.AutoIncrement, tec.Comment, colConstraints...) + return schema.NewColumnWithTypeInfo(tec.Name, tec.Tag, typeInfo, tec.IsPartOfPK, tec.Default, "", false, tec.AutoIncrement, tec.Comment, colConstraints...) } func (tsd testSchemaData) decodeSchema() (schema.Schema, error) { @@ -297,8 +297,7 @@ func getColumns(t *testing.T) (cols []schema.Column) { for i := range cols { name := "col" + strconv.Itoa(i) tag := uint64(i) - cols[i], err = schema.NewColumnWithTypeInfo( - name, tag, ti[i], false, "", false, "") + cols[i], err = schema.NewColumnWithTypeInfo(name, tag, ti[i], false, "", "", false, false, "") require.NoError(t, err) } return diff --git a/go/libraries/doltcore/schema/encoding/serialization.go b/go/libraries/doltcore/schema/encoding/serialization.go index 767f9c89fc..75f8dcbfc1 100644 --- a/go/libraries/doltcore/schema/encoding/serialization.go +++ b/go/libraries/doltcore/schema/encoding/serialization.go @@ -222,8 +222,8 @@ func serializeSchemaColumns(b *fb.Builder, sch schema.Schema) fb.UOffsetT { serial.ColumnAddPrimaryKey(b, col.IsPartOfPK) serial.ColumnAddAutoIncrement(b, col.AutoIncrement) serial.ColumnAddNullable(b, col.IsNullable()) - serial.ColumnAddGenerated(b, false) - serial.ColumnAddVirtual(b, false) + serial.ColumnAddGenerated(b, col.Generated != "") + serial.ColumnAddVirtual(b, col.Virtual) serial.ColumnAddHidden(b, false) offs[i] = serial.ColumnEnd(b) } @@ -294,15 +294,7 @@ func deserializeColumns(ctx context.Context, s *serial.TableSchema) ([]schema.Co return nil, err } - cols[i], err = schema.NewColumnWithTypeInfo( - string(c.Name()), - c.Tag(), - sqlType, - c.PrimaryKey(), - string(c.DefaultValue()), - c.AutoIncrement(), - string(c.Comment()), - constraintsFromSerialColumn(&c)...) + cols[i], err = schema.NewColumnWithTypeInfo(string(c.Name()), c.Tag(), sqlType, c.PrimaryKey(), string(c.DefaultValue()), "", false, c.AutoIncrement(), string(c.Comment()), constraintsFromSerialColumn(&c)...) if err != nil { return nil, err } diff --git a/go/libraries/doltcore/sqle/alterschema.go b/go/libraries/doltcore/sqle/alterschema.go index 1e23c71a0b..a7ff12084e 100755 --- a/go/libraries/doltcore/sqle/alterschema.go +++ b/go/libraries/doltcore/sqle/alterschema.go @@ -104,9 +104,9 @@ func orderToOrder(order *sql.ColumnOrder) *schema.ColumnOrder { func createColumn(nullable Nullable, newColName string, tag uint64, typeInfo typeinfo.TypeInfo, defaultVal, comment string) (schema.Column, error) { if nullable { - return schema.NewColumnWithTypeInfo(newColName, tag, typeInfo, false, defaultVal, false, comment) + return schema.NewColumnWithTypeInfo(newColName, tag, typeInfo, false, defaultVal, "", false, false, comment) } else { - return schema.NewColumnWithTypeInfo(newColName, tag, typeInfo, false, defaultVal, false, comment, schema.NotNullConstraint{}) + return schema.NewColumnWithTypeInfo(newColName, tag, typeInfo, false, defaultVal, "", false, false, comment, schema.NotNullConstraint{}) } } diff --git a/go/libraries/doltcore/sqle/alterschema_test.go b/go/libraries/doltcore/sqle/alterschema_test.go index 8169859878..b415853661 100644 --- a/go/libraries/doltcore/sqle/alterschema_test.go +++ b/go/libraries/doltcore/sqle/alterschema_test.go @@ -650,7 +650,7 @@ func TestModifyColumn(t *testing.T) { ) ti, err := typeinfo.FromSqlType(gmstypes.MustCreateStringWithDefaults(sqltypes.VarChar, 599)) require.NoError(t, err) - newNameColSameTag, err := schema.NewColumnWithTypeInfo("name", dtestutils.NameTag, ti, false, "", false, "", schema.NotNullConstraint{}) + newNameColSameTag, err := schema.NewColumnWithTypeInfo("name", dtestutils.NameTag, ti, false, "", "", false, false, "", schema.NotNullConstraint{}) require.NoError(t, err) alteredTypeSch2 := dtestutils.CreateSchema( schema.NewColumn("id", dtestutils.IdTag, types.StringKind, true, schema.NotNullConstraint{}), diff --git a/go/libraries/doltcore/sqle/common_test.go b/go/libraries/doltcore/sqle/common_test.go index 914ceb8b22..142abc6803 100644 --- a/go/libraries/doltcore/sqle/common_test.go +++ b/go/libraries/doltcore/sqle/common_test.go @@ -112,7 +112,7 @@ func schemaNewColumn(t *testing.T, name string, tag uint64, sqlType sql.Type, pa func schemaNewColumnWDefVal(t *testing.T, name string, tag uint64, sqlType sql.Type, partOfPK bool, defaultVal string, constraints ...schema.ColConstraint) schema.Column { typeInfo, err := typeinfo.FromSqlType(sqlType) require.NoError(t, err) - col, err := schema.NewColumnWithTypeInfo(name, tag, typeInfo, partOfPK, defaultVal, false, "", constraints...) + col, err := schema.NewColumnWithTypeInfo(name, tag, typeInfo, partOfPK, defaultVal, "", false, false, "", constraints...) require.NoError(t, err) return col } diff --git a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go index 35ad70438f..f2d6d45dc4 100644 --- a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go +++ b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go @@ -715,7 +715,7 @@ func calculateConflictSchema(base, ours, theirs schema.Schema) (schema.Schema, * if !stripConstraints { cons = col.Constraints } - c, err := schema.NewColumnWithTypeInfo(prefix+col.Name, uint64(i), col.TypeInfo, false, col.Default, false, col.Comment, cons...) + c, err := schema.NewColumnWithTypeInfo(prefix+col.Name, uint64(i), col.TypeInfo, false, col.Default, "", false, false, col.Comment, cons...) if err != nil { return true, err } @@ -732,7 +732,7 @@ func calculateConflictSchema(base, ours, theirs schema.Schema) (schema.Schema, * if !stripConstraints { cons = col.Constraints } - c, err := schema.NewColumnWithTypeInfo(prefix+col.Name, uint64(i), col.TypeInfo, false, col.Default, false, col.Comment, cons...) + c, err := schema.NewColumnWithTypeInfo(prefix+col.Name, uint64(i), col.TypeInfo, false, col.Default, "", false, false, col.Comment, cons...) if err != nil { return true, err } diff --git a/go/libraries/doltcore/sqle/dtables/diff_table.go b/go/libraries/doltcore/sqle/dtables/diff_table.go index 31b610b95f..2b534e62fe 100644 --- a/go/libraries/doltcore/sqle/dtables/diff_table.go +++ b/go/libraries/doltcore/sqle/dtables/diff_table.go @@ -917,7 +917,7 @@ func CalculateDiffSchema(fromSch, toSch schema.Schema) (schema.Schema, error) { i := 0 err = toSch.GetAllCols().Iter(func(tag uint64, col schema.Column) (stop bool, err error) { - toCol, err := schema.NewColumnWithTypeInfo(diff.ToColNamer(col.Name), uint64(i), col.TypeInfo, false, col.Default, false, col.Comment) + toCol, err := schema.NewColumnWithTypeInfo(diff.ToColNamer(col.Name), uint64(i), col.TypeInfo, false, col.Default, "", false, false, col.Comment) if err != nil { return true, err } @@ -931,7 +931,7 @@ func CalculateDiffSchema(fromSch, toSch schema.Schema) (schema.Schema, error) { j := toSch.GetAllCols().Size() err = fromSch.GetAllCols().Iter(func(tag uint64, col schema.Column) (stop bool, err error) { - fromCol, err := schema.NewColumnWithTypeInfo(diff.FromColNamer(col.Name), uint64(j), col.TypeInfo, false, col.Default, false, col.Comment) + fromCol, err := schema.NewColumnWithTypeInfo(diff.FromColNamer(col.Name), uint64(j), col.TypeInfo, false, col.Default, "", false, false, col.Comment) if err != nil { return true, err } diff --git a/go/libraries/doltcore/sqle/index/noms_index_iter.go b/go/libraries/doltcore/sqle/index/noms_index_iter.go index 686628b962..a9ca8f4b7d 100644 --- a/go/libraries/doltcore/sqle/index/noms_index_iter.go +++ b/go/libraries/doltcore/sqle/index/noms_index_iter.go @@ -261,7 +261,7 @@ func NewCoveringIndexRowIterAdapter(ctx *sql.Context, idx DoltIndex, keyIter *no _, partOfIndexKey := idxCols.GetByTag(col.Tag) _, partOfTableKeys := tblPKCols.GetByTag(col.Tag) if partOfIndexKey != partOfTableKeys { - cols[i], _ = schema.NewColumnWithTypeInfo(col.Name, col.Tag, col.TypeInfo, partOfIndexKey, col.Default, col.AutoIncrement, col.Comment, col.Constraints...) + cols[i], _ = schema.NewColumnWithTypeInfo(col.Name, col.Tag, col.TypeInfo, partOfIndexKey, col.Default, "", false, col.AutoIncrement, col.Comment, col.Constraints...) } } diff --git a/go/libraries/doltcore/sqle/schema_table.go b/go/libraries/doltcore/sqle/schema_table.go index 9efc5ce3d3..8174721b4e 100644 --- a/go/libraries/doltcore/sqle/schema_table.go +++ b/go/libraries/doltcore/sqle/schema_table.go @@ -40,7 +40,7 @@ type Extra struct { } func mustNewColWithTypeInfo(name string, tag uint64, typeInfo typeinfo.TypeInfo, partOfPK bool, defaultVal string, autoIncrement bool, comment string, constraints ...schema.ColConstraint) schema.Column { - col, err := schema.NewColumnWithTypeInfo(name, tag, typeInfo, partOfPK, defaultVal, autoIncrement, comment, constraints...) + col, err := schema.NewColumnWithTypeInfo(name, tag, typeInfo, partOfPK, defaultVal, "", false, autoIncrement, comment, constraints...) if err != nil { panic(err) } diff --git a/go/libraries/doltcore/sqle/sqlutil/convert.go b/go/libraries/doltcore/sqle/sqlutil/convert.go index d52b8a7ee3..32c0c05b24 100644 --- a/go/libraries/doltcore/sqle/sqlutil/convert.go +++ b/go/libraries/doltcore/sqle/sqlutil/convert.go @@ -137,7 +137,7 @@ func ToDoltCol(tag uint64, col *sql.Column) (schema.Column, error) { return schema.Column{}, err } - return schema.NewColumnWithTypeInfo(col.Name, tag, typeInfo, col.PrimaryKey, col.Default.String(), col.AutoIncrement, col.Comment, constraints...) + return schema.NewColumnWithTypeInfo(col.Name, tag, typeInfo, col.PrimaryKey, col.Default.String(), "", false, col.AutoIncrement, col.Comment, constraints...) } // ToDoltResultSchema returns a dolt Schema from the sql schema given, suitable for use as a result set diff --git a/go/libraries/doltcore/sqle/testdata.go b/go/libraries/doltcore/sqle/testdata.go index 7c19497795..a455077bcd 100644 --- a/go/libraries/doltcore/sqle/testdata.go +++ b/go/libraries/doltcore/sqle/testdata.go @@ -115,7 +115,7 @@ func createAppearancesTestSchema() schema.Schema { } func newColumnWithTypeInfo(name string, tag uint64, info typeinfo.TypeInfo, partOfPk bool, constraints ...schema.ColConstraint) schema.Column { - col, err := schema.NewColumnWithTypeInfo(name, tag, info, partOfPk, "", false, "", constraints...) + col, err := schema.NewColumnWithTypeInfo(name, tag, info, partOfPk, "", "", false, false, "", constraints...) if err != nil { panic(fmt.Sprintf("unexpected error creating column: %s", err.Error())) } From c084c3188762286cced2b883c484247dda6b45c9 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Thu, 26 Oct 2023 12:54:53 -0700 Subject: [PATCH 03/26] Fixed deserialization --- .../doltcore/schema/encoding/serialization.go | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/go/libraries/doltcore/schema/encoding/serialization.go b/go/libraries/doltcore/schema/encoding/serialization.go index 75f8dcbfc1..a7ac2a3a98 100644 --- a/go/libraries/doltcore/schema/encoding/serialization.go +++ b/go/libraries/doltcore/schema/encoding/serialization.go @@ -294,7 +294,26 @@ func deserializeColumns(ctx context.Context, s *serial.TableSchema) ([]schema.Co return nil, err } - cols[i], err = schema.NewColumnWithTypeInfo(string(c.Name()), c.Tag(), sqlType, c.PrimaryKey(), string(c.DefaultValue()), "", false, c.AutoIncrement(), string(c.Comment()), constraintsFromSerialColumn(&c)...) + defVal := "" + generatedVal := "" + if c.DefaultValue() != nil { + defVal = string(c.DefaultValue()) + } else { + generatedVal = string(c.DefaultValue()) + } + + cols[i], err = schema.NewColumnWithTypeInfo( + string(c.Name()), + c.Tag(), + sqlType, + c.PrimaryKey(), + defVal, + generatedVal, + c.Virtual(), + c.AutoIncrement(), + string(c.Comment()), + constraintsFromSerialColumn(&c)..., + ) if err != nil { return nil, err } From dabe6b972815e0482f776fc60d3789124c18b6e4 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Thu, 26 Oct 2023 13:01:20 -0700 Subject: [PATCH 04/26] Bug fix for serialization, added generated tests to dolt_engine_tests --- go/libraries/doltcore/schema/encoding/serialization.go | 9 ++++++++- .../doltcore/sqle/enginetest/dolt_engine_test.go | 4 ++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/go/libraries/doltcore/schema/encoding/serialization.go b/go/libraries/doltcore/schema/encoding/serialization.go index a7ac2a3a98..bcbaca840f 100644 --- a/go/libraries/doltcore/schema/encoding/serialization.go +++ b/go/libraries/doltcore/schema/encoding/serialization.go @@ -205,11 +205,18 @@ func serializeSchemaColumns(b *fb.Builder, sch schema.Schema) fb.UOffsetT { for i := len(cols) - 1; i >= 0; i-- { col := cols[i] co := b.CreateString(col.Comment) - do := b.CreateString(col.Default) typeString := sqlTypeString(col.TypeInfo) to := b.CreateString(typeString) no := b.CreateString(col.Name) + defVal := "" + if col.Default != "" { + defVal = col.Default + } else { + defVal = col.Generated + } + do := b.CreateString(defVal) + serial.ColumnStart(b) serial.ColumnAddName(b, no) serial.ColumnAddSqlType(b, to) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go index e12d00e5ef..9ce5c3f922 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go @@ -507,6 +507,10 @@ func TestInsertIntoErrors(t *testing.T) { enginetest.TestInsertIntoErrors(t, h) } +func TestGeneratedColumns(t *testing.T) { + enginetest.TestGeneratedColumns(t, enginetest.NewDefaultMemoryHarness()) +} + func TestSpatialQueries(t *testing.T) { h := newDoltHarness(t) defer h.Close() From ca43a450134311833f9bb9cf75039d4feaf8f2fe Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Fri, 27 Oct 2023 16:48:04 -0700 Subject: [PATCH 05/26] Fixed some bugs in serialization / deserialization, getting started on converting virtual columns on store / read --- .../doltcore/doltdb/durable/artifact_index.go | 6 ++-- .../doltcore/doltdb/durable/conflict_index.go | 6 ++-- go/libraries/doltcore/doltdb/durable/index.go | 6 ++-- go/libraries/doltcore/doltdb/durable/table.go | 2 +- .../doltcore/schema/encoding/serialization.go | 8 +++-- go/libraries/doltcore/schema/schema_impl.go | 8 +++++ .../doltcore/sqle/index/prolly_row_iter.go | 2 +- go/libraries/doltcore/sqle/sqlutil/convert.go | 30 +++++++++++++++++-- .../writer/prolly_index_writer_keyless.go | 2 ++ .../sqle/writer/prolly_table_writer.go | 7 ++--- 10 files changed, 56 insertions(+), 21 deletions(-) diff --git a/go/libraries/doltcore/doltdb/durable/artifact_index.go b/go/libraries/doltcore/doltdb/durable/artifact_index.go index 07da0dd342..0378bd6278 100644 --- a/go/libraries/doltcore/doltdb/durable/artifact_index.go +++ b/go/libraries/doltcore/doltdb/durable/artifact_index.go @@ -50,7 +50,7 @@ func RefFromArtifactIndex(ctx context.Context, vrw types.ValueReadWriter, idx Ar return refFromNomsValue(ctx, vrw, b) default: - return types.Ref{}, errNbfUnkown + return types.Ref{}, errNbfUnknown } } @@ -69,7 +69,7 @@ func NewEmptyArtifactIndex(ctx context.Context, vrw types.ValueReadWriter, ns tr return ArtifactIndexFromProllyMap(m), nil default: - return nil, errNbfUnkown + return nil, errNbfUnknown } } @@ -107,7 +107,7 @@ func artifactIndexFromAddr(ctx context.Context, vrw types.ValueReadWriter, ns tr return ArtifactIndexFromProllyMap(m), nil default: - return nil, errNbfUnkown + return nil, errNbfUnknown } } diff --git a/go/libraries/doltcore/doltdb/durable/conflict_index.go b/go/libraries/doltcore/doltdb/durable/conflict_index.go index d892826543..bad33664f1 100644 --- a/go/libraries/doltcore/doltdb/durable/conflict_index.go +++ b/go/libraries/doltcore/doltdb/durable/conflict_index.go @@ -40,7 +40,7 @@ func RefFromConflictIndex(ctx context.Context, vrw types.ValueReadWriter, idx Co return types.Ref{}, fmt.Errorf("__DOLT__ conflicts should be stored in ArtifactIndex") default: - return types.Ref{}, errNbfUnkown + return types.Ref{}, errNbfUnknown } } @@ -58,7 +58,7 @@ func NewEmptyConflictIndex(ctx context.Context, vrw types.ValueReadWriter, ns tr return nil, fmt.Errorf("__DOLT__ conflicts should be stored in ArtifactIndex") default: - return nil, errNbfUnkown + return nil, errNbfUnknown } } @@ -91,7 +91,7 @@ func conflictIndexFromAddr(ctx context.Context, vrw types.ValueReadWriter, ns tr return nil, fmt.Errorf("__DOLT__ conflicts should be stored in ArtifactIndex") default: - return nil, errNbfUnkown + return nil, errNbfUnknown } } diff --git a/go/libraries/doltcore/doltdb/durable/index.go b/go/libraries/doltcore/doltdb/durable/index.go index e1f6009a29..4953880b35 100644 --- a/go/libraries/doltcore/doltdb/durable/index.go +++ b/go/libraries/doltcore/doltdb/durable/index.go @@ -88,7 +88,7 @@ func RefFromIndex(ctx context.Context, vrw types.ValueReadWriter, idx Index) (ty return refFromNomsValue(ctx, vrw, b) default: - return types.Ref{}, errNbfUnkown + return types.Ref{}, errNbfUnknown } } @@ -115,7 +115,7 @@ func indexFromAddr(ctx context.Context, vrw types.ValueReadWriter, ns tree.NodeS return IndexFromProllyMap(pm), nil default: - return nil, errNbfUnkown + return nil, errNbfUnknown } } @@ -138,7 +138,7 @@ func NewEmptyIndex(ctx context.Context, vrw types.ValueReadWriter, ns tree.NodeS return IndexFromProllyMap(m), nil default: - return nil, errNbfUnkown + return nil, errNbfUnknown } } diff --git a/go/libraries/doltcore/doltdb/durable/table.go b/go/libraries/doltcore/doltdb/durable/table.go index d67f3b719a..badb50ac46 100644 --- a/go/libraries/doltcore/doltdb/durable/table.go +++ b/go/libraries/doltcore/doltdb/durable/table.go @@ -52,7 +52,7 @@ var ( ) var ( - errNbfUnkown = fmt.Errorf("unknown NomsBinFormat") + errNbfUnknown = fmt.Errorf("unknown NomsBinFormat") errNbfUnsupported = fmt.Errorf("operation unsupported for NomsBinFormat") ) diff --git a/go/libraries/doltcore/schema/encoding/serialization.go b/go/libraries/doltcore/schema/encoding/serialization.go index bcbaca840f..d0c5b0a9bb 100644 --- a/go/libraries/doltcore/schema/encoding/serialization.go +++ b/go/libraries/doltcore/schema/encoding/serialization.go @@ -304,9 +304,11 @@ func deserializeColumns(ctx context.Context, s *serial.TableSchema) ([]schema.Co defVal := "" generatedVal := "" if c.DefaultValue() != nil { - defVal = string(c.DefaultValue()) - } else { - generatedVal = string(c.DefaultValue()) + if c.Generated() { + generatedVal = string(c.DefaultValue()) + } else { + defVal = string(c.DefaultValue()) + } } cols[i], err = schema.NewColumnWithTypeInfo( diff --git a/go/libraries/doltcore/schema/schema_impl.go b/go/libraries/doltcore/schema/schema_impl.go index 9ffabf7906..f3eabfc859 100644 --- a/go/libraries/doltcore/schema/schema_impl.go +++ b/go/libraries/doltcore/schema/schema_impl.go @@ -417,6 +417,10 @@ func (si *schemaImpl) GetKeyDescriptor() val.TupleDesc { useCollations := false // We only use collations if a string exists var collations []sql.CollationID _ = si.GetPKCols().Iter(func(tag uint64, col Column) (stop bool, err error) { + if col.Virtual { + return + } + sqlType := col.TypeInfo.ToSqlType() queryType := sqlType.Type() var t val.Type @@ -473,6 +477,10 @@ func (si *schemaImpl) GetValueDescriptor() val.TupleDesc { useCollations := false // We only use collations if a string exists _ = si.GetNonPKCols().Iter(func(tag uint64, col Column) (stop bool, err error) { + if col.Virtual { + return + } + sqlType := col.TypeInfo.ToSqlType() queryType := sqlType.Type() tt = append(tt, val.Type{ diff --git a/go/libraries/doltcore/sqle/index/prolly_row_iter.go b/go/libraries/doltcore/sqle/index/prolly_row_iter.go index 24ef387329..0600b78477 100644 --- a/go/libraries/doltcore/sqle/index/prolly_row_iter.go +++ b/go/libraries/doltcore/sqle/index/prolly_row_iter.go @@ -32,7 +32,7 @@ type prollyRowIter struct { keyProj []int valProj []int - // orjProj is a concatenated list of output ordinals for |keyProj| and |valProj| + // ordProj is a concatenated list of output ordinals for |keyProj| and |valProj| ordProj []int rowLen int } diff --git a/go/libraries/doltcore/sqle/sqlutil/convert.go b/go/libraries/doltcore/sqle/sqlutil/convert.go index 32c0c05b24..1bb2cba36c 100644 --- a/go/libraries/doltcore/sqle/sqlutil/convert.go +++ b/go/libraries/doltcore/sqle/sqlutil/convert.go @@ -40,21 +40,26 @@ func FromDoltSchema(dbName, tableName string, sch schema.Schema) (sql.PrimaryKey extra = "auto_increment" } - var deflt *sql.ColumnDefaultValue + var deflt, generated *sql.ColumnDefaultValue if col.Default != "" { deflt = sql.NewUnresolvedColumnDefaultValue(col.Default) } - + if col.Generated != "" { + generated = sql.NewUnresolvedColumnDefaultValue(col.Generated) + } + cols[i] = &sql.Column{ Name: col.Name, Type: sqlType, Default: deflt, + Generated: generated, Nullable: col.IsNullable(), DatabaseSource: dbName, Source: tableName, PrimaryKey: col.IsPartOfPK, AutoIncrement: col.AutoIncrement, Comment: col.Comment, + Virtual: col.Virtual, Extra: extra, } i++ @@ -137,7 +142,26 @@ func ToDoltCol(tag uint64, col *sql.Column) (schema.Column, error) { return schema.Column{}, err } - return schema.NewColumnWithTypeInfo(col.Name, tag, typeInfo, col.PrimaryKey, col.Default.String(), "", false, col.AutoIncrement, col.Comment, constraints...) + defaultVal := "" + generatedVal := "" + if col.Default != nil { + defaultVal = col.Default.String() + } else { + generatedVal = col.Generated.String() + } + + return schema.NewColumnWithTypeInfo( + col.Name, + tag, + typeInfo, + col.PrimaryKey, + defaultVal, + generatedVal, + col.Virtual, + col.AutoIncrement, + col.Comment, + constraints..., + ) } // ToDoltResultSchema returns a dolt Schema from the sql schema given, suitable for use as a result set diff --git a/go/libraries/doltcore/sqle/writer/prolly_index_writer_keyless.go b/go/libraries/doltcore/sqle/writer/prolly_index_writer_keyless.go index a7aa042f99..ebdb52fc2d 100644 --- a/go/libraries/doltcore/sqle/writer/prolly_index_writer_keyless.go +++ b/go/libraries/doltcore/sqle/writer/prolly_index_writer_keyless.go @@ -32,6 +32,7 @@ type prollyKeylessWriter struct { keyBld *val.TupleBuilder valBld *val.TupleBuilder valMap val.OrdinalMapping + virtualCols []int } var _ indexWriter = prollyKeylessWriter{} @@ -51,6 +52,7 @@ func (k prollyKeylessWriter) ValidateKeyViolations(ctx context.Context, sqlRow s } func (k prollyKeylessWriter) Insert(ctx context.Context, sqlRow sql.Row) error { + // TODO: this thing needs a function that can convert a row to it storage equivalent as necessary hashId, value, err := k.tuplesFromRow(ctx, sqlRow) if err != nil { return err diff --git a/go/libraries/doltcore/sqle/writer/prolly_table_writer.go b/go/libraries/doltcore/sqle/writer/prolly_table_writer.go index 449d3e92f7..a91b68b1b4 100644 --- a/go/libraries/doltcore/sqle/writer/prolly_table_writer.go +++ b/go/libraries/doltcore/sqle/writer/prolly_table_writer.go @@ -380,10 +380,9 @@ func makeOrdinalMapping(from sql.Schema, to *schema.ColCollection) (m val.Ordina m = make(val.OrdinalMapping, len(to.GetColumns())) for i := range m { name := to.GetByIndex(i).Name - for j, col := range from { - if col.Name == name { - m[i] = j - } + colIdx := from.IndexOfColName(name) + if !from[colIdx].Virtual { + m[i] = colIdx } } return From 5dd51c613af7e0926186f2cc03a0dafe49eb392f Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Mon, 30 Oct 2023 13:50:49 -0700 Subject: [PATCH 06/26] Fixed inserts on virtual columns --- go/libraries/doltcore/schema/col_coll.go | 53 +++++++++++++--- go/libraries/doltcore/schema/schema.go | 4 ++ .../sqle/enginetest/dolt_engine_test.go | 63 +++++++++++++------ .../doltcore/sqle/enginetest/dolt_harness.go | 1 + .../doltcore/sqle/index/prolly_row_iter.go | 20 +++++- .../sqle/writer/prolly_table_writer.go | 9 ++- 6 files changed, 115 insertions(+), 35 deletions(-) diff --git a/go/libraries/doltcore/schema/col_coll.go b/go/libraries/doltcore/schema/col_coll.go index d11c4eaf13..731df6a5af 100644 --- a/go/libraries/doltcore/schema/col_coll.go +++ b/go/libraries/doltcore/schema/col_coll.go @@ -37,13 +37,13 @@ var ErrNoPrimaryKeyColumns = errors.New("no primary key columns") var ErrNonAutoIncType = errors.New("column type cannot be auto incremented") var EmptyColColl = &ColCollection{ - []Column{}, - []uint64{}, - []uint64{}, - map[uint64]Column{}, - map[string]Column{}, - map[string]Column{}, - map[uint64]int{}, + cols: []Column{}, + Tags: []uint64{}, + SortedTags: []uint64{}, + TagToCol: map[uint64]Column{}, + NameToCol: map[string]Column{}, + LowerNameToCol: map[string]Column{}, + TagToIdx: map[uint64]int{}, } // ColCollection is a collection of columns. As a stand-alone collection, all columns in the collection must have unique @@ -51,6 +51,8 @@ var EmptyColColl = &ColCollection{ // See schema.ValidateForInsert for details. type ColCollection struct { cols []Column + // virtualColumns stores the indexes of any virtual columns in the collection + virtualColumns []int // Tags is a list of all the tags in the ColCollection in their original order. Tags []uint64 // SortedTags is a list of all the tags in the ColCollection in sorted order. @@ -78,6 +80,7 @@ func NewColCollection(cols ...Column) *ColCollection { nameToCol := make(map[string]Column, len(cols)) lowerNameToCol := make(map[string]Column, len(cols)) tagToIdx := make(map[uint64]int, len(cols)) + var virtualColumns []int var columns []Column for i, col := range cols { @@ -96,12 +99,17 @@ func NewColCollection(cols ...Column) *ColCollection { if _, ok := lowerNameToCol[lowerCaseName]; !ok { lowerNameToCol[lowerCaseName] = cols[i] } + + if col.Virtual { + virtualColumns = append(virtualColumns, i) + } } sort.Slice(sortedTags, func(i, j int) bool { return sortedTags[i] < sortedTags[j] }) return &ColCollection{ cols: columns, + virtualColumns: virtualColumns, Tags: tags, SortedTags: sortedTags, TagToCol: tagToCol, @@ -225,11 +233,42 @@ func (cc *ColCollection) GetByIndex(idx int) Column { return cc.cols[idx] } +// GetByStoredIndex returns the column with the given storage index (omitting virtual columns from index calculation) +func (cc *ColCollection) GetByStoredIndex(idx int) Column { + if len(cc.virtualColumns) == 0 { + return cc.cols[idx] + } + + storageIdx := 0 + for _, col := range cc.cols { + if col.Virtual { + continue + } + if idx == storageIdx { + return col + } + storageIdx++ + } + + return InvalidCol +} + // Size returns the number of columns in the collection. func (cc *ColCollection) Size() int { return len(cc.cols) } +// StoredSize returns the number of non-virtual columns in the collection +func (cc *ColCollection) StoredSize() int { + num := 0 + for _, col := range cc.cols { + if !col.Virtual { + num++ + } + } + return num +} + // Contains returns whether this column collection contains a column with the name given, case insensitive func (cc *ColCollection) Contains(name string) bool { _, ok := cc.GetByNameCaseInsensitive(name) diff --git a/go/libraries/doltcore/schema/schema.go b/go/libraries/doltcore/schema/schema.go index bcb3c3c735..86f85aa484 100644 --- a/go/libraries/doltcore/schema/schema.go +++ b/go/libraries/doltcore/schema/schema.go @@ -129,6 +129,10 @@ func IsKeyless(sch Schema) bool { sch.GetAllCols().Size() != 0 } +func IsVirtual(sch Schema) bool { + return sch != nil && len(sch.GetAllCols().virtualColumns) > 0 +} + func HasAutoIncrement(sch Schema) (ok bool) { _ = sch.GetAllCols().Iter(func(tag uint64, col Column) (stop bool, err error) { if col.AutoIncrement { diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go index 9ce5c3f922..6761d23c09 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go @@ -29,6 +29,7 @@ import ( "github.com/dolthub/go-mysql-server/sql" "github.com/dolthub/go-mysql-server/sql/memo" "github.com/dolthub/go-mysql-server/sql/mysql_db" + "github.com/dolthub/go-mysql-server/sql/plan" gmstypes "github.com/dolthub/go-mysql-server/sql/types" "github.com/dolthub/vitess/go/mysql" "github.com/stretchr/testify/assert" @@ -91,8 +92,8 @@ func TestSingleQuery(t *testing.T) { enginetest.RunQuery(t, engine, harness, q) } - engine.EngineAnalyzer().Debug = true - engine.EngineAnalyzer().Verbose = true + // engine.EngineAnalyzer().Debug = true + // engine.EngineAnalyzer().Verbose = true var test queries.QueryTest test = queries.QueryTest{ @@ -115,41 +116,63 @@ func TestSingleQuery(t *testing.T) { // Convenience test for debugging a single query. Unskip and set to the desired query. func TestSingleScript(t *testing.T) { - t.Skip() + // t.Skip() var scripts = []queries.ScriptTest{ { - Name: "failed statements data validation for DELETE, REPLACE", + Name: "virtual column ordering", SetUpScript: []string{ - "CREATE TABLE test (pk BIGINT PRIMARY KEY, v1 BIGINT, INDEX (v1));", - "INSERT INTO test VALUES (1,1), (4,4), (5,5);", - "CREATE TABLE test2 (pk BIGINT PRIMARY KEY, CONSTRAINT fk_test FOREIGN KEY (pk) REFERENCES test (v1));", - "INSERT INTO test2 VALUES (4);", + // virtual is the default for generated columns + "create table t1 (v1 int generated always as (2), a int, v2 int generated always as (a + v1), c int)", }, Assertions: []queries.ScriptTestAssertion{ { - Query: "DELETE FROM test WHERE pk > 0;", - ExpectedErr: sql.ErrForeignKeyParentViolation, + Query: "insert into t1 (a, c) values (1,5), (3,7)", + Expected: []sql.Row{{gmstypes.NewOkResult(2)}}, }, { - Query: "SELECT * FROM test;", - Expected: []sql.Row{{1, 1}, {4, 4}, {5, 5}}, + Query: "insert into t1 (c, a) values (5,6), (7,8)", + Expected: []sql.Row{{gmstypes.NewOkResult(2)}}, }, { - Query: "SELECT * FROM test2;", - Expected: []sql.Row{{4}}, + Query: "select * from t1 order by a", + Expected: []sql.Row{ + {2, 1, 3, 5}, + {2, 3, 5, 7}, + {2, 6, 8, 5}, + {2, 8, 10, 7}, + }, }, { - Query: "REPLACE INTO test VALUES (1,7), (4,8), (5,9);", - ExpectedErr: sql.ErrForeignKeyParentViolation, + Query: "update t1 set a = 4 where a = 3", + Expected: []sql.Row{{gmstypes.OkResult{ + RowsAffected: 1, + Info: plan.UpdateInfo{ + Matched: 1, + Updated: 1, + }}, + }}, }, { - Query: "SELECT * FROM test;", - Expected: []sql.Row{{1, 1}, {4, 4}, {5, 5}}, + Query: "select * from t1 order by a", + Expected: []sql.Row{ + {2, 1, 3, 5}, + {2, 4, 6, 7}, + {2, 6, 8, 5}, + {2, 8, 10, 7}, + }, }, { - Query: "SELECT * FROM test2;", - Expected: []sql.Row{{4}}, + Query: "delete from t1 where v2 = 6", + Expected: []sql.Row{{gmstypes.OkResult{RowsAffected: 1}}}, + }, + { + Query: "select * from t1 order by a", + Expected: []sql.Row{ + {2, 1, 3, 5}, + {2, 6, 8, 5}, + {2, 8, 10, 7}, + }, }, }, }, diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_harness.go b/go/libraries/doltcore/sqle/enginetest/dolt_harness.go index 532a20bee3..0d391ea91f 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_harness.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_harness.go @@ -69,6 +69,7 @@ func newDoltHarness(t *testing.T) *DoltHarness { dh := &DoltHarness{ t: t, skippedQueries: defaultSkippedQueries, + parallelism: 1, } return dh diff --git a/go/libraries/doltcore/sqle/index/prolly_row_iter.go b/go/libraries/doltcore/sqle/index/prolly_row_iter.go index 0600b78477..6268264f0e 100644 --- a/go/libraries/doltcore/sqle/index/prolly_row_iter.go +++ b/go/libraries/doltcore/sqle/index/prolly_row_iter.go @@ -90,6 +90,7 @@ func projectionMappings(sch schema.Schema, projections []uint64) (keyMap, valMap j-- } } + keyMap = allMap[:i] valMap = allMap[i:len(projections)] ordMap = allMap[len(projections):] @@ -102,6 +103,16 @@ func projectionMappings(sch schema.Schema, projections []uint64) (keyMap, valMap valMap[i]++ } } + + // For virtual schemas, insert -1 into the ordinal mapping for every virtual column + if schema.IsVirtual(sch) { + for i := range ordMap { + if sch.GetAllCols().GetByIndex(i).Virtual { + ordMap[i] = -1 + } + } + } + return } @@ -173,9 +184,12 @@ func (it *prollyKeylessIter) nextTuple(ctx *sql.Context) error { for i, idx := range it.valProj { outputIdx := it.ordProj[i] - it.curr[outputIdx], err = GetField(ctx, it.valDesc, idx, value, it.ns) - if err != nil { - return err + // TODO: this is bad, it introduces a branch here where it isn't necessary + if outputIdx >= 0 { + it.curr[outputIdx], err = GetField(ctx, it.valDesc, idx, value, it.ns) + if err != nil { + return err + } } } return nil diff --git a/go/libraries/doltcore/sqle/writer/prolly_table_writer.go b/go/libraries/doltcore/sqle/writer/prolly_table_writer.go index a91b68b1b4..39d06486ab 100644 --- a/go/libraries/doltcore/sqle/writer/prolly_table_writer.go +++ b/go/libraries/doltcore/sqle/writer/prolly_table_writer.go @@ -377,13 +377,12 @@ func ordinalMappingsFromSchema(from sql.Schema, to schema.Schema) (km, vm val.Or } func makeOrdinalMapping(from sql.Schema, to *schema.ColCollection) (m val.OrdinalMapping) { - m = make(val.OrdinalMapping, len(to.GetColumns())) + m = make(val.OrdinalMapping, to.StoredSize()) for i := range m { - name := to.GetByIndex(i).Name + col := to.GetByStoredIndex(i) + name := col.Name colIdx := from.IndexOfColName(name) - if !from[colIdx].Virtual { - m[i] = colIdx - } + m[i] = colIdx } return } From 3d2a23f7ea71a3062f86511a03b0cf1af588dfe0 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Mon, 30 Oct 2023 14:09:34 -0700 Subject: [PATCH 07/26] Second pass --- .../doltcore/sqle/index/prolly_row_iter.go | 47 ++++++++++--------- 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/go/libraries/doltcore/sqle/index/prolly_row_iter.go b/go/libraries/doltcore/sqle/index/prolly_row_iter.go index 6268264f0e..819a26d9c8 100644 --- a/go/libraries/doltcore/sqle/index/prolly_row_iter.go +++ b/go/libraries/doltcore/sqle/index/prolly_row_iter.go @@ -76,24 +76,37 @@ func projectionMappings(sch schema.Schema, projections []uint64) (keyMap, valMap pks := sch.GetPKCols() nonPks := sch.GetNonPKCols() - allMap := make([]int, 2*len(projections)) + // Our mappings should only contain the physical columns of the schema + numPhysicalColumns := len(projections) + if schema.IsVirtual(sch) { + numPhysicalColumns = 0 + for _, t := range projections { + if idx, ok := pks.TagToIdx[t]; ok && !sch.GetAllCols().GetByIndex(idx).Virtual { + numPhysicalColumns++ + } else if idx, ok := nonPks.TagToIdx[t]; ok && !sch.GetAllCols().GetByIndex(idx).Virtual { + numPhysicalColumns++ + } + } + } + + allMap := make([]int, 2*numPhysicalColumns) i := 0 - j := len(projections) - 1 + j := numPhysicalColumns - 1 for k, t := range projections { - if idx, ok := pks.TagToIdx[t]; ok { - allMap[len(projections)+i] = k + if idx, ok := pks.TagToIdx[t]; ok && !pks.GetByIndex(idx).Virtual { + allMap[numPhysicalColumns+i] = k allMap[i] = idx i++ - } else if idx, ok := nonPks.TagToIdx[t]; ok { + } else if idx, ok := nonPks.TagToIdx[t]; ok && !nonPks.GetByIndex(idx).Virtual { allMap[j] = idx - allMap[len(projections)+j] = k + allMap[numPhysicalColumns+j] = k j-- } } keyMap = allMap[:i] - valMap = allMap[i:len(projections)] - ordMap = allMap[len(projections):] + valMap = allMap[i:numPhysicalColumns] + ordMap = allMap[numPhysicalColumns:] if schema.IsKeyless(sch) { // skip the cardinality value, increment every index for i := range keyMap { @@ -104,15 +117,6 @@ func projectionMappings(sch schema.Schema, projections []uint64) (keyMap, valMap } } - // For virtual schemas, insert -1 into the ordinal mapping for every virtual column - if schema.IsVirtual(sch) { - for i := range ordMap { - if sch.GetAllCols().GetByIndex(i).Virtual { - ordMap[i] = -1 - } - } - } - return } @@ -184,12 +188,9 @@ func (it *prollyKeylessIter) nextTuple(ctx *sql.Context) error { for i, idx := range it.valProj { outputIdx := it.ordProj[i] - // TODO: this is bad, it introduces a branch here where it isn't necessary - if outputIdx >= 0 { - it.curr[outputIdx], err = GetField(ctx, it.valDesc, idx, value, it.ns) - if err != nil { - return err - } + it.curr[outputIdx], err = GetField(ctx, it.valDesc, idx, value, it.ns) + if err != nil { + return err } } return nil From 3164e39a984ec17e57567636a2ad8c5931af2a00 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Mon, 30 Oct 2023 16:49:22 -0700 Subject: [PATCH 08/26] Better book keeping for storage / virtual columns, first working test --- go/libraries/doltcore/schema/col_coll.go | 54 ++++++++++--------- .../doltcore/sqle/index/prolly_row_iter.go | 39 ++++++++------ 2 files changed, 51 insertions(+), 42 deletions(-) diff --git a/go/libraries/doltcore/schema/col_coll.go b/go/libraries/doltcore/schema/col_coll.go index 731df6a5af..c0fdd5c879 100644 --- a/go/libraries/doltcore/schema/col_coll.go +++ b/go/libraries/doltcore/schema/col_coll.go @@ -53,6 +53,8 @@ type ColCollection struct { cols []Column // virtualColumns stores the indexes of any virtual columns in the collection virtualColumns []int + // storedIndexes stores the indexes of the stored columns in the collection + storedIndexes []int // Tags is a list of all the tags in the ColCollection in their original order. Tags []uint64 // SortedTags is a list of all the tags in the ColCollection in sorted order. @@ -65,6 +67,8 @@ type ColCollection struct { LowerNameToCol map[string]Column // TagToIdx is a map from a tag to the column index TagToIdx map[uint64]int + // tagToStorageIndex is a map from a tag to the physical storage column index + tagToStorageIndex map[uint64]int } // NewColCollection creates a new collection from a list of columns. If any columns have the same tag, by-tag lookups in @@ -80,9 +84,12 @@ func NewColCollection(cols ...Column) *ColCollection { nameToCol := make(map[string]Column, len(cols)) lowerNameToCol := make(map[string]Column, len(cols)) tagToIdx := make(map[uint64]int, len(cols)) + tagToStorageIndex := make(map[uint64]int, len(cols)) var virtualColumns []int var columns []Column + var storedIndexes []int + storageIdx := 0 for i, col := range cols { // If multiple columns have the same tag, the last one is used for tag lookups. // Columns must have unique tags to pass schema.ValidateForInsert. @@ -102,20 +109,26 @@ func NewColCollection(cols ...Column) *ColCollection { if col.Virtual { virtualColumns = append(virtualColumns, i) + } else { + storedIndexes = append(storedIndexes, i) + tagToStorageIndex[col.Tag] = storageIdx + storageIdx++ } } sort.Slice(sortedTags, func(i, j int) bool { return sortedTags[i] < sortedTags[j] }) return &ColCollection{ - cols: columns, - virtualColumns: virtualColumns, - Tags: tags, - SortedTags: sortedTags, - TagToCol: tagToCol, - NameToCol: nameToCol, - LowerNameToCol: lowerNameToCol, - TagToIdx: tagToIdx, + cols: columns, + virtualColumns: virtualColumns, + storedIndexes: storedIndexes, + tagToStorageIndex: tagToStorageIndex, + Tags: tags, + SortedTags: sortedTags, + TagToCol: tagToCol, + NameToCol: nameToCol, + LowerNameToCol: lowerNameToCol, + TagToIdx: tagToIdx, } } @@ -228,29 +241,20 @@ func (cc *ColCollection) GetByTag(tag uint64) (Column, bool) { return InvalidCol, false } -// GetByIndex returns a column with a given index +// GetByIndex returns the Nth column in the collection func (cc *ColCollection) GetByIndex(idx int) Column { return cc.cols[idx] } -// GetByStoredIndex returns the column with the given storage index (omitting virtual columns from index calculation) +// GetByStoredIndex returns the Nth stored column (omitting virtual columns from index calculation) func (cc *ColCollection) GetByStoredIndex(idx int) Column { - if len(cc.virtualColumns) == 0 { - return cc.cols[idx] - } + return cc.cols[cc.storedIndexes[idx]] +} - storageIdx := 0 - for _, col := range cc.cols { - if col.Virtual { - continue - } - if idx == storageIdx { - return col - } - storageIdx++ - } - - return InvalidCol +// StoredIndexByTag returns the storage index of the column with the given tag, ignoring virtual columns +func (cc *ColCollection) StoredIndexByTag(tag uint64) (int, bool) { + idx, ok := cc.tagToStorageIndex[tag] + return idx, ok } // Size returns the number of columns in the collection. diff --git a/go/libraries/doltcore/sqle/index/prolly_row_iter.go b/go/libraries/doltcore/sqle/index/prolly_row_iter.go index 819a26d9c8..920571079b 100644 --- a/go/libraries/doltcore/sqle/index/prolly_row_iter.go +++ b/go/libraries/doltcore/sqle/index/prolly_row_iter.go @@ -76,36 +76,41 @@ func projectionMappings(sch schema.Schema, projections []uint64) (keyMap, valMap pks := sch.GetPKCols() nonPks := sch.GetNonPKCols() - // Our mappings should only contain the physical columns of the schema + + numPhysicalColumns := len(projections) if schema.IsVirtual(sch) { numPhysicalColumns = 0 for _, t := range projections { - if idx, ok := pks.TagToIdx[t]; ok && !sch.GetAllCols().GetByIndex(idx).Virtual { - numPhysicalColumns++ - } else if idx, ok := nonPks.TagToIdx[t]; ok && !sch.GetAllCols().GetByIndex(idx).Virtual { + if idx, ok := sch.GetAllCols().TagToIdx[t]; ok && !sch.GetAllCols().GetByIndex(idx).Virtual { numPhysicalColumns++ } } } + // Build a slice of positional values. For a set of P projections, for K key columns and N=P-K non-key columns, + // we'll generate a slice 2P long structured as follows: + // [K key projections, // list of tuple indexes to read for key columns + // N non-key projections, // list of tuple indexes to read for non-key columns + // P output ordinals] // list of output column ordinals for each projection + // Afterward we slice this into three separate mappings to return. allMap := make([]int, 2*numPhysicalColumns) - i := 0 - j := numPhysicalColumns - 1 - for k, t := range projections { - if idx, ok := pks.TagToIdx[t]; ok && !pks.GetByIndex(idx).Virtual { - allMap[numPhysicalColumns+i] = k - allMap[i] = idx - i++ - } else if idx, ok := nonPks.TagToIdx[t]; ok && !nonPks.GetByIndex(idx).Virtual { - allMap[j] = idx - allMap[numPhysicalColumns+j] = k - j-- + keyIdx := 0 + nonKeyIdx := numPhysicalColumns - 1 + for projNum, tag := range projections { + if idx, ok := pks.StoredIndexByTag(tag); ok && !pks.GetByStoredIndex(idx).Virtual { + allMap[keyIdx] = idx + allMap[numPhysicalColumns+keyIdx] = projNum + keyIdx++ + } else if idx, ok := nonPks.StoredIndexByTag(tag); ok && !nonPks.GetByStoredIndex(idx).Virtual { + allMap[nonKeyIdx] = idx + allMap[numPhysicalColumns+nonKeyIdx] = projNum + nonKeyIdx-- } } - keyMap = allMap[:i] - valMap = allMap[i:numPhysicalColumns] + keyMap = allMap[:keyIdx] + valMap = allMap[keyIdx:numPhysicalColumns] ordMap = allMap[numPhysicalColumns:] if schema.IsKeyless(sch) { // skip the cardinality value, increment every index From f1279258ee1497264493a45f31b8db1e3fe7b93b Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Mon, 30 Oct 2023 17:11:33 -0700 Subject: [PATCH 09/26] Bug fix and eliminated redundancy for index iter mappings --- .../sqle/enginetest/dolt_engine_test.go | 47 +++++++------------ .../doltcore/sqle/index/prolly_index_iter.go | 34 +------------- .../doltcore/sqle/index/prolly_row_iter.go | 38 ++++++++------- 3 files changed, 39 insertions(+), 80 deletions(-) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go index 6761d23c09..7e72272790 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go @@ -120,28 +120,18 @@ func TestSingleScript(t *testing.T) { var scripts = []queries.ScriptTest{ { - Name: "virtual column ordering", + Name: "virtual column inserts, updates, deletes", SetUpScript: []string{ - // virtual is the default for generated columns - "create table t1 (v1 int generated always as (2), a int, v2 int generated always as (a + v1), c int)", + "create table t1 (a int primary key, b int generated always as (a + 1) virtual)", }, Assertions: []queries.ScriptTestAssertion{ { - Query: "insert into t1 (a, c) values (1,5), (3,7)", - Expected: []sql.Row{{gmstypes.NewOkResult(2)}}, + Query: "insert into t1 (a) values (1), (2), (3)", + Expected: []sql.Row{{gmstypes.NewOkResult(3)}}, }, { - Query: "insert into t1 (c, a) values (5,6), (7,8)", - Expected: []sql.Row{{gmstypes.NewOkResult(2)}}, - }, - { - Query: "select * from t1 order by a", - Expected: []sql.Row{ - {2, 1, 3, 5}, - {2, 3, 5, 7}, - {2, 6, 8, 5}, - {2, 8, 10, 7}, - }, + Query: "select * from t1 order by a", + Expected: []sql.Row{{1, 2}, {2, 3}, {3, 4}}, }, { Query: "update t1 set a = 4 where a = 3", @@ -154,25 +144,20 @@ func TestSingleScript(t *testing.T) { }}, }, { - Query: "select * from t1 order by a", - Expected: []sql.Row{ - {2, 1, 3, 5}, - {2, 4, 6, 7}, - {2, 6, 8, 5}, - {2, 8, 10, 7}, - }, + Query: "select * from t1 order by a", + Expected: []sql.Row{{1, 2}, {2, 3}, {4, 5}}, }, { - Query: "delete from t1 where v2 = 6", + Query: "delete from t1 where a = 2", Expected: []sql.Row{{gmstypes.OkResult{RowsAffected: 1}}}, }, { - Query: "select * from t1 order by a", - Expected: []sql.Row{ - {2, 1, 3, 5}, - {2, 6, 8, 5}, - {2, 8, 10, 7}, - }, + Query: "select * from t1 order by a", + Expected: []sql.Row{{1, 2}, {4, 5}}, + }, + { + Query: "update t1 set b = b + 1", + ExpectedErr: sql.ErrGeneratedColumnValue, }, }, }, @@ -531,7 +516,7 @@ func TestInsertIntoErrors(t *testing.T) { } func TestGeneratedColumns(t *testing.T) { - enginetest.TestGeneratedColumns(t, enginetest.NewDefaultMemoryHarness()) + enginetest.TestGeneratedColumns(t, newDoltHarness(t)) } func TestSpatialQueries(t *testing.T) { diff --git a/go/libraries/doltcore/sqle/index/prolly_index_iter.go b/go/libraries/doltcore/sqle/index/prolly_index_iter.go index 4551997d3e..85fc45e25c 100644 --- a/go/libraries/doltcore/sqle/index/prolly_index_iter.go +++ b/go/libraries/doltcore/sqle/index/prolly_index_iter.go @@ -17,7 +17,6 @@ package index import ( "context" "io" - "strings" "github.com/dolthub/go-mysql-server/sql" "golang.org/x/sync/errgroup" @@ -263,38 +262,7 @@ func coveringIndexMapping(d DoltIndex, projections []uint64) (keyMap, ordMap val } func primaryIndexMapping(idx DoltIndex, sqlSch sql.PrimaryKeySchema, projections []uint64) (keyProj, valProj, ordProj val.OrdinalMapping) { - pks := idx.Schema().GetPKCols() - nonPks := idx.Schema().GetNonPKCols() - - allMap := make([]int, len(projections)*2) - i := 0 - j := len(projections) - 1 - for k, p := range projections { - if idx, ok := pks.TagToIdx[p]; ok { - allMap[i] = idx - allMap[len(projections)+i] = k - i++ - } - - if idx, ok := nonPks.TagToIdx[p]; ok { - allMap[j] = idx - allMap[len(projections)+j] = k - j-- - } - } - keyProj = allMap[:i] - valProj = allMap[i:len(projections)] - ordProj = allMap[len(projections):] - return -} - -func contains(slice []string, str string) (ok bool) { - for _, x := range slice { - if strings.ToLower(x) == strings.ToLower(str) { - ok = true - } - } - return + return projectionMappingsForIndex(idx.Schema(), projections) } type prollyKeylessIndexIter struct { diff --git a/go/libraries/doltcore/sqle/index/prolly_row_iter.go b/go/libraries/doltcore/sqle/index/prolly_row_iter.go index 920571079b..58a20b8c46 100644 --- a/go/libraries/doltcore/sqle/index/prolly_row_iter.go +++ b/go/libraries/doltcore/sqle/index/prolly_row_iter.go @@ -73,11 +73,27 @@ func NewProllyRowIter(sch schema.Schema, rows prolly.Map, iter prolly.MapIter, p // projectionMappings returns data structures that specify 1) which fields we read // from key and value tuples, and 2) the position of those fields in the output row. func projectionMappings(sch schema.Schema, projections []uint64) (keyMap, valMap, ordMap val.OrdinalMapping) { + keyMap, valMap, ordMap = projectionMappingsForIndex(sch, projections) + adjustOffsetsForKeylessTable(sch, keyMap, valMap) + return keyMap, valMap, ordMap +} + +func adjustOffsetsForKeylessTable(sch schema.Schema, keyMap val.OrdinalMapping, valMap val.OrdinalMapping) { + if schema.IsKeyless(sch) { + // skip the cardinality value, increment every index + for i := range keyMap { + keyMap[i]++ + } + for i := range valMap { + valMap[i]++ + } + } +} + +func projectionMappingsForIndex(sch schema.Schema, projections []uint64) (keyMap, valMap, ordMap val.OrdinalMapping) { pks := sch.GetPKCols() nonPks := sch.GetNonPKCols() - - numPhysicalColumns := len(projections) if schema.IsVirtual(sch) { numPhysicalColumns = 0 @@ -87,11 +103,11 @@ func projectionMappings(sch schema.Schema, projections []uint64) (keyMap, valMap } } } - + // Build a slice of positional values. For a set of P projections, for K key columns and N=P-K non-key columns, // we'll generate a slice 2P long structured as follows: // [K key projections, // list of tuple indexes to read for key columns - // N non-key projections, // list of tuple indexes to read for non-key columns + // N non-key projections, // list of tuple indexes to read for non-key columns, ordered backward from end // P output ordinals] // list of output column ordinals for each projection // Afterward we slice this into three separate mappings to return. allMap := make([]int, 2*numPhysicalColumns) @@ -108,21 +124,11 @@ func projectionMappings(sch schema.Schema, projections []uint64) (keyMap, valMap nonKeyIdx-- } } - + keyMap = allMap[:keyIdx] valMap = allMap[keyIdx:numPhysicalColumns] ordMap = allMap[numPhysicalColumns:] - if schema.IsKeyless(sch) { - // skip the cardinality value, increment every index - for i := range keyMap { - keyMap[i]++ - } - for i := range valMap { - valMap[i]++ - } - } - - return + return keyMap, valMap, ordMap } func (it prollyRowIter) Next(ctx *sql.Context) (sql.Row, error) { From b49b640399ae6dcec324cbfc9f63594341f3910d Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 31 Oct 2023 09:55:15 -0700 Subject: [PATCH 10/26] Don't attempt to rebuild virtual indexes during engine validation --- .../sqle/enginetest/dolt_engine_test.go | 79 +++++++++++++------ .../doltcore/sqle/enginetest/validation.go | 30 ++++++- 2 files changed, 83 insertions(+), 26 deletions(-) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go index 7e72272790..892c8f07a3 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go @@ -120,44 +120,73 @@ func TestSingleScript(t *testing.T) { var scripts = []queries.ScriptTest{ { - Name: "virtual column inserts, updates, deletes", + Name: "virtual column index", SetUpScript: []string{ - "create table t1 (a int primary key, b int generated always as (a + 1) virtual)", + "create table t1 (a int primary key, b int, c int generated always as (a + b) virtual, index idx_c (c))", + "insert into t1 (a, b) values (1, 2), (3, 4)", }, Assertions: []queries.ScriptTestAssertion{ { - Query: "insert into t1 (a) values (1), (2), (3)", - Expected: []sql.Row{{gmstypes.NewOkResult(3)}}, + Query: "select * from t1 where c = 7", + Expected: []sql.Row{{3, 4, 7}}, }, { - Query: "select * from t1 order by a", - Expected: []sql.Row{{1, 2}, {2, 3}, {3, 4}}, + Query: "explain select * from t1 where c = 7", + Expected: []sql.Row{ + {"IndexedTableAccess(t1)"}, + {" ├─ index: [t1.c]"}, + {" └─ filters: [{[7, 7]}]"}, + }, }, { - Query: "update t1 set a = 4 where a = 3", - Expected: []sql.Row{{gmstypes.OkResult{ - RowsAffected: 1, - Info: plan.UpdateInfo{ - Matched: 1, - Updated: 1, - }}, - }}, + Query: "select * from t1 where c = 8", + Expected: []sql.Row{}, }, { - Query: "select * from t1 order by a", - Expected: []sql.Row{{1, 2}, {2, 3}, {4, 5}}, + Query: "explain update t1 set b = 5 where c = 3", + Expected: []sql.Row{ + {"Update"}, + {" └─ UpdateSource(SET t1.b = 5,SET t1.c = ((t1.a + t1.b)))"}, + {" └─ IndexedTableAccess(t1)"}, + {" ├─ index: [t1.c]"}, + {" └─ filters: [{[3, 3]}]"}, + }, }, { - Query: "delete from t1 where a = 2", - Expected: []sql.Row{{gmstypes.OkResult{RowsAffected: 1}}}, + Query: "update t1 set b = 5 where c = 3", + Expected: []sql.Row{{newUpdateResult(1, 1)}}, }, { - Query: "select * from t1 order by a", - Expected: []sql.Row{{1, 2}, {4, 5}}, + Query: "select * from t1 order by a", + Expected: []sql.Row{ + {1, 5, 6}, + {3, 4, 7}, + }, }, { - Query: "update t1 set b = b + 1", - ExpectedErr: sql.ErrGeneratedColumnValue, + Query: "select * from t1 where c = 6", + Expected: []sql.Row{ + {1, 5, 6}, + }, + }, + { + Query: "explain delete from t1 where c = 6", + Expected: []sql.Row{ + {"Delete"}, + {" └─ IndexedTableAccess(t1)"}, + {" ├─ index: [t1.c]"}, + {" └─ filters: [{[6, 6]}]"}, + }, + }, + { + Query: "delete from t1 where c = 6", + Expected: []sql.Row{{gmstypes.NewOkResult(1)}}, + }, + { + Query: "select * from t1 order by a", + Expected: []sql.Row{ + {3, 4, 7}, + }, }, }, }, @@ -185,6 +214,12 @@ func TestSingleScript(t *testing.T) { } } +func newUpdateResult(matched, updated int) gmstypes.OkResult { + return gmstypes.OkResult{ + RowsAffected: uint64(updated), + Info: plan.UpdateInfo{Matched: matched, Updated: updated}, + } +} // Convenience test for debugging a single query. Unskip and set to the desired query. func TestSingleMergeScript(t *testing.T) { t.Skip() diff --git a/go/libraries/doltcore/sqle/enginetest/validation.go b/go/libraries/doltcore/sqle/enginetest/validation.go index 929e9ed0d1..a47b76865d 100644 --- a/go/libraries/doltcore/sqle/enginetest/validation.go +++ b/go/libraries/doltcore/sqle/enginetest/validation.go @@ -19,10 +19,6 @@ import ( "fmt" "io" - "github.com/dolthub/go-mysql-server/sql" - "github.com/dolthub/go-mysql-server/sql/mysql_db" - sqltypes "github.com/dolthub/go-mysql-server/sql/types" - "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb/durable" "github.com/dolthub/dolt/go/libraries/doltcore/ref" @@ -33,6 +29,9 @@ import ( "github.com/dolthub/dolt/go/store/prolly/tree" "github.com/dolthub/dolt/go/store/types" "github.com/dolthub/dolt/go/store/val" + "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/go-mysql-server/sql/mysql_db" + sqltypes "github.com/dolthub/go-mysql-server/sql/types" ) func ValidateDatabase(ctx context.Context, db sql.Database) (err error) { @@ -170,6 +169,11 @@ func validateKeylessIndex(ctx context.Context, sch schema.Schema, def schema.Ind return nil } + // Indexes on virtual columns cannot be rebuilt via the method below + if isVirtualIndex(def, sch) { + return nil + } + secondary = prolly.ConvertToSecondaryKeylessIndex(secondary) idxDesc, _ := secondary.Descriptors() builder := val.NewTupleBuilder(idxDesc) @@ -236,6 +240,11 @@ func validatePkIndex(ctx context.Context, sch schema.Schema, def schema.Index, p if def.IsFullText() { return nil } + + // Indexes on virtual columns cannot be rebuilt via the method below + if isVirtualIndex(def, sch) { + return nil + } // secondary indexes have empty values idxDesc, _ := secondary.Descriptors() @@ -317,6 +326,19 @@ func validatePkIndex(ctx context.Context, sch schema.Schema, def schema.Index, p } } +func isVirtualIndex(def schema.Index, sch schema.Schema) bool { + for _, colName := range def.ColumnNames() { + col, ok := sch.GetAllCols().GetByName(colName) + if !ok { + panic(fmt.Sprintf("column not found: %s", colName)) + } + if col.Virtual { + return true + } + } + return false +} + // shouldDereferenceContent returns true if address encoded content should be dereferenced when // building a key for a secondary index. This is determined by looking at the encoding of the field // in the main table (|tablePos| and |tableValueDescriptor|) and the encoding of the field in the index From ca07a9274680b08bf7e9c822959bd4404d041a42 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 31 Oct 2023 11:17:24 -0700 Subject: [PATCH 11/26] New engine tests --- .../sqle/enginetest/dolt_engine_test.go | 28 ++++++---- .../sqle/enginetest/dolt_queries_merge.go | 54 +++++++++++++++++++ 2 files changed, 72 insertions(+), 10 deletions(-) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go index 892c8f07a3..53130c839c 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go @@ -390,7 +390,7 @@ func TestQueryPlans(t *testing.T) { } // Parallelism introduces Exchange nodes into the query plans, so disable. // TODO: exchange nodes should really only be part of the explain plan under certain debug settings - harness := newDoltHarness(t).WithParallelism(1).WithSkippedQueries(skipped) + harness := newDoltHarness(t).WithSkippedQueries(skipped) if !types.IsFormat_DOLT(types.Format_Default) { // only new format supports reverse IndexTableAccess reverseIndexSkip := []string{ @@ -415,7 +415,7 @@ func TestQueryPlans(t *testing.T) { } func TestIntegrationQueryPlans(t *testing.T) { - harness := newDoltHarness(t).WithParallelism(1) + harness := newDoltHarness(t) defer harness.Close() enginetest.TestIntegrationPlans(t, harness) @@ -441,7 +441,7 @@ func TestDoltDiffQueryPlans(t *testing.T) { func TestBranchPlans(t *testing.T) { for _, script := range BranchPlanTests { t.Run(script.Name, func(t *testing.T) { - harness := newDoltHarness(t).WithParallelism(1) + harness := newDoltHarness(t) defer harness.Close() e := mustNewEngine(t, harness) @@ -552,6 +552,14 @@ func TestInsertIntoErrors(t *testing.T) { func TestGeneratedColumns(t *testing.T) { enginetest.TestGeneratedColumns(t, newDoltHarness(t)) + + for _, script := range GeneratedColumnMergeTestScripts { + func() { + h := newDoltHarness(t) + defer h.Close() + enginetest.TestScript(t, h, script) + }() + } } func TestSpatialQueries(t *testing.T) { @@ -662,7 +670,7 @@ func TestScripts(t *testing.T) { if types.IsFormat_DOLT(types.Format_Default) { skipped = append(skipped, newFormatSkippedScripts...) } - h := newDoltHarness(t).WithSkippedQueries(skipped).WithParallelism(1) + h := newDoltHarness(t).WithSkippedQueries(skipped) defer h.Close() enginetest.TestScripts(t, h) } @@ -746,7 +754,7 @@ func TestJoinPlanning(t *testing.T) { if types.IsFormat_LD(types.Format_Default) { t.Skip("DOLT_LD keyless indexes are not sorted") } - h := newDoltHarness(t).WithParallelism(1) + h := newDoltHarness(t) defer h.Close() enginetest.TestJoinPlanning(t, h) } @@ -1592,7 +1600,7 @@ func TestDoltMerge(t *testing.T) { // harness can't reset effectively when there are new commits / branches created, so use a new harness for // each script func() { - h := newDoltHarness(t).WithParallelism(1) + h := newDoltHarness(t) defer h.Close() h.Setup(setup.MydbData) enginetest.TestScript(t, h, script) @@ -1605,7 +1613,7 @@ func TestDoltMergePrepared(t *testing.T) { // harness can't reset effectively when there are new commits / branches created, so use a new harness for // each script func() { - h := newDoltHarness(t).WithParallelism(1) + h := newDoltHarness(t) defer h.Close() enginetest.TestScriptPrepared(t, h, script) }() @@ -1616,7 +1624,7 @@ func TestDoltRevert(t *testing.T) { for _, script := range RevertScripts { // harness can't reset effectively. Use a new harness for each script func() { - h := newDoltHarness(t).WithParallelism(1) + h := newDoltHarness(t) defer h.Close() enginetest.TestScript(t, h, script) }() @@ -1627,7 +1635,7 @@ func TestDoltRevertPrepared(t *testing.T) { for _, script := range RevertScripts { // harness can't reset effectively. Use a new harness for each script func() { - h := newDoltHarness(t).WithParallelism(1) + h := newDoltHarness(t) defer h.Close() enginetest.TestScriptPrepared(t, h, script) }() @@ -2501,7 +2509,7 @@ func TestScriptsPrepared(t *testing.T) { skipped = append(skipped, newFormatSkippedScripts...) } skipPreparedTests(t) - h := newDoltHarness(t).WithSkippedQueries(skipped).WithParallelism(1) + h := newDoltHarness(t).WithSkippedQueries(skipped) defer h.Close() enginetest.TestScriptsPrepared(t, h) } diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go index 540324c38c..61d27e1881 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go @@ -4273,6 +4273,60 @@ var DoltVerifyConstraintsTestScripts = []queries.ScriptTest{ }, } +var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ + { + Name: "merge a generated stored column", + SetUpScript: []string{ + "create table t1 (id bigint primary key, v1 bigint, v2 bigint, v3 bigint as (v1 + v2) stored, index (v3))", + "insert into t1 (id, v1, v2) values (1, 1, 1), (2, 2, 2)", + "call dolt_commit('-Am', 'first commit')", + "call dolt_checkout('-b', 'branch1')", + "insert into t1 (id, v1, v2) values (3, 3, 3)", + "call dolt_commit('-Am', 'branch1 commit')", + "call dolt_checkout('main')", + "call dolt_checkout('-b', 'branch2')", + "insert into t1 (id, v1, v2) values (4, 4, 4)", + "call dolt_commit('-Am', 'branch2 commit')", + "call dolt_checkout('main')", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "call dolt_merge('branch1')", + SkipResultsCheck: true, + }, + { + Query: "select * from t1 order by id", + Expected: []sql.Row{ + {1, 1, 1, 2}, + {2, 2, 2, 4}, + {3, 3, 3, 6}, + }, + }, + { + Query: "select id from t1 where v3 = 6", + Expected: []sql.Row{{3}}, + }, + { + Query: "call dolt_merge('branch2')", + SkipResultsCheck: true, + }, + { + Query: "select * from t1 order by id", + Expected: []sql.Row{ + {1, 1, 1, 2}, + {2, 2, 2, 4}, + {3, 3, 3, 6}, + {4, 4, 4, 8}, + }, + }, + { + Query: "select id from t1 where v3 = 8", + Expected: []sql.Row{{4}}, + }, + }, + }, +} + // convertMergeScriptTest converts a MergeScriptTest into a standard ScriptTest. If flipSides is true, then the // left and right setup is swapped (i.e. left setup is done on right branch and right setup is done on main branch). // This enables us to test merges in both directions, since the merge code is asymmetric and some code paths currently From 3966913576af6b99d52ce79f47b2fc336e43bedc Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 31 Oct 2023 11:22:50 -0700 Subject: [PATCH 12/26] More tests --- .../sqle/enginetest/dolt_queries_merge.go | 155 ++++++++++++++++++ 1 file changed, 155 insertions(+) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go index 61d27e1881..bf06948220 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go @@ -4325,6 +4325,161 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ }, }, }, + { + Name: "merge a generated column created on another branch", + SetUpScript: []string{ + "create table t1 (id bigint primary key, v1 bigint, v2 bigint)", + "insert into t1 (id, v1, v2) values (1, 1, 1), (2, 2, 2)", + "call dolt_commit('-Am', 'first commit')", + "call dolt_branch('branch1')", + "insert into t1 (id, v1, v2) values (3, 3, 3)", + "call dolt_commit('-Am', 'main commit')", + "call dolt_checkout('branch1')", + "alter table t1 add column v3 bigint as (v1 + v2) stored", + "alter table t1 add key idx_v3 (v3)", + "insert into t1 (id, v1, v2) values (4, 4, 4)", + "call dolt_commit('-Am', 'branch1 commit')", + "call dolt_checkout('main')", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "call dolt_merge('branch1')", + SkipResultsCheck: true, + }, + { + Query: "select * from t1 order by id", + Expected: []sql.Row{ + {1, 1, 1, 2}, + {2, 2, 2, 4}, + {3, 3, 3, 6}, + }, + }, + { + Query: "select id from t1 where v3 = 6", + Expected: []sql.Row{{3}}, + }, + { + Query: "call dolt_merge('branch2')", + SkipResultsCheck: true, + }, + { + Query: "select * from t1 order by id", + Expected: []sql.Row{ + {1, 1, 1, 2}, + {2, 2, 2, 4}, + {3, 3, 3, 6}, + {4, 4, 4, 8}, + }, + }, + { + Query: "select id from t1 where v3 = 8", + Expected: []sql.Row{{4}}, + }, + }, + }, + { + Name: "merge a virtual column", + SetUpScript: []string{ + "create table t1 (id bigint primary key, v1 bigint, v2 bigint, v3 bigint as (v1 + v2), index (v3))", + "insert into t1 (id, v1, v2) values (1, 1, 1), (2, 2, 2)", + "call dolt_commit('-Am', 'first commit')", + "call dolt_checkout('-b', 'branch1')", + "insert into t1 (id, v1, v2) values (3, 3, 3)", + "call dolt_commit('-Am', 'branch1 commit')", + "call dolt_checkout('main')", + "call dolt_checkout('-b', 'branch2')", + "insert into t1 (id, v1, v2) values (4, 4, 4)", + "call dolt_commit('-Am', 'branch2 commit')", + "call dolt_checkout('main')", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "call dolt_merge('branch1')", + SkipResultsCheck: true, + }, + { + Query: "select * from t1 order by id", + Expected: []sql.Row{ + {1, 1, 1, 2}, + {2, 2, 2, 4}, + {3, 3, 3, 6}, + }, + }, + { + Query: "select id from t1 where v3 = 6", + Expected: []sql.Row{{3}}, + }, + { + Query: "call dolt_merge('branch2')", + SkipResultsCheck: true, + }, + { + Query: "select * from t1 order by id", + Expected: []sql.Row{ + {1, 1, 1, 2}, + {2, 2, 2, 4}, + {3, 3, 3, 6}, + {4, 4, 4, 8}, + }, + }, + { + Query: "select id from t1 where v3 = 8", + Expected: []sql.Row{{4}}, + }, + }, + }, + { + Name: "merge a virtual column created on another branch", + SetUpScript: []string{ + "create table t1 (id bigint primary key, v1 bigint, v2 bigint)", + "insert into t1 (id, v1, v2) values (1, 1, 1), (2, 2, 2)", + "call dolt_commit('-Am', 'first commit')", + "call dolt_branch('branch1')", + "insert into t1 (id, v1, v2) values (3, 3, 3)", + "call dolt_commit('-Am', 'main commit')", + "call dolt_checkout('branch1')", + "alter table t1 add column v3 bigint as (v1 + v2)", + "alter table t1 add key idx_v3 (v3)", + "insert into t1 (id, v1, v2) values (4, 4, 4)", + "call dolt_commit('-Am', 'branch1 commit')", + "call dolt_checkout('main')", + }, + Assertions: []queries.ScriptTestAssertion{ + { + Query: "call dolt_merge('branch1')", + SkipResultsCheck: true, + }, + { + Query: "select * from t1 order by id", + Expected: []sql.Row{ + {1, 1, 1, 2}, + {2, 2, 2, 4}, + {3, 3, 3, 6}, + }, + }, + { + Query: "select id from t1 where v3 = 6", + Expected: []sql.Row{{3}}, + }, + { + Query: "call dolt_merge('branch2')", + SkipResultsCheck: true, + }, + { + Query: "select * from t1 order by id", + Expected: []sql.Row{ + {1, 1, 1, 2}, + {2, 2, 2, 4}, + {3, 3, 3, 6}, + {4, 4, 4, 8}, + }, + }, + { + Query: "select id from t1 where v3 = 8", + Expected: []sql.Row{{4}}, + }, + }, + }, } // convertMergeScriptTest converts a MergeScriptTest into a standard ScriptTest. If flipSides is true, then the From fe42f6d21d7d1094794aa56bc0793ebf7359c645 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Tue, 31 Oct 2023 16:06:30 -0700 Subject: [PATCH 13/26] Skip failing merge tests --- .../sqle/enginetest/dolt_queries_merge.go | 28 ++++++++++--------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go index bf06948220..a386009850 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go @@ -4352,24 +4352,14 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ {1, 1, 1, 2}, {2, 2, 2, 4}, {3, 3, 3, 6}, + {4, 4, 4, 8}, }, + Skip: true, }, { Query: "select id from t1 where v3 = 6", Expected: []sql.Row{{3}}, - }, - { - Query: "call dolt_merge('branch2')", - SkipResultsCheck: true, - }, - { - Query: "select * from t1 order by id", - Expected: []sql.Row{ - {1, 1, 1, 2}, - {2, 2, 2, 4}, - {3, 3, 3, 6}, - {4, 4, 4, 8}, - }, + Skip: true, }, { Query: "select id from t1 where v3 = 8", @@ -4396,6 +4386,7 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ { Query: "call dolt_merge('branch1')", SkipResultsCheck: true, + Skip: true, }, { Query: "select * from t1 order by id", @@ -4404,14 +4395,17 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ {2, 2, 2, 4}, {3, 3, 3, 6}, }, + Skip: true, }, { Query: "select id from t1 where v3 = 6", Expected: []sql.Row{{3}}, + Skip: true, }, { Query: "call dolt_merge('branch2')", SkipResultsCheck: true, + Skip: true, }, { Query: "select * from t1 order by id", @@ -4421,10 +4415,12 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ {3, 3, 3, 6}, {4, 4, 4, 8}, }, + Skip: true, }, { Query: "select id from t1 where v3 = 8", Expected: []sql.Row{{4}}, + Skip: true, }, }, }, @@ -4448,6 +4444,7 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ { Query: "call dolt_merge('branch1')", SkipResultsCheck: true, + Skip: true, }, { Query: "select * from t1 order by id", @@ -4456,14 +4453,17 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ {2, 2, 2, 4}, {3, 3, 3, 6}, }, + Skip: true, }, { Query: "select id from t1 where v3 = 6", Expected: []sql.Row{{3}}, + Skip: true, }, { Query: "call dolt_merge('branch2')", SkipResultsCheck: true, + Skip: true, }, { Query: "select * from t1 order by id", @@ -4473,10 +4473,12 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ {3, 3, 3, 6}, {4, 4, 4, 8}, }, + Skip: true, }, { Query: "select id from t1 where v3 = 8", Expected: []sql.Row{{4}}, + Skip: true, }, }, }, From 8e042db048145419d16b99b1c2327ebf17c4a3cb Mon Sep 17 00:00:00 2001 From: zachmu Date: Tue, 31 Oct 2023 23:19:51 +0000 Subject: [PATCH 14/26] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/libraries/doltcore/schema/col_coll.go | 2 +- go/libraries/doltcore/schema/column.go | 22 +++---- .../doltcore/schema/encoding/serialization.go | 4 +- go/libraries/doltcore/schema/schema_impl.go | 6 +- .../sqle/enginetest/dolt_engine_test.go | 3 +- .../doltcore/sqle/enginetest/dolt_harness.go | 2 +- .../sqle/enginetest/dolt_queries_merge.go | 62 +++++++++---------- .../doltcore/sqle/enginetest/validation.go | 9 +-- .../doltcore/sqle/index/prolly_row_iter.go | 2 +- go/libraries/doltcore/sqle/sqlutil/convert.go | 6 +- .../writer/prolly_index_writer_keyless.go | 6 +- .../sqle/writer/prolly_table_writer.go | 2 +- 12 files changed, 64 insertions(+), 62 deletions(-) diff --git a/go/libraries/doltcore/schema/col_coll.go b/go/libraries/doltcore/schema/col_coll.go index c0fdd5c879..6e9b177523 100644 --- a/go/libraries/doltcore/schema/col_coll.go +++ b/go/libraries/doltcore/schema/col_coll.go @@ -106,7 +106,7 @@ func NewColCollection(cols ...Column) *ColCollection { if _, ok := lowerNameToCol[lowerCaseName]; !ok { lowerNameToCol[lowerCaseName] = cols[i] } - + if col.Virtual { virtualColumns = append(virtualColumns, i) } else { diff --git a/go/libraries/doltcore/schema/column.go b/go/libraries/doltcore/schema/column.go index 7ba0a48e3e..9012b795b4 100644 --- a/go/libraries/doltcore/schema/column.go +++ b/go/libraries/doltcore/schema/column.go @@ -70,10 +70,10 @@ type Column struct { // Default is the default value of this column. This is the string representation of a sql.Expression. Default string - + // Generated is the generated value of this column. This is the string representation of a sql.Expression. Generated string - + // Virtual is true if this is a virtual column. Virtual bool @@ -99,14 +99,14 @@ func NewColumn(name string, tag uint64, kind types.NomsKind, partOfPK bool, cons // NewColumnWithTypeInfo creates a Column instance with the given type info. func NewColumnWithTypeInfo( - name string, - tag uint64, - typeInfo typeinfo.TypeInfo, - partOfPK bool, - defaultVal, generatedVal string, - virtual, autoIncrement bool, - comment string, - constraints ...ColConstraint, + name string, + tag uint64, + typeInfo typeinfo.TypeInfo, + partOfPK bool, + defaultVal, generatedVal string, + virtual, autoIncrement bool, + comment string, + constraints ...ColConstraint, ) (Column, error) { for _, c := range constraints { if c == nil { @@ -126,7 +126,7 @@ func NewColumnWithTypeInfo( TypeInfo: typeInfo, Default: defaultVal, Generated: generatedVal, - Virtual: virtual, + Virtual: virtual, AutoIncrement: autoIncrement, Comment: comment, Constraints: constraints, diff --git a/go/libraries/doltcore/schema/encoding/serialization.go b/go/libraries/doltcore/schema/encoding/serialization.go index d0c5b0a9bb..be473d4fee 100644 --- a/go/libraries/doltcore/schema/encoding/serialization.go +++ b/go/libraries/doltcore/schema/encoding/serialization.go @@ -216,7 +216,7 @@ func serializeSchemaColumns(b *fb.Builder, sch schema.Schema) fb.UOffsetT { defVal = col.Generated } do := b.CreateString(defVal) - + serial.ColumnStart(b) serial.ColumnAddName(b, no) serial.ColumnAddSqlType(b, to) @@ -310,7 +310,7 @@ func deserializeColumns(ctx context.Context, s *serial.TableSchema) ([]schema.Co defVal = string(c.DefaultValue()) } } - + cols[i], err = schema.NewColumnWithTypeInfo( string(c.Name()), c.Tag(), diff --git a/go/libraries/doltcore/schema/schema_impl.go b/go/libraries/doltcore/schema/schema_impl.go index f3eabfc859..74ec2dc563 100644 --- a/go/libraries/doltcore/schema/schema_impl.go +++ b/go/libraries/doltcore/schema/schema_impl.go @@ -419,8 +419,8 @@ func (si *schemaImpl) GetKeyDescriptor() val.TupleDesc { _ = si.GetPKCols().Iter(func(tag uint64, col Column) (stop bool, err error) { if col.Virtual { return - } - + } + sqlType := col.TypeInfo.ToSqlType() queryType := sqlType.Type() var t val.Type @@ -480,7 +480,7 @@ func (si *schemaImpl) GetValueDescriptor() val.TupleDesc { if col.Virtual { return } - + sqlType := col.TypeInfo.ToSqlType() queryType := sqlType.Type() tt = append(tt, val.Type{ diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go index 53130c839c..968a225a86 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go @@ -220,6 +220,7 @@ func newUpdateResult(matched, updated int) gmstypes.OkResult { Info: plan.UpdateInfo{Matched: matched, Updated: updated}, } } + // Convenience test for debugging a single query. Unskip and set to the desired query. func TestSingleMergeScript(t *testing.T) { t.Skip() @@ -552,7 +553,7 @@ func TestInsertIntoErrors(t *testing.T) { func TestGeneratedColumns(t *testing.T) { enginetest.TestGeneratedColumns(t, newDoltHarness(t)) - + for _, script := range GeneratedColumnMergeTestScripts { func() { h := newDoltHarness(t) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_harness.go b/go/libraries/doltcore/sqle/enginetest/dolt_harness.go index 0d391ea91f..26d43f3548 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_harness.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_harness.go @@ -69,7 +69,7 @@ func newDoltHarness(t *testing.T) *DoltHarness { dh := &DoltHarness{ t: t, skippedQueries: defaultSkippedQueries, - parallelism: 1, + parallelism: 1, } return dh diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go index a386009850..283e699ea9 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_queries_merge.go @@ -4291,11 +4291,11 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ }, Assertions: []queries.ScriptTestAssertion{ { - Query: "call dolt_merge('branch1')", + Query: "call dolt_merge('branch1')", SkipResultsCheck: true, }, { - Query: "select * from t1 order by id", + Query: "select * from t1 order by id", Expected: []sql.Row{ {1, 1, 1, 2}, {2, 2, 2, 4}, @@ -4303,15 +4303,15 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ }, }, { - Query: "select id from t1 where v3 = 6", + Query: "select id from t1 where v3 = 6", Expected: []sql.Row{{3}}, }, { - Query: "call dolt_merge('branch2')", + Query: "call dolt_merge('branch2')", SkipResultsCheck: true, }, { - Query: "select * from t1 order by id", + Query: "select * from t1 order by id", Expected: []sql.Row{ {1, 1, 1, 2}, {2, 2, 2, 4}, @@ -4320,7 +4320,7 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ }, }, { - Query: "select id from t1 where v3 = 8", + Query: "select id from t1 where v3 = 8", Expected: []sql.Row{{4}}, }, }, @@ -4343,11 +4343,11 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ }, Assertions: []queries.ScriptTestAssertion{ { - Query: "call dolt_merge('branch1')", + Query: "call dolt_merge('branch1')", SkipResultsCheck: true, }, { - Query: "select * from t1 order by id", + Query: "select * from t1 order by id", Expected: []sql.Row{ {1, 1, 1, 2}, {2, 2, 2, 4}, @@ -4357,12 +4357,12 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ Skip: true, }, { - Query: "select id from t1 where v3 = 6", + Query: "select id from t1 where v3 = 6", Expected: []sql.Row{{3}}, - Skip: true, + Skip: true, }, { - Query: "select id from t1 where v3 = 8", + Query: "select id from t1 where v3 = 8", Expected: []sql.Row{{4}}, }, }, @@ -4384,12 +4384,12 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ }, Assertions: []queries.ScriptTestAssertion{ { - Query: "call dolt_merge('branch1')", + Query: "call dolt_merge('branch1')", SkipResultsCheck: true, - Skip: true, + Skip: true, }, { - Query: "select * from t1 order by id", + Query: "select * from t1 order by id", Expected: []sql.Row{ {1, 1, 1, 2}, {2, 2, 2, 4}, @@ -4398,17 +4398,17 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ Skip: true, }, { - Query: "select id from t1 where v3 = 6", + Query: "select id from t1 where v3 = 6", Expected: []sql.Row{{3}}, - Skip: true, + Skip: true, }, { - Query: "call dolt_merge('branch2')", + Query: "call dolt_merge('branch2')", SkipResultsCheck: true, - Skip: true, + Skip: true, }, { - Query: "select * from t1 order by id", + Query: "select * from t1 order by id", Expected: []sql.Row{ {1, 1, 1, 2}, {2, 2, 2, 4}, @@ -4418,9 +4418,9 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ Skip: true, }, { - Query: "select id from t1 where v3 = 8", + Query: "select id from t1 where v3 = 8", Expected: []sql.Row{{4}}, - Skip: true, + Skip: true, }, }, }, @@ -4442,12 +4442,12 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ }, Assertions: []queries.ScriptTestAssertion{ { - Query: "call dolt_merge('branch1')", + Query: "call dolt_merge('branch1')", SkipResultsCheck: true, - Skip: true, + Skip: true, }, { - Query: "select * from t1 order by id", + Query: "select * from t1 order by id", Expected: []sql.Row{ {1, 1, 1, 2}, {2, 2, 2, 4}, @@ -4456,17 +4456,17 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ Skip: true, }, { - Query: "select id from t1 where v3 = 6", + Query: "select id from t1 where v3 = 6", Expected: []sql.Row{{3}}, - Skip: true, + Skip: true, }, { - Query: "call dolt_merge('branch2')", + Query: "call dolt_merge('branch2')", SkipResultsCheck: true, - Skip: true, + Skip: true, }, { - Query: "select * from t1 order by id", + Query: "select * from t1 order by id", Expected: []sql.Row{ {1, 1, 1, 2}, {2, 2, 2, 4}, @@ -4476,9 +4476,9 @@ var GeneratedColumnMergeTestScripts = []queries.ScriptTest{ Skip: true, }, { - Query: "select id from t1 where v3 = 8", + Query: "select id from t1 where v3 = 8", Expected: []sql.Row{{4}}, - Skip: true, + Skip: true, }, }, }, diff --git a/go/libraries/doltcore/sqle/enginetest/validation.go b/go/libraries/doltcore/sqle/enginetest/validation.go index a47b76865d..62ca18aad4 100644 --- a/go/libraries/doltcore/sqle/enginetest/validation.go +++ b/go/libraries/doltcore/sqle/enginetest/validation.go @@ -19,6 +19,10 @@ import ( "fmt" "io" + "github.com/dolthub/go-mysql-server/sql" + "github.com/dolthub/go-mysql-server/sql/mysql_db" + sqltypes "github.com/dolthub/go-mysql-server/sql/types" + "github.com/dolthub/dolt/go/libraries/doltcore/doltdb" "github.com/dolthub/dolt/go/libraries/doltcore/doltdb/durable" "github.com/dolthub/dolt/go/libraries/doltcore/ref" @@ -29,9 +33,6 @@ import ( "github.com/dolthub/dolt/go/store/prolly/tree" "github.com/dolthub/dolt/go/store/types" "github.com/dolthub/dolt/go/store/val" - "github.com/dolthub/go-mysql-server/sql" - "github.com/dolthub/go-mysql-server/sql/mysql_db" - sqltypes "github.com/dolthub/go-mysql-server/sql/types" ) func ValidateDatabase(ctx context.Context, db sql.Database) (err error) { @@ -240,7 +241,7 @@ func validatePkIndex(ctx context.Context, sch schema.Schema, def schema.Index, p if def.IsFullText() { return nil } - + // Indexes on virtual columns cannot be rebuilt via the method below if isVirtualIndex(def, sch) { return nil diff --git a/go/libraries/doltcore/sqle/index/prolly_row_iter.go b/go/libraries/doltcore/sqle/index/prolly_row_iter.go index 58a20b8c46..b3aa20e769 100644 --- a/go/libraries/doltcore/sqle/index/prolly_row_iter.go +++ b/go/libraries/doltcore/sqle/index/prolly_row_iter.go @@ -104,7 +104,7 @@ func projectionMappingsForIndex(sch schema.Schema, projections []uint64) (keyMap } } - // Build a slice of positional values. For a set of P projections, for K key columns and N=P-K non-key columns, + // Build a slice of positional values. For a set of P projections, for K key columns and N=P-K non-key columns, // we'll generate a slice 2P long structured as follows: // [K key projections, // list of tuple indexes to read for key columns // N non-key projections, // list of tuple indexes to read for non-key columns, ordered backward from end diff --git a/go/libraries/doltcore/sqle/sqlutil/convert.go b/go/libraries/doltcore/sqle/sqlutil/convert.go index 1bb2cba36c..e271b04db7 100644 --- a/go/libraries/doltcore/sqle/sqlutil/convert.go +++ b/go/libraries/doltcore/sqle/sqlutil/convert.go @@ -47,7 +47,7 @@ func FromDoltSchema(dbName, tableName string, sch schema.Schema) (sql.PrimaryKey if col.Generated != "" { generated = sql.NewUnresolvedColumnDefaultValue(col.Generated) } - + cols[i] = &sql.Column{ Name: col.Name, Type: sqlType, @@ -59,7 +59,7 @@ func FromDoltSchema(dbName, tableName string, sch schema.Schema) (sql.PrimaryKey PrimaryKey: col.IsPartOfPK, AutoIncrement: col.AutoIncrement, Comment: col.Comment, - Virtual: col.Virtual, + Virtual: col.Virtual, Extra: extra, } i++ @@ -149,7 +149,7 @@ func ToDoltCol(tag uint64, col *sql.Column) (schema.Column, error) { } else { generatedVal = col.Generated.String() } - + return schema.NewColumnWithTypeInfo( col.Name, tag, diff --git a/go/libraries/doltcore/sqle/writer/prolly_index_writer_keyless.go b/go/libraries/doltcore/sqle/writer/prolly_index_writer_keyless.go index ebdb52fc2d..0bc0fbd905 100644 --- a/go/libraries/doltcore/sqle/writer/prolly_index_writer_keyless.go +++ b/go/libraries/doltcore/sqle/writer/prolly_index_writer_keyless.go @@ -29,9 +29,9 @@ type prollyKeylessWriter struct { name string mut *prolly.MutableMap - keyBld *val.TupleBuilder - valBld *val.TupleBuilder - valMap val.OrdinalMapping + keyBld *val.TupleBuilder + valBld *val.TupleBuilder + valMap val.OrdinalMapping virtualCols []int } diff --git a/go/libraries/doltcore/sqle/writer/prolly_table_writer.go b/go/libraries/doltcore/sqle/writer/prolly_table_writer.go index 39d06486ab..7deee6f345 100644 --- a/go/libraries/doltcore/sqle/writer/prolly_table_writer.go +++ b/go/libraries/doltcore/sqle/writer/prolly_table_writer.go @@ -382,7 +382,7 @@ func makeOrdinalMapping(from sql.Schema, to *schema.ColCollection) (m val.Ordina col := to.GetByStoredIndex(i) name := col.Name colIdx := from.IndexOfColName(name) - m[i] = colIdx + m[i] = colIdx } return } From 2598165fd97434940b9f48e1d32a7d5f124afd14 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 1 Nov 2023 12:48:40 -0700 Subject: [PATCH 15/26] PR feedback --- go/libraries/doltcore/schema/col_coll.go | 8 +------- go/libraries/doltcore/schema/encoding/serialization.go | 10 +++++----- go/libraries/doltcore/schema/schema_impl.go | 4 ---- .../doltcore/sqle/enginetest/dolt_engine_test.go | 4 ++++ 4 files changed, 10 insertions(+), 16 deletions(-) diff --git a/go/libraries/doltcore/schema/col_coll.go b/go/libraries/doltcore/schema/col_coll.go index c0fdd5c879..75fb80738b 100644 --- a/go/libraries/doltcore/schema/col_coll.go +++ b/go/libraries/doltcore/schema/col_coll.go @@ -264,13 +264,7 @@ func (cc *ColCollection) Size() int { // StoredSize returns the number of non-virtual columns in the collection func (cc *ColCollection) StoredSize() int { - num := 0 - for _, col := range cc.cols { - if !col.Virtual { - num++ - } - } - return num + return len(cc.storedIndexes) } // Contains returns whether this column collection contains a column with the name given, case insensitive diff --git a/go/libraries/doltcore/schema/encoding/serialization.go b/go/libraries/doltcore/schema/encoding/serialization.go index d0c5b0a9bb..f7e3ebd35d 100644 --- a/go/libraries/doltcore/schema/encoding/serialization.go +++ b/go/libraries/doltcore/schema/encoding/serialization.go @@ -204,18 +204,18 @@ func serializeSchemaColumns(b *fb.Builder, sch schema.Schema) fb.UOffsetT { // serialize columns in |cols| for i := len(cols) - 1; i >= 0; i-- { col := cols[i] - co := b.CreateString(col.Comment) - typeString := sqlTypeString(col.TypeInfo) - to := b.CreateString(typeString) - no := b.CreateString(col.Name) - defVal := "" if col.Default != "" { defVal = col.Default } else { defVal = col.Generated } + + co := b.CreateString(col.Comment) do := b.CreateString(defVal) + typeString := sqlTypeString(col.TypeInfo) + to := b.CreateString(typeString) + no := b.CreateString(col.Name) serial.ColumnStart(b) serial.ColumnAddName(b, no) diff --git a/go/libraries/doltcore/schema/schema_impl.go b/go/libraries/doltcore/schema/schema_impl.go index f3eabfc859..7d86a93a2b 100644 --- a/go/libraries/doltcore/schema/schema_impl.go +++ b/go/libraries/doltcore/schema/schema_impl.go @@ -417,10 +417,6 @@ func (si *schemaImpl) GetKeyDescriptor() val.TupleDesc { useCollations := false // We only use collations if a string exists var collations []sql.CollationID _ = si.GetPKCols().Iter(func(tag uint64, col Column) (stop bool, err error) { - if col.Virtual { - return - } - sqlType := col.TypeInfo.ToSqlType() queryType := sqlType.Type() var t val.Type diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go index 53130c839c..e2147490e9 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go @@ -562,6 +562,10 @@ func TestGeneratedColumns(t *testing.T) { } } +func TestGeneratedColumnPlans(t *testing.T) { + enginetest.TestGeneratedColumnPlans(t, newDoltHarness(t)) +} + func TestSpatialQueries(t *testing.T) { h := newDoltHarness(t) defer h.Close() From f5d8a54b3844af6006c22aabe6cfa5557362cb50 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 1 Nov 2023 15:51:24 -0700 Subject: [PATCH 16/26] Reverted param changes to NewColumnWithTypeInfo --- go/cmd/dolt/commands/diff.go | 14 ++++- go/libraries/doltcore/doltdb/system_table.go | 2 +- go/libraries/doltcore/doltdb/table.go | 4 +- .../doltcore/merge/schema_integration_test.go | 2 +- .../doltcore/migrate/integration_test.go | 6 +- go/libraries/doltcore/migrate/transform.go | 6 +- go/libraries/doltcore/rowconv/joiner.go | 2 +- go/libraries/doltcore/schema/column.go | 57 +++++++++++-------- .../schema/encoding/schema_marshaling.go | 2 +- .../schema/encoding/schema_marshaling_test.go | 6 +- .../doltcore/schema/encoding/serialization.go | 26 ++++----- go/libraries/doltcore/sqle/alterschema.go | 4 +- .../doltcore/sqle/alterschema_test.go | 2 +- go/libraries/doltcore/sqle/common_test.go | 2 +- .../sqle/dtables/conflicts_tables_prolly.go | 4 +- .../doltcore/sqle/dtables/diff_table.go | 4 +- .../doltcore/sqle/index/noms_index_iter.go | 2 +- go/libraries/doltcore/sqle/schema_table.go | 2 +- go/libraries/doltcore/sqle/sqlutil/convert.go | 48 +++++++--------- go/libraries/doltcore/sqle/testdata.go | 2 +- 20 files changed, 103 insertions(+), 94 deletions(-) diff --git a/go/cmd/dolt/commands/diff.go b/go/cmd/dolt/commands/diff.go index 41100e2b9a..91cdf52d73 100644 --- a/go/cmd/dolt/commands/diff.go +++ b/go/cmd/dolt/commands/diff.go @@ -864,6 +864,8 @@ func getTableSchemaAtRef(queryist cli.Queryist, sqlCtx *sql.Context, tableName s return sch, createStmt, nil } +// schemaFromCreateTableStmt returns a schema for the CREATE TABLE statement given +// TODO: this is substantially incorrect, doesn't handle primary key ordering, probably other things too func schemaFromCreateTableStmt(createTableStmt string) (schema.Schema, error) { parsed, err := ast.Parse(createTableStmt) if err != nil { @@ -900,7 +902,17 @@ func schemaFromCreateTableStmt(createTableStmt string) (schema.Schema, error) { if col.Type.Comment != nil { comment = col.Type.Comment.String() } - sCol, err := schema.NewColumnWithTypeInfo(col.Name.String(), 0, typeInfo, primaryCols[col.Name.Lowered()], defBuf.String(), "", false, col.Type.Autoincrement == true, comment) + sCol := schema.Column{ + Name: col.Name.String(), + Kind: typeInfo.NomsKind(), + IsPartOfPK: primaryCols[col.Name.Lowered()], + TypeInfo: typeInfo, + Default: defBuf.String(), + Generated: "", // TODO + Virtual: false, // TODO + AutoIncrement: col.Type.Autoincrement == true, + Comment: comment, + } cols = append(cols, sCol) } diff --git a/go/libraries/doltcore/doltdb/system_table.go b/go/libraries/doltcore/doltdb/system_table.go index 2122e7d393..69ec018206 100644 --- a/go/libraries/doltcore/doltdb/system_table.go +++ b/go/libraries/doltcore/doltdb/system_table.go @@ -43,7 +43,7 @@ var OldDocsSchema = schema.MustSchemaFromCols(schema.NewColCollection( var DocsSchema schema.Schema func init() { - docTextCol, err := schema.NewColumnWithTypeInfo(DocTextColumnName, schema.DocTextTag, typeinfo.LongTextType, false, "", "", false, false, "") + docTextCol, err := schema.NewColumnWithTypeInfo(DocTextColumnName, schema.DocTextTag, typeinfo.LongTextType, false, "", false, "") if err != nil { panic(err) } diff --git a/go/libraries/doltcore/doltdb/table.go b/go/libraries/doltcore/doltdb/table.go index 2b5d9c10e5..d94229ea1d 100644 --- a/go/libraries/doltcore/doltdb/table.go +++ b/go/libraries/doltcore/doltdb/table.go @@ -343,11 +343,11 @@ func (t *Table) GetConstraintViolationsSchema(ctx context.Context) (schema.Schem if err != nil { return nil, err } - typeCol, err := schema.NewColumnWithTypeInfo("violation_type", schema.DoltConstraintViolationsTypeTag, typeType, true, "", "", false, false, "") + typeCol, err := schema.NewColumnWithTypeInfo("violation_type", schema.DoltConstraintViolationsTypeTag, typeType, true, "", false, "") if err != nil { return nil, err } - infoCol, err := schema.NewColumnWithTypeInfo("violation_info", schema.DoltConstraintViolationsInfoTag, typeinfo.JSONType, false, "", "", false, false, "") + infoCol, err := schema.NewColumnWithTypeInfo("violation_info", schema.DoltConstraintViolationsInfoTag, typeinfo.JSONType, false, "", false, "") if err != nil { return nil, err } diff --git a/go/libraries/doltcore/merge/schema_integration_test.go b/go/libraries/doltcore/merge/schema_integration_test.go index a3f55bc9ff..64cf18bc5a 100644 --- a/go/libraries/doltcore/merge/schema_integration_test.go +++ b/go/libraries/doltcore/merge/schema_integration_test.go @@ -538,7 +538,7 @@ func schemaFromColsAndIdxs(allCols *schema.ColCollection, indexes ...schema.Inde } func newColTypeInfo(name string, tag uint64, typeInfo typeinfo.TypeInfo, partOfPK bool, constraints ...schema.ColConstraint) schema.Column { - c, err := schema.NewColumnWithTypeInfo(name, tag, typeInfo, partOfPK, "", "", false, false, "", constraints...) + c, err := schema.NewColumnWithTypeInfo(name, tag, typeInfo, partOfPK, "", false, "", constraints...) if err != nil { panic("could not create column") } diff --git a/go/libraries/doltcore/migrate/integration_test.go b/go/libraries/doltcore/migrate/integration_test.go index bf86a85d71..4ab378dc85 100644 --- a/go/libraries/doltcore/migrate/integration_test.go +++ b/go/libraries/doltcore/migrate/integration_test.go @@ -182,9 +182,9 @@ func setupMigrationTest(t *testing.T, ctx context.Context, test migrationTest) * } func SetupHookRefKeys(ctx context.Context, dEnv *env.DoltEnv) (*env.DoltEnv, error) { - pk, _ := schema.NewColumnWithTypeInfo("pk", 1, typeinfo.TextType, true, "", "", false, false, "", schema.NotNullConstraint{}) - c0, _ := schema.NewColumnWithTypeInfo("c0", 2, typeinfo.Int32Type, false, "", "", false, false, "") - c1, _ := schema.NewColumnWithTypeInfo("c1", 3, typeinfo.BlobType, false, "", "", false, false, "") + pk, _ := schema.NewColumnWithTypeInfo("pk", 1, typeinfo.TextType, true, "", false, "", schema.NotNullConstraint{}) + c0, _ := schema.NewColumnWithTypeInfo("c0", 2, typeinfo.Int32Type, false, "", false, "") + c1, _ := schema.NewColumnWithTypeInfo("c1", 3, typeinfo.BlobType, false, "", false, "") sch, err := schema.SchemaFromCols(schema.NewColCollection(pk, c0, c1)) if err != nil { diff --git a/go/libraries/doltcore/migrate/transform.go b/go/libraries/doltcore/migrate/transform.go index 9ac80681e7..ae16f3154d 100644 --- a/go/libraries/doltcore/migrate/transform.go +++ b/go/libraries/doltcore/migrate/transform.go @@ -484,11 +484,11 @@ func migrateSchema(ctx context.Context, tableName string, existing schema.Schema case query.Type_TEXT: patched = true info := typeinfo.StringDefaultType - cols[i], err = schema.NewColumnWithTypeInfo(c.Name, c.Tag, info, c.IsPartOfPK, c.Default, "", false, c.AutoIncrement, c.Comment, c.Constraints...) + cols[i], err = schema.NewColumnWithTypeInfo(c.Name, c.Tag, info, c.IsPartOfPK, c.Default, c.AutoIncrement, c.Comment, c.Constraints...) case query.Type_BLOB: patched = true info := typeinfo.VarbinaryDefaultType - cols[i], err = schema.NewColumnWithTypeInfo(c.Name, c.Tag, info, c.IsPartOfPK, c.Default, "", false, c.AutoIncrement, c.Comment, c.Constraints...) + cols[i], err = schema.NewColumnWithTypeInfo(c.Name, c.Tag, info, c.IsPartOfPK, c.Default, c.AutoIncrement, c.Comment, c.Constraints...) } if err != nil { return nil, err @@ -521,7 +521,7 @@ func migrateSchema(ctx context.Context, tableName string, existing schema.Schema return nil, err } - cols[i], err = schema.NewColumnWithTypeInfo(c.Name, c.Tag, info, c.IsPartOfPK, c.Default, "", false, c.AutoIncrement, c.Comment, c.Constraints...) + cols[i], err = schema.NewColumnWithTypeInfo(c.Name, c.Tag, info, c.IsPartOfPK, c.Default, c.AutoIncrement, c.Comment, c.Constraints...) if err != nil { return nil, err } diff --git a/go/libraries/doltcore/rowconv/joiner.go b/go/libraries/doltcore/rowconv/joiner.go index 77f3683297..fea8b34a5b 100644 --- a/go/libraries/doltcore/rowconv/joiner.go +++ b/go/libraries/doltcore/rowconv/joiner.go @@ -73,7 +73,7 @@ func NewJoiner(namedSchemas []NamedSchema, namers map[string]ColNamingFunc) (*Jo allCols.IterInSortedOrder(func(srcTag uint64, col schema.Column) (stop bool) { newColName := namer(col.Name) var newCol schema.Column - newCol, err = schema.NewColumnWithTypeInfo(newColName, destTag, col.TypeInfo, false, col.Default, "", false, false, col.Comment) + newCol, err = schema.NewColumnWithTypeInfo(newColName, destTag, col.TypeInfo, false, col.Default, false, col.Comment) if err != nil { return true } diff --git a/go/libraries/doltcore/schema/column.go b/go/libraries/doltcore/schema/column.go index 9012b795b4..e342701fda 100644 --- a/go/libraries/doltcore/schema/column.go +++ b/go/libraries/doltcore/schema/column.go @@ -90,7 +90,7 @@ type Column struct { // NewColumn creates a Column instance with the default type info for the NomsKind func NewColumn(name string, tag uint64, kind types.NomsKind, partOfPK bool, constraints ...ColConstraint) Column { typeInfo := typeinfo.FromKind(kind) - col, err := NewColumnWithTypeInfo(name, tag, typeInfo, partOfPK, "", "", false, false, "", constraints...) + col, err := NewColumnWithTypeInfo(name, tag, typeInfo, partOfPK, "", false, "", constraints...) if err != nil { panic(err) } @@ -98,39 +98,46 @@ func NewColumn(name string, tag uint64, kind types.NomsKind, partOfPK bool, cons } // NewColumnWithTypeInfo creates a Column instance with the given type info. -func NewColumnWithTypeInfo( - name string, - tag uint64, - typeInfo typeinfo.TypeInfo, - partOfPK bool, - defaultVal, generatedVal string, - virtual, autoIncrement bool, - comment string, - constraints ...ColConstraint, -) (Column, error) { - for _, c := range constraints { - if c == nil { - return Column{}, errors.New("nil passed as a constraint") - } - } - - if typeInfo == nil { - return Column{}, errors.New("cannot instantiate column with nil type info") - } - - return Column{ +// Callers are encouraged to construct schema.Column structs directly instead of using this method, then call +// ValidateColumn. +func NewColumnWithTypeInfo(name string, tag uint64, typeInfo typeinfo.TypeInfo, partOfPK bool, defaultVal string, autoIncrement bool, comment string, constraints ...ColConstraint, ) (Column, error) { + c := Column{ Name: name, Tag: tag, Kind: typeInfo.NomsKind(), IsPartOfPK: partOfPK, TypeInfo: typeInfo, Default: defaultVal, - Generated: generatedVal, - Virtual: virtual, AutoIncrement: autoIncrement, Comment: comment, Constraints: constraints, - }, nil + } + + err := ValidateColumn(c) + if err != nil { + return InvalidCol, err + } + + return c, nil +} + +// ValidateColumn validates the given column. +func ValidateColumn(c Column) error { + for _, c := range c.Constraints { + if c == nil { + return errors.New("nil passed as a constraint") + } + } + + if c.TypeInfo == nil { + return errors.New("cannot instantiate column with nil type info") + } + + if c.TypeInfo.NomsKind() != c.Kind { + return errors.New("type info and kind do not match") + } + + return nil } // IsNullable returns whether the column can be set to a null value. diff --git a/go/libraries/doltcore/schema/encoding/schema_marshaling.go b/go/libraries/doltcore/schema/encoding/schema_marshaling.go index e31bef7d15..e891b85ba1 100644 --- a/go/libraries/doltcore/schema/encoding/schema_marshaling.go +++ b/go/libraries/doltcore/schema/encoding/schema_marshaling.go @@ -117,7 +117,7 @@ func (nfd encodedColumn) decodeColumn() (schema.Column, error) { return schema.Column{}, errors.New("cannot decode column due to unknown schema format") } colConstraints := decodeAllColConstraint(nfd.Constraints) - return schema.NewColumnWithTypeInfo(nfd.Name, nfd.Tag, typeInfo, nfd.IsPartOfPK, nfd.Default, "", false, nfd.AutoIncrement, nfd.Comment, colConstraints...) + return schema.NewColumnWithTypeInfo(nfd.Name, nfd.Tag, typeInfo, nfd.IsPartOfPK, nfd.Default, nfd.AutoIncrement, nfd.Comment, colConstraints...) } type encodedConstraint struct { diff --git a/go/libraries/doltcore/schema/encoding/schema_marshaling_test.go b/go/libraries/doltcore/schema/encoding/schema_marshaling_test.go index 3a4fc95053..7358775e00 100644 --- a/go/libraries/doltcore/schema/encoding/schema_marshaling_test.go +++ b/go/libraries/doltcore/schema/encoding/schema_marshaling_test.go @@ -137,7 +137,7 @@ func TestTypeInfoMarshalling(t *testing.T) { t.Run(sqlType.String(), func(t *testing.T) { ti, err := typeinfo.FromSqlType(sqlType) require.NoError(t, err) - col, err := schema.NewColumnWithTypeInfo("pk", 1, ti, true, "", "", false, false, "", schema.NotNullConstraint{}) + col, err := schema.NewColumnWithTypeInfo("pk", 1, ti, true, "", false, "", schema.NotNullConstraint{}) require.NoError(t, err) colColl := schema.NewColCollection(col) originalSch, err := schema.SchemaFromCols(colColl) @@ -232,7 +232,7 @@ func (tec testEncodedColumn) decodeColumn() (schema.Column, error) { return schema.Column{}, errors.New("cannot decode column due to unknown schema format") } colConstraints := decodeAllColConstraint(tec.Constraints) - return schema.NewColumnWithTypeInfo(tec.Name, tec.Tag, typeInfo, tec.IsPartOfPK, tec.Default, "", false, tec.AutoIncrement, tec.Comment, colConstraints...) + return schema.NewColumnWithTypeInfo(tec.Name, tec.Tag, typeInfo, tec.IsPartOfPK, tec.Default, tec.AutoIncrement, tec.Comment, colConstraints...) } func (tsd testSchemaData) decodeSchema() (schema.Schema, error) { @@ -297,7 +297,7 @@ func getColumns(t *testing.T) (cols []schema.Column) { for i := range cols { name := "col" + strconv.Itoa(i) tag := uint64(i) - cols[i], err = schema.NewColumnWithTypeInfo(name, tag, ti[i], false, "", "", false, false, "") + cols[i], err = schema.NewColumnWithTypeInfo(name, tag, ti[i], false, "", false, "") require.NoError(t, err) } return diff --git a/go/libraries/doltcore/schema/encoding/serialization.go b/go/libraries/doltcore/schema/encoding/serialization.go index 074a1f0527..2e04ae90eb 100644 --- a/go/libraries/doltcore/schema/encoding/serialization.go +++ b/go/libraries/doltcore/schema/encoding/serialization.go @@ -311,20 +311,18 @@ func deserializeColumns(ctx context.Context, s *serial.TableSchema) ([]schema.Co } } - cols[i], err = schema.NewColumnWithTypeInfo( - string(c.Name()), - c.Tag(), - sqlType, - c.PrimaryKey(), - defVal, - generatedVal, - c.Virtual(), - c.AutoIncrement(), - string(c.Comment()), - constraintsFromSerialColumn(&c)..., - ) - if err != nil { - return nil, err + cols[i] = schema.Column{ + Name: string(c.Name()), + Tag: c.Tag(), + Kind: sqlType.NomsKind(), + IsPartOfPK: c.PrimaryKey(), + TypeInfo: sqlType, + Default: defVal, + Generated: generatedVal, + Virtual: c.Virtual(), + AutoIncrement: c.AutoIncrement(), + Comment: string(c.Comment()), + Constraints: constraintsFromSerialColumn(&c), } } return cols, nil diff --git a/go/libraries/doltcore/sqle/alterschema.go b/go/libraries/doltcore/sqle/alterschema.go index a7ff12084e..1e23c71a0b 100755 --- a/go/libraries/doltcore/sqle/alterschema.go +++ b/go/libraries/doltcore/sqle/alterschema.go @@ -104,9 +104,9 @@ func orderToOrder(order *sql.ColumnOrder) *schema.ColumnOrder { func createColumn(nullable Nullable, newColName string, tag uint64, typeInfo typeinfo.TypeInfo, defaultVal, comment string) (schema.Column, error) { if nullable { - return schema.NewColumnWithTypeInfo(newColName, tag, typeInfo, false, defaultVal, "", false, false, comment) + return schema.NewColumnWithTypeInfo(newColName, tag, typeInfo, false, defaultVal, false, comment) } else { - return schema.NewColumnWithTypeInfo(newColName, tag, typeInfo, false, defaultVal, "", false, false, comment, schema.NotNullConstraint{}) + return schema.NewColumnWithTypeInfo(newColName, tag, typeInfo, false, defaultVal, false, comment, schema.NotNullConstraint{}) } } diff --git a/go/libraries/doltcore/sqle/alterschema_test.go b/go/libraries/doltcore/sqle/alterschema_test.go index b415853661..8169859878 100644 --- a/go/libraries/doltcore/sqle/alterschema_test.go +++ b/go/libraries/doltcore/sqle/alterschema_test.go @@ -650,7 +650,7 @@ func TestModifyColumn(t *testing.T) { ) ti, err := typeinfo.FromSqlType(gmstypes.MustCreateStringWithDefaults(sqltypes.VarChar, 599)) require.NoError(t, err) - newNameColSameTag, err := schema.NewColumnWithTypeInfo("name", dtestutils.NameTag, ti, false, "", "", false, false, "", schema.NotNullConstraint{}) + newNameColSameTag, err := schema.NewColumnWithTypeInfo("name", dtestutils.NameTag, ti, false, "", false, "", schema.NotNullConstraint{}) require.NoError(t, err) alteredTypeSch2 := dtestutils.CreateSchema( schema.NewColumn("id", dtestutils.IdTag, types.StringKind, true, schema.NotNullConstraint{}), diff --git a/go/libraries/doltcore/sqle/common_test.go b/go/libraries/doltcore/sqle/common_test.go index 142abc6803..914ceb8b22 100644 --- a/go/libraries/doltcore/sqle/common_test.go +++ b/go/libraries/doltcore/sqle/common_test.go @@ -112,7 +112,7 @@ func schemaNewColumn(t *testing.T, name string, tag uint64, sqlType sql.Type, pa func schemaNewColumnWDefVal(t *testing.T, name string, tag uint64, sqlType sql.Type, partOfPK bool, defaultVal string, constraints ...schema.ColConstraint) schema.Column { typeInfo, err := typeinfo.FromSqlType(sqlType) require.NoError(t, err) - col, err := schema.NewColumnWithTypeInfo(name, tag, typeInfo, partOfPK, defaultVal, "", false, false, "", constraints...) + col, err := schema.NewColumnWithTypeInfo(name, tag, typeInfo, partOfPK, defaultVal, false, "", constraints...) require.NoError(t, err) return col } diff --git a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go index f2d6d45dc4..35ad70438f 100644 --- a/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go +++ b/go/libraries/doltcore/sqle/dtables/conflicts_tables_prolly.go @@ -715,7 +715,7 @@ func calculateConflictSchema(base, ours, theirs schema.Schema) (schema.Schema, * if !stripConstraints { cons = col.Constraints } - c, err := schema.NewColumnWithTypeInfo(prefix+col.Name, uint64(i), col.TypeInfo, false, col.Default, "", false, false, col.Comment, cons...) + c, err := schema.NewColumnWithTypeInfo(prefix+col.Name, uint64(i), col.TypeInfo, false, col.Default, false, col.Comment, cons...) if err != nil { return true, err } @@ -732,7 +732,7 @@ func calculateConflictSchema(base, ours, theirs schema.Schema) (schema.Schema, * if !stripConstraints { cons = col.Constraints } - c, err := schema.NewColumnWithTypeInfo(prefix+col.Name, uint64(i), col.TypeInfo, false, col.Default, "", false, false, col.Comment, cons...) + c, err := schema.NewColumnWithTypeInfo(prefix+col.Name, uint64(i), col.TypeInfo, false, col.Default, false, col.Comment, cons...) if err != nil { return true, err } diff --git a/go/libraries/doltcore/sqle/dtables/diff_table.go b/go/libraries/doltcore/sqle/dtables/diff_table.go index 2b534e62fe..31b610b95f 100644 --- a/go/libraries/doltcore/sqle/dtables/diff_table.go +++ b/go/libraries/doltcore/sqle/dtables/diff_table.go @@ -917,7 +917,7 @@ func CalculateDiffSchema(fromSch, toSch schema.Schema) (schema.Schema, error) { i := 0 err = toSch.GetAllCols().Iter(func(tag uint64, col schema.Column) (stop bool, err error) { - toCol, err := schema.NewColumnWithTypeInfo(diff.ToColNamer(col.Name), uint64(i), col.TypeInfo, false, col.Default, "", false, false, col.Comment) + toCol, err := schema.NewColumnWithTypeInfo(diff.ToColNamer(col.Name), uint64(i), col.TypeInfo, false, col.Default, false, col.Comment) if err != nil { return true, err } @@ -931,7 +931,7 @@ func CalculateDiffSchema(fromSch, toSch schema.Schema) (schema.Schema, error) { j := toSch.GetAllCols().Size() err = fromSch.GetAllCols().Iter(func(tag uint64, col schema.Column) (stop bool, err error) { - fromCol, err := schema.NewColumnWithTypeInfo(diff.FromColNamer(col.Name), uint64(j), col.TypeInfo, false, col.Default, "", false, false, col.Comment) + fromCol, err := schema.NewColumnWithTypeInfo(diff.FromColNamer(col.Name), uint64(j), col.TypeInfo, false, col.Default, false, col.Comment) if err != nil { return true, err } diff --git a/go/libraries/doltcore/sqle/index/noms_index_iter.go b/go/libraries/doltcore/sqle/index/noms_index_iter.go index a9ca8f4b7d..686628b962 100644 --- a/go/libraries/doltcore/sqle/index/noms_index_iter.go +++ b/go/libraries/doltcore/sqle/index/noms_index_iter.go @@ -261,7 +261,7 @@ func NewCoveringIndexRowIterAdapter(ctx *sql.Context, idx DoltIndex, keyIter *no _, partOfIndexKey := idxCols.GetByTag(col.Tag) _, partOfTableKeys := tblPKCols.GetByTag(col.Tag) if partOfIndexKey != partOfTableKeys { - cols[i], _ = schema.NewColumnWithTypeInfo(col.Name, col.Tag, col.TypeInfo, partOfIndexKey, col.Default, "", false, col.AutoIncrement, col.Comment, col.Constraints...) + cols[i], _ = schema.NewColumnWithTypeInfo(col.Name, col.Tag, col.TypeInfo, partOfIndexKey, col.Default, col.AutoIncrement, col.Comment, col.Constraints...) } } diff --git a/go/libraries/doltcore/sqle/schema_table.go b/go/libraries/doltcore/sqle/schema_table.go index 8174721b4e..9efc5ce3d3 100644 --- a/go/libraries/doltcore/sqle/schema_table.go +++ b/go/libraries/doltcore/sqle/schema_table.go @@ -40,7 +40,7 @@ type Extra struct { } func mustNewColWithTypeInfo(name string, tag uint64, typeInfo typeinfo.TypeInfo, partOfPK bool, defaultVal string, autoIncrement bool, comment string, constraints ...schema.ColConstraint) schema.Column { - col, err := schema.NewColumnWithTypeInfo(name, tag, typeInfo, partOfPK, defaultVal, "", false, autoIncrement, comment, constraints...) + col, err := schema.NewColumnWithTypeInfo(name, tag, typeInfo, partOfPK, defaultVal, autoIncrement, comment, constraints...) if err != nil { panic(err) } diff --git a/go/libraries/doltcore/sqle/sqlutil/convert.go b/go/libraries/doltcore/sqle/sqlutil/convert.go index e271b04db7..56f24318b4 100644 --- a/go/libraries/doltcore/sqle/sqlutil/convert.go +++ b/go/libraries/doltcore/sqle/sqlutil/convert.go @@ -70,7 +70,6 @@ func FromDoltSchema(dbName, tableName string, sch schema.Schema) (sql.PrimaryKey } // ToDoltSchema returns a dolt Schema from the sql schema given, suitable for use in creating a table. -// For result set schemas, see ToDoltResultSchema. func ToDoltSchema( ctx context.Context, root *doltdb.RootValue, @@ -150,31 +149,24 @@ func ToDoltCol(tag uint64, col *sql.Column) (schema.Column, error) { generatedVal = col.Generated.String() } - return schema.NewColumnWithTypeInfo( - col.Name, - tag, - typeInfo, - col.PrimaryKey, - defaultVal, - generatedVal, - col.Virtual, - col.AutoIncrement, - col.Comment, - constraints..., - ) -} - -// ToDoltResultSchema returns a dolt Schema from the sql schema given, suitable for use as a result set -func ToDoltResultSchema(sqlSchema sql.Schema) (schema.Schema, error) { - var cols []schema.Column - for i, col := range sqlSchema { - convertedCol, err := ToDoltCol(uint64(i), col) - if err != nil { - return nil, err - } - cols = append(cols, convertedCol) + c := schema.Column{ + Name: col.Name, + Tag: tag, + Kind: typeInfo.NomsKind(), + IsPartOfPK: col.PrimaryKey, + TypeInfo: typeInfo, + Default: defaultVal, + Generated: generatedVal, + Virtual: col.Virtual, + AutoIncrement: col.AutoIncrement, + Comment: col.Comment, + Constraints: constraints, } - - colColl := schema.NewColCollection(cols...) - return schema.UnkeyedSchemaFromCols(colColl), nil -} + + err = schema.ValidateColumn(c) + if err != nil { + return schema.Column{}, err + } + + return c, nil +} \ No newline at end of file diff --git a/go/libraries/doltcore/sqle/testdata.go b/go/libraries/doltcore/sqle/testdata.go index a455077bcd..7c19497795 100644 --- a/go/libraries/doltcore/sqle/testdata.go +++ b/go/libraries/doltcore/sqle/testdata.go @@ -115,7 +115,7 @@ func createAppearancesTestSchema() schema.Schema { } func newColumnWithTypeInfo(name string, tag uint64, info typeinfo.TypeInfo, partOfPk bool, constraints ...schema.ColConstraint) schema.Column { - col, err := schema.NewColumnWithTypeInfo(name, tag, info, partOfPk, "", "", false, false, "", constraints...) + col, err := schema.NewColumnWithTypeInfo(name, tag, info, partOfPk, "", false, "", constraints...) if err != nil { panic(fmt.Sprintf("unexpected error creating column: %s", err.Error())) } From 1880e00c5e305daea8688d78f5e40dacfbe9cd30 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 1 Nov 2023 15:54:23 -0700 Subject: [PATCH 17/26] Revert --- .../doltcore/sqle/writer/prolly_index_writer_keyless.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/go/libraries/doltcore/sqle/writer/prolly_index_writer_keyless.go b/go/libraries/doltcore/sqle/writer/prolly_index_writer_keyless.go index 0bc0fbd905..a7aa042f99 100644 --- a/go/libraries/doltcore/sqle/writer/prolly_index_writer_keyless.go +++ b/go/libraries/doltcore/sqle/writer/prolly_index_writer_keyless.go @@ -29,10 +29,9 @@ type prollyKeylessWriter struct { name string mut *prolly.MutableMap - keyBld *val.TupleBuilder - valBld *val.TupleBuilder - valMap val.OrdinalMapping - virtualCols []int + keyBld *val.TupleBuilder + valBld *val.TupleBuilder + valMap val.OrdinalMapping } var _ indexWriter = prollyKeylessWriter{} @@ -52,7 +51,6 @@ func (k prollyKeylessWriter) ValidateKeyViolations(ctx context.Context, sqlRow s } func (k prollyKeylessWriter) Insert(ctx context.Context, sqlRow sql.Row) error { - // TODO: this thing needs a function that can convert a row to it storage equivalent as necessary hashId, value, err := k.tuplesFromRow(ctx, sqlRow) if err != nil { return err From 341a1bfab99b187f13418e3a9023640914e18ac5 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 1 Nov 2023 15:57:18 -0700 Subject: [PATCH 18/26] Latest gms --- go/go.mod | 4 ++-- go/go.sum | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go/go.mod b/go/go.mod index cd4778dff2..d7817c1c46 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-20231024164600-7606aaf59e2e + github.com/dolthub/vitess v0.0.0-20231024170615-f475795064f6 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.17.1-0.20231025183208-42851176e976 + github.com/dolthub/go-mysql-server v0.17.1-0.20231101214538-17133d65440d 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 bf3e5acb71..364324fbb5 100644 --- a/go/go.sum +++ b/go/go.sum @@ -181,8 +181,8 @@ 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.17.1-0.20231025183208-42851176e976 h1:ANLG2MSEs/QanWBtroRzf5xgQfBEhuuJncjHlNzBXLM= -github.com/dolthub/go-mysql-server v0.17.1-0.20231025183208-42851176e976/go.mod h1:1cFkU3fI0/MWMWdR3K8oSbLynO3IArpfjoL704rksc4= +github.com/dolthub/go-mysql-server v0.17.1-0.20231101214538-17133d65440d h1:d2p9YUIJQwZuG3EmDemjgXSRHCFJBETKjxfM93Y/ooQ= +github.com/dolthub/go-mysql-server v0.17.1-0.20231101214538-17133d65440d/go.mod h1:YKNZpEARxfNl5LUyIZ8oHEoMDM7DzjnPlhl9cj89QBg= 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= @@ -193,8 +193,8 @@ github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81 h1:7/v8q9X github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81/go.mod h1:siLfyv2c92W1eN/R4QqG/+RjjX5W2+gCTRjZxBjI3TY= 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-20231024164600-7606aaf59e2e h1:l65gk7CyrwNWs2CD5WmFBSouyhP4yJDDzdrvntrfoMs= -github.com/dolthub/vitess v0.0.0-20231024164600-7606aaf59e2e/go.mod h1:IwjNXSQPymrja5pVqmfnYdcy7Uv7eNJNBPK/MEh9OOw= +github.com/dolthub/vitess v0.0.0-20231024170615-f475795064f6 h1:2fGQYxszmACz2xtuyRVblnwvusIefQK9AloUKGIdcCI= +github.com/dolthub/vitess v0.0.0-20231024170615-f475795064f6/go.mod h1:IwjNXSQPymrja5pVqmfnYdcy7Uv7eNJNBPK/MEh9OOw= 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 197ff0986bdb7d593047291fa67a2f3a7c400ae7 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 1 Nov 2023 15:58:27 -0700 Subject: [PATCH 19/26] upgrade gms --- go/go.mod | 2 +- go/go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go/go.mod b/go/go.mod index d38e98760f..866882ed10 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.17.1-0.20231101171732-15bf29b50c09 + github.com/dolthub/go-mysql-server v0.17.1-0.20231101214538-17133d65440d 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 8cc69be9f7..9f988d0cf1 100644 --- a/go/go.sum +++ b/go/go.sum @@ -181,8 +181,8 @@ 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.17.1-0.20231101171732-15bf29b50c09 h1:dwusb2oFbrVhTVofUZ8BvLLFEP798IbsofhRXW4QMWo= -github.com/dolthub/go-mysql-server v0.17.1-0.20231101171732-15bf29b50c09/go.mod h1:YKNZpEARxfNl5LUyIZ8oHEoMDM7DzjnPlhl9cj89QBg= +github.com/dolthub/go-mysql-server v0.17.1-0.20231101214538-17133d65440d h1:d2p9YUIJQwZuG3EmDemjgXSRHCFJBETKjxfM93Y/ooQ= +github.com/dolthub/go-mysql-server v0.17.1-0.20231101214538-17133d65440d/go.mod h1:YKNZpEARxfNl5LUyIZ8oHEoMDM7DzjnPlhl9cj89QBg= 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 962ddec3b1f82135f0c9d0f285f54c03df1dace0 Mon Sep 17 00:00:00 2001 From: zachmu Date: Wed, 1 Nov 2023 23:08:15 +0000 Subject: [PATCH 20/26] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/libraries/doltcore/schema/column.go | 8 ++++---- go/libraries/doltcore/schema/encoding/serialization.go | 4 ++-- go/libraries/doltcore/sqle/sqlutil/convert.go | 6 +++--- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/go/libraries/doltcore/schema/column.go b/go/libraries/doltcore/schema/column.go index e342701fda..dcfe62962f 100644 --- a/go/libraries/doltcore/schema/column.go +++ b/go/libraries/doltcore/schema/column.go @@ -100,7 +100,7 @@ func NewColumn(name string, tag uint64, kind types.NomsKind, partOfPK bool, cons // NewColumnWithTypeInfo creates a Column instance with the given type info. // Callers are encouraged to construct schema.Column structs directly instead of using this method, then call // ValidateColumn. -func NewColumnWithTypeInfo(name string, tag uint64, typeInfo typeinfo.TypeInfo, partOfPK bool, defaultVal string, autoIncrement bool, comment string, constraints ...ColConstraint, ) (Column, error) { +func NewColumnWithTypeInfo(name string, tag uint64, typeInfo typeinfo.TypeInfo, partOfPK bool, defaultVal string, autoIncrement bool, comment string, constraints ...ColConstraint) (Column, error) { c := Column{ Name: name, Tag: tag, @@ -112,12 +112,12 @@ func NewColumnWithTypeInfo(name string, tag uint64, typeInfo typeinfo.TypeInfo, Comment: comment, Constraints: constraints, } - + err := ValidateColumn(c) if err != nil { return InvalidCol, err } - + return c, nil } @@ -132,7 +132,7 @@ func ValidateColumn(c Column) error { if c.TypeInfo == nil { return errors.New("cannot instantiate column with nil type info") } - + if c.TypeInfo.NomsKind() != c.Kind { return errors.New("type info and kind do not match") } diff --git a/go/libraries/doltcore/schema/encoding/serialization.go b/go/libraries/doltcore/schema/encoding/serialization.go index 2e04ae90eb..5739c328fa 100644 --- a/go/libraries/doltcore/schema/encoding/serialization.go +++ b/go/libraries/doltcore/schema/encoding/serialization.go @@ -213,10 +213,10 @@ func serializeSchemaColumns(b *fb.Builder, sch schema.Schema) fb.UOffsetT { co := b.CreateString(col.Comment) do := b.CreateString(defVal) -typeString := sqlTypeString(col.TypeInfo) + typeString := sqlTypeString(col.TypeInfo) to := b.CreateString(typeString) no := b.CreateString(col.Name) - + serial.ColumnStart(b) serial.ColumnAddName(b, no) serial.ColumnAddSqlType(b, to) diff --git a/go/libraries/doltcore/sqle/sqlutil/convert.go b/go/libraries/doltcore/sqle/sqlutil/convert.go index 56f24318b4..bf33398d50 100644 --- a/go/libraries/doltcore/sqle/sqlutil/convert.go +++ b/go/libraries/doltcore/sqle/sqlutil/convert.go @@ -162,11 +162,11 @@ func ToDoltCol(tag uint64, col *sql.Column) (schema.Column, error) { Comment: col.Comment, Constraints: constraints, } - + err = schema.ValidateColumn(c) if err != nil { return schema.Column{}, err } - + return c, nil -} \ No newline at end of file +} From bcb8be4a6d9e0c0a69d41cf1a3aeb05a4cbf020d Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 1 Nov 2023 16:14:05 -0700 Subject: [PATCH 21/26] Bump feature version --- 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 68ef835bc3..5225976d2e 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 = 4 // last bumped when adding sql_mode column to dolt_schemas +var DoltFeatureVersion FeatureVersion = 5 // last bumped when adding virtual columns to schema storage // RootValue is the value of the Database and is the committed value in every Dolt commit. type RootValue struct { From e0f383197008b67b033572d71a1e8c855302e84a Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Wed, 1 Nov 2023 16:34:18 -0700 Subject: [PATCH 22/26] Update hard-coded head hash due to feature version bump --- 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 5ea8359ab4..837c3981e6 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 = "m1gkfp9ii4hiqhpmgcfet5sojvopo4da" + headCommitHash = "li3mp6hml1bctgon5hptfh9b8rqc1i6a" case types.Format_LD_1: headCommitHash = "73hc2robs4v0kt9taoe3m5hd49dmrgun" } From 577a2873f04631a6f4fe3a1e889b80536faa5a4f Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Thu, 2 Nov 2023 13:19:02 -0700 Subject: [PATCH 23/26] Update feature version for bats --- 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 c4a0a03ddb..0667922d72 100644 --- a/integration-tests/bats/helper/common.bash +++ b/integration-tests/bats/helper/common.bash @@ -81,7 +81,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: 4" ]] || exit 1 + [[ "$output" =~ "feature version: 5" ]] || exit 1 else # Clear status to avoid BATS failing if this is the last run command status=0 From fc6ae719b88ddad8e0cff95c12e2e089ddd52af8 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Thu, 2 Nov 2023 14:23:29 -0700 Subject: [PATCH 24/26] Removed redundant feature version test (we check this in setup in many places already) --- integration-tests/bats/status.bats | 8 -------- 1 file changed, 8 deletions(-) diff --git a/integration-tests/bats/status.bats b/integration-tests/bats/status.bats index 412ec96367..32eda479dc 100644 --- a/integration-tests/bats/status.bats +++ b/integration-tests/bats/status.bats @@ -14,14 +14,6 @@ get_head_commit() { dolt log -n 1 | grep -m 1 commit | cut -c 13-44 } -@test "status: dolt version --feature" { - # bump this test with feature version bumps - run dolt version --feature - [ "$status" -eq 0 ] - [[ "$output" =~ "dolt version" ]] || false - [[ "$output" =~ "feature version: 4" ]] || false -} - @test "status: no changes" { run dolt status [ "$status" -eq 0 ] From 0402ec26625335489a82f48fd64028b23d6cce08 Mon Sep 17 00:00:00 2001 From: Zach Musgrave Date: Thu, 2 Nov 2023 14:25:21 -0700 Subject: [PATCH 25/26] Re-skip test --- .../sqle/enginetest/dolt_engine_test.go | 73 +------------------ 1 file changed, 1 insertion(+), 72 deletions(-) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go index 05f2f93774..12350228a6 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go @@ -116,80 +116,9 @@ func TestSingleQuery(t *testing.T) { // Convenience test for debugging a single query. Unskip and set to the desired query. func TestSingleScript(t *testing.T) { - // t.Skip() + t.Skip() var scripts = []queries.ScriptTest{ - { - Name: "virtual column index", - SetUpScript: []string{ - "create table t1 (a int primary key, b int, c int generated always as (a + b) virtual, index idx_c (c))", - "insert into t1 (a, b) values (1, 2), (3, 4)", - }, - Assertions: []queries.ScriptTestAssertion{ - { - Query: "select * from t1 where c = 7", - Expected: []sql.Row{{3, 4, 7}}, - }, - { - Query: "explain select * from t1 where c = 7", - Expected: []sql.Row{ - {"IndexedTableAccess(t1)"}, - {" ├─ index: [t1.c]"}, - {" └─ filters: [{[7, 7]}]"}, - }, - }, - { - Query: "select * from t1 where c = 8", - Expected: []sql.Row{}, - }, - { - Query: "explain update t1 set b = 5 where c = 3", - Expected: []sql.Row{ - {"Update"}, - {" └─ UpdateSource(SET t1.b = 5,SET t1.c = ((t1.a + t1.b)))"}, - {" └─ IndexedTableAccess(t1)"}, - {" ├─ index: [t1.c]"}, - {" └─ filters: [{[3, 3]}]"}, - }, - }, - { - Query: "update t1 set b = 5 where c = 3", - Expected: []sql.Row{{newUpdateResult(1, 1)}}, - }, - { - Query: "select * from t1 order by a", - Expected: []sql.Row{ - {1, 5, 6}, - {3, 4, 7}, - }, - }, - { - Query: "select * from t1 where c = 6", - Expected: []sql.Row{ - {1, 5, 6}, - }, - }, - { - Query: "explain delete from t1 where c = 6", - Expected: []sql.Row{ - {"Delete"}, - {" └─ IndexedTableAccess(t1)"}, - {" ├─ index: [t1.c]"}, - {" └─ filters: [{[6, 6]}]"}, - }, - }, - { - Query: "delete from t1 where c = 6", - Expected: []sql.Row{{gmstypes.NewOkResult(1)}}, - }, - { - Query: "select * from t1 order by a", - Expected: []sql.Row{ - {3, 4, 7}, - }, - }, - }, - }, } tcc := &testCommitClock{} From 18b095db796bd7951f82c22144183abb89c697b9 Mon Sep 17 00:00:00 2001 From: zachmu Date: Thu, 2 Nov 2023 21:43:38 +0000 Subject: [PATCH 26/26] [ga-format-pr] Run go/utils/repofmt/format_repo.sh and go/Godeps/update.sh --- go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go index 12350228a6..895d74f477 100644 --- a/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go +++ b/go/libraries/doltcore/sqle/enginetest/dolt_engine_test.go @@ -118,8 +118,7 @@ func TestSingleQuery(t *testing.T) { func TestSingleScript(t *testing.T) { t.Skip() - var scripts = []queries.ScriptTest{ - } + var scripts = []queries.ScriptTest{} tcc := &testCommitClock{} cleanup := installTestCommitClock(tcc)