Skip to content

Commit

Permalink
Merge pull request #7441 from dolthub/fulghum/table-comments
Browse files Browse the repository at this point in the history
Persist table comment from `create table`
  • Loading branch information
fulghum authored Feb 1, 2024
2 parents 52c4fe7 + 89355f4 commit 88b475c
Show file tree
Hide file tree
Showing 15 changed files with 61 additions and 12 deletions.
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.20240201064609-1585c52bf41a
github.com/dolthub/go-mysql-server v0.17.1-0.20240201220254-4d817f9151c8
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.20240201064609-1585c52bf41a h1:MMOkz3SxiSdqmcsxB8NJO0bn46+nv0QNLABLpp/WlKw=
github.com/dolthub/go-mysql-server v0.17.1-0.20240201064609-1585c52bf41a/go.mod h1:1MiTBuNubXFJsO67tJpt4DGErQx/M5puDcejNKs65gg=
github.com/dolthub/go-mysql-server v0.17.1-0.20240201220254-4d817f9151c8 h1:1FqMNjiCl4aoe4WgvgpLQcKNNjUJI6AqTEthV/B1OCY=
github.com/dolthub/go-mysql-server v0.17.1-0.20240201220254-4d817f9151c8/go.mod h1:1MiTBuNubXFJsO67tJpt4DGErQx/M5puDcejNKs65gg=
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.20240201003050-392940944c15 h1:sfTETOpsrNJPDn2KydiCtDgVu6Xopq8k3JP8PjFT22s=
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

0 comments on commit 88b475c

Please sign in to comment.