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

Persist table comment from create table #7441

Merged
merged 11 commits into from
Feb 1, 2024
Merged
13 changes: 12 additions & 1 deletion go/gen/fb/serial/schema.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion go/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,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.20240131203940-2ebd4ec1754d
github.com/dolthub/go-mysql-server v0.17.1-0.20240131221758-2e9aaefccc28
github.com/dolthub/swiss v0.1.0
github.com/goccy/go-json v0.10.2
github.com/google/go-github/v57 v57.0.0
Expand Down
2 changes: 2 additions & 0 deletions go/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,8 @@ github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e h1:kPsT4a47cw
github.com/dolthub/go-icu-regex v0.0.0-20230524105445-af7e7991c97e/go.mod h1:KPUcpx070QOfJK1gNe0zx4pA5sicIK1GMikIGLKC168=
github.com/dolthub/go-mysql-server v0.17.1-0.20240131203940-2ebd4ec1754d h1:UTH7fcEYsuyg1ytXe/BOt12I5nV9gFN3mf4OG53uxWU=
github.com/dolthub/go-mysql-server v0.17.1-0.20240131203940-2ebd4ec1754d/go.mod h1:ZJuTZ1/lCAWdtHU0Vm+7GaPVBEZORtfk2sBk55eSnCs=
github.com/dolthub/go-mysql-server v0.17.1-0.20240131221758-2e9aaefccc28 h1:oyfSDfE8UxPQkXbuLjG9CWWYnwKRC6R1Y05xZtGPVRo=
github.com/dolthub/go-mysql-server v0.17.1-0.20240131221758-2e9aaefccc28/go.mod h1:ZJuTZ1/lCAWdtHU0Vm+7GaPVBEZORtfk2sBk55eSnCs=
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=
Expand Down
2 changes: 1 addition & 1 deletion go/libraries/doltcore/mvdata/engine_table_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ func (s *SqlEngineTableWriter) createTable() error {
sqlCols = append(sqlCols, fmt.Sprintf("PRIMARY KEY (%s)", pks))
}

