Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support multi-inherit #2738

Merged
merged 8 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require (
github.com/dolthub/go-icu-regex v0.0.0-20240916130659-0118adc6b662
github.com/dolthub/jsonpath v0.0.2-0.20240227200619-19675ab05c71
github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81
github.com/dolthub/vitess v0.0.0-20241104125316-860772ba6683
github.com/dolthub/vitess v0.0.0-20241108004726-fd871a8e800e
github.com/go-kit/kit v0.10.0
github.com/go-sql-driver/mysql v1.7.2-0.20231213112541-0004702b931d
github.com/gocraft/dbr/v2 v2.7.2
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ github.com/dolthub/jsonpath v0.0.2-0.20240227200619-19675ab05c71 h1:bMGS25NWAGTE
github.com/dolthub/jsonpath v0.0.2-0.20240227200619-19675ab05c71/go.mod h1:2/2zjLQ/JOOSbbSboojeg+cAwcRV0fDLzIiWch/lhqI=
github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81 h1:7/v8q9XGFa6q5Ap4Z/OhNkAMBaK5YeuEzwJt+NZdhiE=
github.com/dolthub/sqllogictest/go v0.0.0-20201107003712-816f3ae12d81/go.mod h1:siLfyv2c92W1eN/R4QqG/+RjjX5W2+gCTRjZxBjI3TY=
github.com/dolthub/vitess v0.0.0-20241104125316-860772ba6683 h1:2/RJeUfNAXS7mbBnEr9C36htiCJHk5XldDPzhxtEsME=
github.com/dolthub/vitess v0.0.0-20241104125316-860772ba6683/go.mod h1:uBvlRluuL+SbEWTCZ68o0xvsdYZER3CEG/35INdzfJM=
github.com/dolthub/vitess v0.0.0-20241108004726-fd871a8e800e h1:LzLeTanHHI4h75GGqo7GVlFZsOpR6VOn5j3ug+a3+R8=
github.com/dolthub/vitess v0.0.0-20241108004726-fd871a8e800e/go.mod h1:uBvlRluuL+SbEWTCZ68o0xvsdYZER3CEG/35INdzfJM=
github.com/dustin/go-humanize v0.0.0-20171111073723-bb3d318650d4/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25KnS6fMYU6eOk=
github.com/eapache/go-resiliency v1.1.0/go.mod h1:kFI+JgMyC7bLPUVY133qvEBtVayf5mFgVsvEsIPBvNs=
github.com/eapache/go-xerial-snappy v0.0.0-20180814174437-776d5712da21/go.mod h1:+020luEh2TKB4/GOp8oxxtq0Daoen/Cii55CzbTV6DU=
Expand Down
181 changes: 116 additions & 65 deletions sql/planbuilder/ddl.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,85 +331,136 @@ func assignColumnIndexesInSchema(schema sql.Schema) sql.Schema {
return newSch
}

func (b *Builder) buildCreateTableLike(inScope *scope, ct *ast.DDL) *scope {
outScope, ok := b.buildTablescan(inScope, ct.OptLike.LikeTable, nil)
if !ok {
b.handleErr(sql.ErrTableNotFound.New(ct.OptLike.LikeTable.Name.String()))
func (b *Builder) getIndexDefs(table sql.Table) sql.IndexDefs {
idxTbl, isIdxTbl := table.(sql.IndexAddressableTable)
if !isIdxTbl {
return nil
}

likeTable, ok := outScope.node.(*plan.ResolvedTable)
if !ok {
err := fmt.Errorf("expected resolved table: %s", ct.OptLike.LikeTable.Name.String())
var idxDefs sql.IndexDefs
idxs, err := idxTbl.GetIndexes(b.ctx)
if err != nil {
b.handleErr(err)
}
for _, idx := range idxs {
if idx.IsGenerated() {
continue
}
constraint := sql.IndexConstraint_None
if idx.IsUnique() {
if idx.ID() == "PRIMARY" {
constraint = sql.IndexConstraint_Primary
} else {
constraint = sql.IndexConstraint_Unique
}
}
columns := make([]sql.IndexColumn, len(idx.Expressions()))
for i, col := range idx.Expressions() {
// TODO: find a better way to get only the column name if the table is present
col = strings.TrimPrefix(col, idxTbl.Name()+".")
columns[i] = sql.IndexColumn{Name: col}
}
idxDefs = append(idxDefs, &sql.IndexDef{
Name: idx.ID(),
Storage: sql.IndexUsing_Default,
Constraint: constraint,
Columns: columns,
Comment: idx.Comment(),
})
}
return idxDefs
}

func (b *Builder) buildCreateTableLike(inScope *scope, ct *ast.DDL) *scope {
database := b.resolveDbForTable(ct.Table)
newTableName := strings.ToLower(ct.Table.Name.String())
outScope.setTableAlias(newTableName)

var pkSch sql.PrimaryKeySchema
var coll sql.CollationID
var comment string
outScope := inScope.push()
if ct.TableSpec != nil {
pkSch, coll, _ = b.tableSpecToSchema(inScope, outScope, database, strings.ToLower(ct.Table.Name.String()), ct.TableSpec, false)
}

var ok bool
var pkOrdinals []int
var newSch sql.Schema
newSchMap := make(map[string]struct{})
var idxDefs sql.IndexDefs
if indexableTable, ok := likeTable.Table.(sql.IndexAddressableTable); ok {
indexes, err := indexableTable.GetIndexes(b.ctx)
if err != nil {
var checkDefs []*sql.CheckConstraint
for _, likeTable := range ct.OptLike.LikeTables {
outScope, ok = b.buildTablescan(outScope, likeTable, nil)
if !ok {
b.handleErr(sql.ErrTableNotFound.New(likeTable.Name.String()))
}
lTable, isResTbl := outScope.node.(*plan.ResolvedTable)
if !isResTbl {
err := fmt.Errorf("expected resolved table: %s", likeTable.Name.String())
b.handleErr(err)
}
for _, index := range indexes {
if index.IsGenerated() {
continue
}
constraint := sql.IndexConstraint_None
if index.IsUnique() {
if index.ID() == "PRIMARY" {
constraint = sql.IndexConstraint_Primary
} else {
constraint = sql.IndexConstraint_Unique
}
}

columns := make([]sql.IndexColumn, len(index.Expressions()))
for i, col := range index.Expressions() {
//TODO: find a better way to get only the column name if the table is present
col = strings.TrimPrefix(col, indexableTable.Name()+".")
columns[i] = sql.IndexColumn{Name: col}
}
idxDefs = append(idxDefs, &sql.IndexDef{
Name: index.ID(),
Storage: sql.IndexUsing_Default,
Constraint: constraint,
Columns: columns,
Comment: index.Comment(),
})
if coll == sql.Collation_Unspecified {
coll = lTable.Collation()
}
}
origSch := likeTable.Schema()
newSch := make(sql.Schema, len(origSch))
for i, col := range origSch {
tempCol := *col
tempCol.Source = newTableName
newSch[i] = &tempCol
}

var pkOrdinals []int
if pkTable, ok := likeTable.Table.(sql.PrimaryKeyTable); ok {
pkOrdinals = pkTable.PrimaryKeySchema().PkOrdinals
}
if comment == "" {
comment = lTable.Comment()
}

var checkDefs []*sql.CheckConstraint
if checksTable, ok := likeTable.Table.(sql.CheckTable); ok {
checks, err := checksTable.GetChecks(b.ctx)
if err != nil {
b.handleErr(err)
schOff := len(newSch)
hasSkippedCols := false
for _, col := range lTable.Schema() {
newCol := *col
name := strings.ToLower(newCol.Name)
if _, ok := newSchMap[name]; ok {
// TODO: throw warning
hasSkippedCols = true
continue
}
newSchMap[name] = struct{}{}
newCol.Source = newTableName
newSch = append(newSch, &newCol)
}

for _, check := range checks {
checkConstraint := b.buildCheckConstraint(outScope, &check)
if err != nil {
b.handleErr(err)
// if a column was skipped due to duplicates, don't copy over PK ords, idxDefs, or checkDefs
// since they might be incorrect
if hasSkippedCols {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are the semantics really this simple? seems like you'd need way more testing to figure out correct behavior

continue
}

// Copy over primary key schema ordinals
if pkTable, isPkTable := lTable.Table.(sql.PrimaryKeyTable); isPkTable {
for _, pkOrd := range pkTable.PrimaryKeySchema().PkOrdinals {
pkOrdinals = append(pkOrdinals, schOff+pkOrd)
}
}

// Load index definitions
idxDefs = append(idxDefs, b.getIndexDefs(lTable.Table)...)

// Load check constraints
newCheckDefs := b.loadChecksFromTable(outScope, lTable.Table)
for _, check := range newCheckDefs {
// Prevent a name collision between old and new checks.
// New check will be assigned a name during building.
checkConstraint.Name = ""
checkDefs = append(checkDefs, checkConstraint)
// New check name will be assigned a name during building.
check.Name = ""
}
checkDefs = append(checkDefs, newCheckDefs...)
}

var hasSkippedCols bool
for _, col := range pkSch.Schema {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is explicit schema not incompatible with the LIKE list?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we have tests for this?

name := strings.ToLower(col.Name)
if _, ok := newSchMap[name]; ok {
// TODO: throw warning
hasSkippedCols = true
continue
}
newSch = append(newSch, col)
}
if !hasSkippedCols {
for _, pkOrd := range pkSch.PkOrdinals {
pkOrdinals = append(pkOrdinals, len(newSch)+pkOrd)
}
}

Expand All @@ -420,13 +471,13 @@ func (b *Builder) buildCreateTableLike(inScope *scope, ct *ast.DDL) *scope {
Schema: pkSchema,
IdxDefs: idxDefs,
ChDefs: checkDefs,
Collation: likeTable.Collation(),
Comment: likeTable.Comment(),
Collation: coll,
Comment: comment,
}

database := b.resolveDbForTable(ct.Table)

b.qFlags.Set(sql.QFlagSetDatabase)

outScope.setTableAlias(newTableName)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is probably incorrect strictly speaking, as you'll only have the columns in the last table in the list in this scope. Not sure when it matters though. @max-hoffman ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's definitely an abuse of the model...a more reasonable pass might accumulate the table cols into a top-level output scope or return an empty scope

outScope.node = plan.NewCreateTable(database, newTableName, ct.IfNotExists, ct.Temporary, tableSpec)
return outScope
}
Expand Down
Loading