From 136807549d37b581f09d0d4133715d28642598c6 Mon Sep 17 00:00:00 2001 From: Ariel Mashraki <7413593+a8m@users.noreply.github.com> Date: Mon, 26 Dec 2022 14:48:37 +0200 Subject: [PATCH] dialect/entsql: supports setting SQL column comments (#3191) * feat: Add column comment in SQL DDL, using EntSQL annotations to achieve it. * Update annotation.go * fix lint * Add table primary key column comment * entsql.Comment(string) is unnecessary * entc/gen: minor changes to entsql.WithComments + add tests Co-authored-by: chenghonour --- dialect/entsql/annotation.go | 27 +++++++++++++ dialect/sql/schema/atlas.go | 3 ++ dialect/sql/schema/schema.go | 1 + entc/gen/graph.go | 2 +- entc/gen/template/migrate/schema.tmpl | 1 + entc/gen/type.go | 39 +++++++++++++++---- entc/integration/migrate/entv2/media.go | 4 +- .../migrate/entv2/migrate/schema.go | 2 +- .../integration/migrate/entv2/schema/media.go | 18 +++++---- entc/integration/migrate/migrate_test.go | 18 ++++++++- 10 files changed, 94 insertions(+), 21 deletions(-) diff --git a/dialect/entsql/annotation.go b/dialect/entsql/annotation.go index bdbe9f525..f0ff39625 100644 --- a/dialect/entsql/annotation.go +++ b/dialect/entsql/annotation.go @@ -89,6 +89,16 @@ type Annotation struct { // Size int64 `json:"size,omitempty"` + // WithComments specifies whether fields' comments should + // be stored in the database schema as column comments. + // + // withCommentsEnabled := true + // entsql.WithComments{ + // WithComments: &withCommentsEnabled, + // } + // + WithComments *bool `json:"with_comments,omitempty"` + // Incremental defines the auto-incremental behavior of a column. For example: // // incrementalEnabled := true @@ -205,6 +215,20 @@ func DefaultExprs(exprs map[string]string) *Annotation { } } +// WithComments specifies whether fields' comments should +// be stored in the database schema as column comments. +// +// func (T) Annotations() []schema.Annotation { +// return []schema.Annotation{ +// entsql.WithComments(true), +// } +// } +func WithComments(b bool) *Annotation { + return &Annotation{ + WithComments: &b, + } +} + // Merge implements the schema.Merger interface. func (a Annotation) Merge(other schema.Annotation) schema.Annotation { var ant Annotation @@ -247,6 +271,9 @@ func (a Annotation) Merge(other schema.Annotation) schema.Annotation { if s := ant.Size; s != 0 { a.Size = s } + if b := ant.WithComments; b != nil { + a.WithComments = b + } if i := ant.Incremental; i != nil { a.Incremental = i } diff --git a/dialect/sql/schema/atlas.go b/dialect/sql/schema/atlas.go index e06176bf6..4f9b4d187 100644 --- a/dialect/sql/schema/atlas.go +++ b/dialect/sql/schema/atlas.go @@ -912,6 +912,9 @@ func (a *Atlas) aColumns(et *Table, at *schema.Table) error { if c1.Collation != "" { c2.SetCollation(c1.Collation) } + if c1.Comment != "" { + c2.SetComment(c1.Comment) + } if err := a.sqlDialect.atTypeC(c1, c2); err != nil { return err } diff --git a/dialect/sql/schema/schema.go b/dialect/sql/schema/schema.go index 4670579d8..400185267 100644 --- a/dialect/sql/schema/schema.go +++ b/dialect/sql/schema/schema.go @@ -295,6 +295,7 @@ type Column struct { typ string // row column type (used for Rows.Scan). indexes Indexes // linked indexes. foreign *ForeignKey // linked foreign-key. + Comment string // column comment. } // Expr represents a raw expression. It is used to distinguish between diff --git a/entc/gen/graph.go b/entc/gen/graph.go index 4d8105b2e..731b0d5b5 100644 --- a/entc/gen/graph.go +++ b/entc/gen/graph.go @@ -687,7 +687,7 @@ func (g *Graph) Tables() (all []*schema.Table, err error) { table.AddIndex(idx.Name, idx.Unique, idx.Columns) // Set the entsql.IndexAnnotation from the schema if exists. index, _ := table.Index(idx.Name) - index.Annotation = entsqlIndexAnnotate(idx.Annotations) + index.Annotation = sqlIndexAnnotate(idx.Annotations) } } if err := ensureUniqueFKs(tables); err != nil { diff --git a/entc/gen/template/migrate/schema.tmpl b/entc/gen/template/migrate/schema.tmpl index 48e767eaf..0bd52379e 100644 --- a/entc/gen/template/migrate/schema.tmpl +++ b/entc/gen/template/migrate/schema.tmpl @@ -36,6 +36,7 @@ var ( {{- if $c.Increment }} Increment: true,{{ end }} {{- if $c.Nullable }} Nullable: {{ $c.Nullable }},{{ end }} {{- with $c.Size }} Size: {{ . }},{{ end }} + {{- with $c.Comment }} Comment: "{{ $c.Comment }}",{{ end }} {{- with $c.Attr }} Attr: "{{ . }}",{{ end }} {{- with $c.Enums }} Enums: []string{ {{ range $e := . }}"{{ $e }}",{{ end }} },{{ end }} {{- if not (isNil $c.Default) -}} diff --git a/entc/gen/type.go b/entc/gen/type.go index 4218e2f4a..ce4453486 100644 --- a/entc/gen/type.go +++ b/entc/gen/type.go @@ -65,6 +65,7 @@ type ( Field struct { cfg *Config def *load.Field + typ *Type // Name is the name of this field in the database schema. Name string // Type holds the type information of the field. @@ -226,6 +227,7 @@ func NewType(c *Config, schema *load.Schema) (*Type, error) { fields: make(map[string]*Field, len(schema.Fields)), foreignKeys: make(map[string]struct{}), } + typ.ID.typ = typ if err := ValidSchemaName(typ.Name); err != nil { return nil, err } @@ -233,6 +235,7 @@ func NewType(c *Config, schema *load.Schema) (*Type, error) { tf := &Field{ cfg: c, def: f, + typ: typ, Name: f.Name, Type: f.Info, Unique: f.Unique, @@ -298,7 +301,7 @@ func (t Type) Table() string { // EntSQL returns the EntSQL annotation if exists. func (t Type) EntSQL() *entsql.Annotation { - return entsqlAnnotate(t.Annotations) + return sqlAnnotate(t.Annotations) } // Package returns the package name of this node. @@ -631,7 +634,7 @@ func (t *Type) AddIndex(idx *load.Index) error { if len(idx.Fields) == 0 && len(idx.Edges) == 0 { return errors.New("missing fields or edges") } - switch ant := entsqlIndexAnnotate(idx.Annotations); { + switch ant := sqlIndexAnnotate(idx.Annotations); { case ant == nil: case len(ant.PrefixColumns) != 0 && ant.Prefix != 0: return fmt.Errorf("index %q cannot contain both entsql.Prefix and entsql.PrefixColumn in annotation", index.Name) @@ -698,6 +701,7 @@ func (t *Type) setupFKs() error { fk := &ForeignKey{ Edge: e, Field: &Field{ + typ: owner, Name: builderField(e.Rel.Column()), Type: refid.Type, Nillable: true, @@ -1079,7 +1083,7 @@ func (f Field) Validator() string { // EntSQL returns the EntSQL annotation if exists. func (f Field) EntSQL() *entsql.Annotation { - return entsqlAnnotate(f.Annotations) + return sqlAnnotate(f.Annotations) } // mutMethods returns the method names of mutation interface. @@ -1367,6 +1371,7 @@ func (f Field) Column() *schema.Column { Nullable: f.Optional, Size: f.size(), Enums: f.EnumValues(), + Comment: f.sqlComment(), } switch { case f.Default && (f.Type.Numeric() || f.Type.Type == field.TypeBool): @@ -1428,6 +1433,7 @@ func (f Field) PK() *schema.Column { Name: f.StorageKey(), Type: f.Type.Type, Key: schema.PrimaryKey, + Comment: f.sqlComment(), Increment: f.incremental(f.Type.Type.Integer()), } // If the PK was defined by the user, and it is UUID or string. @@ -1460,6 +1466,23 @@ func (f Field) PK() *schema.Column { return c } +// sqlComment returns the SQL database comment for the field, if defined and enabled. +func (f Field) sqlComment() string { + fa, ta := f.EntSQL(), f.typ.EntSQL() + switch c := f.Comment(); { + // Field annotation gets precedence over type annotation. + case fa != nil && fa.WithComments != nil: + if *fa.WithComments { + return c + } + case ta != nil && ta.WithComments != nil: + if *ta.WithComments { + return c + } + } + return "" +} + // StorageKey returns the storage name of the field. // SQL column or Gremlin property. func (f Field) StorageKey() string { @@ -1895,7 +1918,7 @@ func (e Edge) StorageKey() (*edge.StorageKey, error) { // EntSQL returns the EntSQL annotation if exists. func (e Edge) EntSQL() *entsql.Annotation { - return entsqlAnnotate(e.Annotations) + return sqlAnnotate(e.Annotations) } // Column returns the first element from the columns slice. @@ -1976,8 +1999,8 @@ func fieldAnnotate(annotation map[string]any) *field.Annotation { return annotate } -// entsqlAnnotate extracts the entsql annotation from a loaded annotation format. -func entsqlAnnotate(annotation map[string]any) *entsql.Annotation { +// sqlAnnotate extracts the entsql.Annotation from a loaded annotation format. +func sqlAnnotate(annotation map[string]any) *entsql.Annotation { annotate := &entsql.Annotation{} if annotation == nil || annotation[annotate.Name()] == nil { return nil @@ -1988,8 +2011,8 @@ func entsqlAnnotate(annotation map[string]any) *entsql.Annotation { return annotate } -// entsqlIndexAnnotate extracts the entsql annotation from a loaded annotation format. -func entsqlIndexAnnotate(annotation map[string]any) *entsql.IndexAnnotation { +// sqlIndexAnnotate extracts the entsql annotation from a loaded annotation format. +func sqlIndexAnnotate(annotation map[string]any) *entsql.IndexAnnotation { annotate := &entsql.IndexAnnotation{} if annotation == nil || annotation[annotate.Name()] == nil { return nil diff --git a/entc/integration/migrate/entv2/media.go b/entc/integration/migrate/entv2/media.go index 69f588409..52221342f 100644 --- a/entc/integration/migrate/entv2/media.go +++ b/entc/integration/migrate/entv2/media.go @@ -21,9 +21,9 @@ type Media struct { ID int `json:"id,omitempty"` // Source holds the value of the "source" field. Source string `json:"source,omitempty"` - // SourceURI holds the value of the "source_uri" field. + // source_ui text SourceURI string `json:"source_uri,omitempty"` - // Text holds the value of the "text" field. + // media text Text string `json:"text,omitempty"` } diff --git a/entc/integration/migrate/entv2/migrate/schema.go b/entc/integration/migrate/entv2/migrate/schema.go index 5c9d167e9..b92a0bf83 100644 --- a/entc/integration/migrate/entv2/migrate/schema.go +++ b/entc/integration/migrate/entv2/migrate/schema.go @@ -90,7 +90,7 @@ var ( {Name: "id", Type: field.TypeInt, Increment: true}, {Name: "source", Type: field.TypeString, Nullable: true}, {Name: "source_uri", Type: field.TypeString, Nullable: true}, - {Name: "text", Type: field.TypeString, Nullable: true, Size: 2147483647}, + {Name: "text", Type: field.TypeString, Nullable: true, Size: 2147483647, Comment: "media text"}, } // MediaTable holds the schema information for the "media" table. MediaTable = &schema.Table{ diff --git a/entc/integration/migrate/entv2/schema/media.go b/entc/integration/migrate/entv2/schema/media.go index 04d2a4aa5..32d619bf1 100644 --- a/entc/integration/migrate/entv2/schema/media.go +++ b/entc/integration/migrate/entv2/schema/media.go @@ -23,9 +23,12 @@ func (Media) Fields() []ent.Field { field.String("source"). Optional(), field.String("source_uri"). - Optional(), + Optional(). + Comment("source_ui text"). + Annotations(entsql.WithComments(false)), field.Text("text"). - Optional(), + Optional(). + Comment("media text"), } } @@ -43,11 +46,10 @@ func (Media) Indexes() []ent.Index { func (Media) Annotations() []schema.Annotation { return []schema.Annotation{ - &entsql.Annotation{ - Check: "text <> 'boring'", - Checks: map[string]string{ - "boring_check": "source_uri <> 'entgo.io'", - }, - }, + entsql.WithComments(true), + entsql.Check("text <> 'boring'"), + entsql.Checks(map[string]string{ + "boring_check": "source_uri <> 'entgo.io'", + }), } } diff --git a/entc/integration/migrate/migrate_test.go b/entc/integration/migrate/migrate_test.go index 7fbb155bd..2cd0e02b0 100644 --- a/entc/integration/migrate/migrate_test.go +++ b/entc/integration/migrate/migrate_test.go @@ -29,6 +29,7 @@ import ( "entgo.io/ent/entc/integration/migrate/entv2/blog" "entgo.io/ent/entc/integration/migrate/entv2/conversion" "entgo.io/ent/entc/integration/migrate/entv2/customtype" + "entgo.io/ent/entc/integration/migrate/entv2/media" migratev2 "entgo.io/ent/entc/integration/migrate/entv2/migrate" "entgo.io/ent/entc/integration/migrate/entv2/predicate" "entgo.io/ent/entc/integration/migrate/entv2/user" @@ -72,6 +73,7 @@ func TestMySQL(t *testing.T) { } NicknameSearch(t, clientv2) TimePrecision(t, drv, "SELECT datetime_precision FROM information_schema.columns WHERE table_schema = 'migrate' AND table_name = ? AND column_name = ?") + ColumnComments(t, drv, "SELECT column_name as name, column_comment as comment FROM information_schema.columns WHERE table_schema = 'migrate' AND table_name = 'media' ORDER BY ordinal_position") require.NoError(t, err, root.Exec(ctx, "DROP DATABASE IF EXISTS versioned_migrate", []any{}, new(sql.Result))) require.NoError(t, root.Exec(ctx, "CREATE DATABASE IF NOT EXISTS versioned_migrate", []any{}, new(sql.Result))) @@ -139,10 +141,10 @@ func TestPostgres(t *testing.T) { DefaultExpr(t, drv, `SELECT column_default FROM information_schema.columns WHERE table_name = 'users' AND column_name = $1`, "lower('hello'::text)", "md5('ent'::text)") PKDefault(t, drv, `SELECT column_default FROM information_schema.columns WHERE table_name = 'zoos' AND column_name = $1`, "floor((random() * ((~ (1 << 31)))::double precision))") IndexOpClass(t, drv) + ColumnComments(t, drv, `SELECT column_name as name, col_description(table_name::regclass::oid, ordinal_position) as comment FROM information_schema.columns WHERE table_name = 'media' ORDER BY ordinal_position`) if version != "10" { IncludeColumns(t, drv) } - vdrv, err := sql.Open(dialect.Postgres, dsn+" dbname=versioned_migrate") require.NoError(t, err, "connecting to versioned migrate database") defer vdrv.Close() @@ -709,6 +711,20 @@ func JSONDefault(t *testing.T, drv *sql.Driver, query string) { require.NotEmpty(t, s) } +func ColumnComments(t *testing.T, drv *sql.Driver, query string) { + ctx := context.Background() + rows, err := drv.QueryContext(ctx, query) + require.NoError(t, err) + var n2c []struct{ Name, Comment string } + require.NoError(t, sql.ScanSlice(rows, &n2c)) + require.NoError(t, rows.Close()) + // 0 and 1 are "id" and "source". + require.Equal(t, media.FieldSourceURI, n2c[2].Name) + require.Empty(t, n2c[2].Comment, "source_uri is disabled using annotation") + require.Equal(t, media.FieldText, n2c[3].Name) + require.Equal(t, "media text", n2c[3].Comment) +} + func DefaultExpr(t *testing.T, drv *sql.Driver, query string, expected1, expected2 string) { ctx := context.Background() rows, err := drv.QueryContext(ctx, query, user.FieldDefaultExpr)