createTable := sql.GenerateCreateTableStatement(s.tableName, sqlCols, sql.CharacterSet_utf8.String(), sql.Collation_Default.String())
createTable := sql.GenerateCreateTableStatement(s.tableName, sqlCols, sql.CharacterSet_utf8.String(), sql.Collation_Default.String(), "")
_, iter, err := s.se.Query(s.sqlCtx, createTable)
if err != nil {
return err
Expand Down
6 changes: 6 additions & 0 deletions go/libraries/doltcore/schema/encoding/serialization.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ func serializeSchemaAsFlatbuffer(sch schema.Schema) ([]byte, error) {
rows := serializeClusteredIndex(b, sch)
indexes := serializeSecondaryIndexes(b, sch, sch.Indexes().AllIndexes())
checks := serializeChecks(b, sch.Checks().AllChecks())
comment := b.CreateString(sch.GetComment())

var hasFeaturesAfterTryAccessors bool
for _, col := range sch.GetAllCols().GetColumns() {
Expand All @@ -71,6 +72,10 @@ func serializeSchemaAsFlatbuffer(sch schema.Schema) ([]byte, error) {
serial.TableSchemaAddSecondaryIndexes(b, indexes)
serial.TableSchemaAddChecks(b, checks)
serial.TableSchemaAddCollation(b, serial.Collation(sch.GetCollation()))
if sch.GetComment() != "" {
serial.TableSchemaAddComment(b, comment)
hasFeaturesAfterTryAccessors = true
}
if hasFeaturesAfterTryAccessors {
serial.TableSchemaAddHasFeaturesAfterTryAccessors(b, hasFeaturesAfterTryAccessors)
}
Expand Down Expand Up @@ -123,6 +128,7 @@ func deserializeSchemaFromFlatbuffer(ctx context.Context, buf []byte) (schema.Sc
}

sch.SetCollation(schema.Collation(s.Collation()))
sch.SetComment(string(s.Comment()))

return sch, nil
}
Expand Down
6 changes: 6 additions & 0 deletions go/libraries/doltcore/schema/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ type Schema interface {
// SetCollation sets the table's collation.
SetCollation(collation Collation)

// GetComment returns the table's comment.
GetComment() string

// SetComment sets the table's comment.
SetComment(comment string)

// Copy returns a copy of this Schema that can be safely modified independently.
Copy() Schema
}
Expand Down
9 changes: 9 additions & 0 deletions go/libraries/doltcore/schema/schema_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ type schemaImpl struct {
pkOrdinals []int
collation Collation
contentHashedFields []uint64
comment string
}

var _ Schema = (*schemaImpl)(nil)
Expand Down Expand Up @@ -264,6 +265,14 @@ func SchemaFromPKAndNonPKCols(pkCols, nonPKCols *ColCollection) (Schema, error)
return SchemaFromColCollections(allColColl, pkCols, nonPKCols), nil
}

func (si *schemaImpl) GetComment() string {
return si.comment
}

func (si *schemaImpl) SetComment(comment string) {
si.comment = comment
}

// GetAllCols gets the collection of all columns (pk and non-pk)
func (si *schemaImpl) GetAllCols() *ColCollection {
return si.allCols
Expand Down
9 changes: 5 additions & 4 deletions go/libraries/doltcore/sqle/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -886,7 +886,7 @@ func (db Database) removeTableFromAutoIncrementTracker(
}

// CreateTable creates a table with the name and schema given.
func (db Database) CreateTable(ctx *sql.Context, tableName string, sch sql.PrimaryKeySchema, collation sql.CollationID) error {
func (db Database) CreateTable(ctx *sql.Context, tableName string, sch sql.PrimaryKeySchema, collation sql.CollationID, comment string) error {
if err := dsess.CheckAccessForDb(ctx, db, branch_control.Permissions_Write); err != nil {
return err
}
Expand All @@ -898,7 +898,7 @@ func (db Database) CreateTable(ctx *sql.Context, tableName string, sch sql.Prima
return ErrInvalidTableName.New(tableName)
}

return db.createSqlTable(ctx, tableName, sch, collation)
return db.createSqlTable(ctx, tableName, sch, collation, comment)
}

// CreateIndexedTable creates a table with the name and schema given.
Expand Down Expand Up @@ -944,7 +944,7 @@ OuterLoop:
}

// createSqlTable is the private version of CreateTable. It doesn't enforce any table name checks.
func (db Database) createSqlTable(ctx *sql.Context, tableName string, sch sql.PrimaryKeySchema, collation sql.CollationID) error {
func (db Database) createSqlTable(ctx *sql.Context, tableName string, sch sql.PrimaryKeySchema, collation sql.CollationID, comment string) error {
ws, err := db.GetWorkingSet(ctx)
if err != nil {
return err
Expand All @@ -966,6 +966,7 @@ func (db Database) createSqlTable(ctx *sql.Context, tableName string, sch sql.Pr
if err != nil {
return err
}
doltSch.SetComment(comment)

// Prevent any tables that use Spatial Types as Primary Key from being created
if schema.IsUsingSpatialColAsKey(doltSch) {
Expand Down Expand Up @@ -1709,7 +1710,7 @@ func (db Database) LoadRebasePlan(ctx *sql.Context) (*rebase.RebasePlan, error)
func (db Database) SaveRebasePlan(ctx *sql.Context, plan *rebase.RebasePlan) error {
pkSchema := sql.NewPrimaryKeySchema(dprocedures.DoltRebaseSystemTableSchema)
// we use createSqlTable, instead of CreateTable to avoid the "dolt_" reserved prefix table name check
err := db.createSqlTable(ctx, doltdb.RebaseTableName, pkSchema, sql.Collation_Default)
err := db.createSqlTable(ctx, doltdb.RebaseTableName, pkSchema, sql.Collation_Default, "")
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion go/libraries/doltcore/sqle/enginetest/dolt_harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ func (d *DoltHarness) newTable(db sql.Database, name string, schema sql.PrimaryK

ctx := enginetest.NewContext(d)
ctx.Session.SetCurrentDatabase(db.Name())
err := tc.CreateTable(ctx, name, schema, sql.Collation_Default)
err := tc.CreateTable(ctx, name, schema, sql.Collation_Default, "")
if err != nil {
return nil, err
}
Expand Down
5 changes: 5 additions & 0 deletions go/libraries/doltcore/sqle/indexed_dolt_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func NewIndexedDoltTable(t *DoltTable, idx index.DoltIndex) *IndexedDoltTable {
}

var _ sql.IndexedTable = (*IndexedDoltTable)(nil)
var _ sql.CommentedTable = (*IndexedDoltTable)(nil)

func (idt *IndexedDoltTable) GetIndexes(ctx *sql.Context) ([]sql.Index, error) {
return idt.table.GetIndexes(ctx)
Expand All @@ -65,6 +66,10 @@ func (idt *IndexedDoltTable) Collation() sql.CollationID {
return sql.CollationID(idt.table.sch.GetCollation())
}

func (idt *IndexedDoltTable) Comment() string {
return idt.table.Comment()
}

func (idt *IndexedDoltTable) LookupPartitions(ctx *sql.Context, lookup sql.IndexLookup) (sql.PartitionIter, error) {
return index.NewRangePartitionIter(ctx, idt.table, lookup, idt.isDoltFormat)
}
Expand Down
2 changes: 1 addition & 1 deletion go/libraries/doltcore/sqle/procedures_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func newDatabaseWithProcedures(t *testing.T, dEnv *env.DoltEnv, opts editor.Opti
{Name: doltdb.ProceduresTableCreateStmtCol, Type: gmstypes.Text, Source: doltdb.ProceduresTableName, PrimaryKey: false},
{Name: doltdb.ProceduresTableCreatedAtCol, Type: gmstypes.Timestamp, Source: doltdb.ProceduresTableName, PrimaryKey: false},
{Name: doltdb.ProceduresTableModifiedAtCol, Type: gmstypes.Timestamp, Source: doltdb.ProceduresTableName, PrimaryKey: false},
}), sql.Collation_Default)
}), sql.Collation_Default, "")
require.NoError(t, err)

sqlTbl, found, err := db.GetTableInsensitive(ctx, doltdb.ProceduresTableName)
Expand Down
4 changes: 2 additions & 2 deletions go/libraries/doltcore/sqle/schema_table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ func TestSchemaTableMigrationOriginal(t *testing.T) {
{Name: doltdb.SchemasTablesTypeCol, Type: gmstypes.Text, Source: doltdb.SchemasTableName, PrimaryKey: true},
{Name: doltdb.SchemasTablesNameCol, Type: gmstypes.Text, Source: doltdb.SchemasTableName, PrimaryKey: true},
{Name: doltdb.SchemasTablesFragmentCol, Type: gmstypes.Text, Source: doltdb.SchemasTableName, PrimaryKey: false},
}), sql.Collation_Default)
}), sql.Collation_Default, "")
require.NoError(t, err)

sqlTbl, found, err := db.GetTableInsensitive(ctx, doltdb.SchemasTableName)
Expand Down Expand Up @@ -105,7 +105,7 @@ func TestSchemaTableMigrationV1(t *testing.T) {
{Name: doltdb.SchemasTablesFragmentCol, Type: gmstypes.Text, Source: doltdb.SchemasTableName, PrimaryKey: false},
{Name: doltdb.SchemasTablesIdCol, Type: gmstypes.Int64, Source: doltdb.SchemasTableName, PrimaryKey: true},
{Name: doltdb.SchemasTablesExtraCol, Type: gmstypes.JsonType{}, Source: doltdb.SchemasTableName, PrimaryKey: false, Nullable: true},
}), sql.Collation_Default)
}), sql.Collation_Default, "")
require.NoError(t, err)

sqlTbl, found, err := db.GetTableInsensitive(ctx, doltdb.SchemasTableName)
Expand Down
2 changes: 1 addition & 1 deletion go/libraries/doltcore/sqle/sqlfmt/schema_fmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ func GenerateCreateTableStatement(tblName string, sch schema.Schema, fks []doltd
}

coll := sql.CollationID(sch.GetCollation())
createTableStmt := sql.GenerateCreateTableStatement(tblName, colStmts, coll.CharacterSet().Name(), coll.Name())
createTableStmt := sql.GenerateCreateTableStatement(tblName, colStmts, coll.CharacterSet().Name(), coll.Name(), sch.GetComment())
return fmt.Sprintf("%s;", createTableStmt), nil
}

Expand Down
6 changes: 6 additions & 0 deletions go/libraries/doltcore/sqle/tables.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ type doltReadOnlyTableInterface interface {
sql.StatisticsTable
sql.CheckTable
sql.PrimaryKeyTable
sql.CommentedTable
}

var _ doltReadOnlyTableInterface = (*DoltTable)(nil)
Expand Down Expand Up @@ -360,6 +361,11 @@ func (t *DoltTable) Collation() sql.CollationID {
return sql.CollationID(t.sch.GetCollation())
}

// Comment returns the comment for this table.
func (t *DoltTable) Comment() string {
return t.sch.GetComment()
}

func (t *DoltTable) sqlSchema() sql.PrimaryKeySchema {
// TODO: this should consider projections
if len(t.sqlSch.Schema) > 0 {
Expand Down
3 changes: 3 additions & 0 deletions go/serial/schema.fbs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ table TableSchema {

// this field is necessary because older dolt clients weren't using TryAccessor for Columns, but are in TableSchemas
has_features_after_try_accessors:bool;

// table comment
comment:string;
}

table Column {
Expand Down