Repository: ariga/atlas
Pull request #591 (2022-02-18 00:14:44)

fixed sqlite escaping on migrate insert


Patch:
diff --git a/doc/md/ddl/sql.mdx b/doc/md/ddl/sql.mdx
index 6a0641a4..f22ace83 100644
--- a/doc/md/ddl/sql.mdx
+++ b/doc/md/ddl/sql.mdx
@@ -99,8 +99,8 @@ table "users" {
   foreign_key "manager_fk" {
     columns = [column.manager_id]
     ref_columns = [column.id]
-    on_delete = "CASCADE"
-    on_update = "NO ACTION"
+    on_delete = CASCADE
+    on_update = NO_ACTION
   }
 }
 ```
@@ -190,8 +190,8 @@ as references to columns in other tables.
 foreign_key "manager_fk" {
   columns = [column.manager_id]
   ref_columns = [column.id]
-  on_delete = "CASCADE"
-  on_update = "NO ACTION"
+  on_delete = CASCADE
+  on_update = NO_ACTION
 }
 ```
 
diff --git a/doc/md/getting-started/02-inspecting.mdx b/doc/md/getting-started/02-inspecting.mdx
index fb0b836b..6b84af17 100644
--- a/doc/md/getting-started/02-inspecting.mdx
+++ b/doc/md/getting-started/02-inspecting.mdx
@@ -148,8 +148,8 @@ table "blog_posts" {
   foreign_key "blog_posts_ibfk_1" {
     columns     = [column.author_id]
     ref_columns = [table.users.column.id]
-    on_update   = "NO ACTION"
-    on_delete   = "NO ACTION"
+    on_update   = NO_ACTION
+    on_delete   = NO_ACTION
   }
   index "author_id" {
     unique  = false
diff --git a/doc/md/getting-started/04-ui.md b/doc/md/getting-started/04-ui.md
index dd507392..58dbbcc9 100644
--- a/doc/md/getting-started/04-ui.md
+++ b/doc/md/getting-started/04-ui.md
@@ -60,14 +60,14 @@ table "likes" {
  foreign_key "like_blog_post" {
    columns     = [table.likes.column.blog_post_id]
    ref_columns = [table.blog_posts.column.id]
-   on_update   = "RESTRICT"
-   on_delete   = "RESTRICT"
+   on_update   = RESTRICT
+   on_delete   = RESTRICT
  }
  foreign_key "like_user" {
    columns     = [table.likes.column.user_id]
    ref_columns = [table.users.column.id]
-   on_update   = "RESTRICT"
-   on_delete   = "RESTRICT"
+   on_update   = RESTRICT
+   on_delete   = RESTRICT
  }
  index "like_blog_post" {
    columns = [table.likes.column.blog_post_id]
diff --git a/schema/schemaspec/extension.go b/schema/schemaspec/extension.go
index 56597d27..a2abfafd 100644
--- a/schema/schemaspec/extension.go
+++ b/schema/schemaspec/extension.go
@@ -294,19 +294,31 @@ func setPtr(field reflect.Value, val Value) error {
 	if field.IsNil() {
 		field.Set(reflect.New(field.Type().Elem()))
 	}
-	switch e := field.Elem(); e.Kind() {
-	case reflect.Bool:
+	switch e := field.Interface().(type) {
+	case *bool:
 		b, err := BoolVal(val)
 		if err != nil {
 			return err
 		}
-		e.SetBool(b)
-	case reflect.String:
+		*e = b
+	case *string:
+		s, err := StrVal(val)
+		if err != nil {
+			return err
+		}
+		*e = s
+	case *LiteralValue:
+		s, err := StrVal(val)
+		if err != nil {
+			return err
+		}
+		e.V = s
+	case *Ref:
 		s, err := StrVal(val)
 		if err != nil {
 			return err
 		}
-		e.SetString(s)
+		e.V = s
 	default:
 		return fmt.Errorf("unhandled pointer type %T", val)
 	}
diff --git a/sql/internal/specutil/convert.go b/sql/internal/specutil/convert.go
index 9dcfa97a..5427d9fc 100644
--- a/sql/internal/specutil/convert.go
+++ b/sql/internal/specutil/convert.go
@@ -234,11 +234,12 @@ func PrimaryKey(spec *sqlspec.PrimaryKey, parent *schema.Table) (*schema.Index,
 // referenced by the FK definitions in the spec.
 func LinkForeignKeys(tbl *schema.Table, sch *schema.Schema, table *sqlspec.Table) error {
 	for _, spec := range table.ForeignKeys {
-		fk := &schema.ForeignKey{
-			Symbol:   spec.Symbol,
-			Table:    tbl,
-			OnUpdate: spec.OnUpdate,
-			OnDelete: spec.OnDelete,
+		fk := &schema.ForeignKey{Symbol: spec.Symbol, Table: tbl}
+		if spec.OnUpdate != nil {
+			fk.OnUpdate = schema.ReferenceOption(FromVar(spec.OnUpdate.V))
+		}
+		if spec.OnDelete != nil {
+			fk.OnDelete = schema.ReferenceOption(FromVar(spec.OnDelete.V))
 		}
 		if n, m := len(spec.Columns), len(spec.RefColumns); n != m {
 			return fmt.Errorf("sqlspec: number of referencing and referenced columns do not match for foreign-key %q", fk.Symbol)
@@ -475,8 +476,8 @@ func FromForeignKey(s *schema.ForeignKey) (*sqlspec.ForeignKey, error) {
 		Symbol:     s.Symbol,
 		Columns:    c,
 		RefColumns: r,
-		OnDelete:   s.OnDelete,
-		OnUpdate:   s.OnUpdate,
+		OnUpdate:   &schemaspec.Ref{V: Var(string(s.OnUpdate))},
+		OnDelete:   &schemaspec.Ref{V: Var(string(s.OnDelete))},
 	}, nil
 }
 
@@ -582,3 +583,20 @@ func convertCommentFromSchema(src []schema.Attr, trgt *[]*schemaspec.Attr) {
 		*trgt = append(*trgt, StrAttr("comment", c.Text))
 	}
 }
+
+// ReferenceVars holds the HCL variables
+// for foreign keys' referential-actions.
+var ReferenceVars = []string{
+	Var(string(schema.NoAction)),
+	Var(string(schema.Restrict)),
+	Var(string(schema.Cascade)),
+	Var(string(schema.SetNull)),
+	Var(string(schema.SetDefault)),
+}
+
+// Var formats a string as variable to make it HCL compatible.
+// The result is simple, replace each space with underscore.
+func Var(s string) string { return strings.ReplaceAll(s, " ", "_") }
+
+// FromVar is the inverse function of Var.
+func FromVar(s string) string { return strings.ReplaceAll(s, "_", " ") }
diff --git a/sql/mysql/sqlspec.go b/sql/mysql/sqlspec.go
index 576afd65..a98e197d 100644
--- a/sql/mysql/sqlspec.go
+++ b/sql/mysql/sqlspec.go
@@ -67,6 +67,8 @@ var (
 	hclState = schemahcl.New(
 		schemahcl.WithTypes(TypeRegistry.Specs()),
 		schemahcl.WithScopedEnums("table.index.type", IndexTypeBTree, IndexTypeHash, IndexTypeFullText, IndexTypeSpatial),
+		schemahcl.WithScopedEnums("table.foreign_key.on_update", specutil.ReferenceVars...),
+		schemahcl.WithScopedEnums("table.foreign_key.on_delete", specutil.ReferenceVars...),
 	)
 	// UnmarshalHCL unmarshals an Atlas HCL DDL document into v.
 	UnmarshalHCL = schemaspec.UnmarshalerFunc(func(bytes []byte, i interface{}) error {
@@ -208,7 +210,8 @@ func indexSpec(idx *schema.Index) (*sqlspec.Index, error) {
 	if err != nil {
 		return nil, err
 	}
-	if i := (IndexType{}); sqlx.Has(idx.Attrs, &i) {
+	// Avoid printing the index type if it is the default.
+	if i := (IndexType{}); sqlx.Has(idx.Attrs, &i) && i.T != IndexTypeBTree {
 		spec.Extra.Attrs = append(spec.Extra.Attrs, specutil.VarAttr("type", strings.ToUpper(i.T)))
 	}
 	return spec, nil
diff --git a/sql/postgres/sqlspec.go b/sql/postgres/sqlspec.go
index 457eddbf..8a161074 100644
--- a/sql/postgres/sqlspec.go
+++ b/sql/postgres/sqlspec.go
@@ -95,6 +95,8 @@ var (
 		schemahcl.WithTypes(TypeRegistry.Specs()),
 		schemahcl.WithScopedEnums("table.index.type", IndexTypeBTree, IndexTypeHash, IndexTypeGIN, IndexTypeGiST),
 		schemahcl.WithScopedEnums("table.column.identity.generated", GeneratedTypeAlways, GeneratedTypeByDefault),
+		schemahcl.WithScopedEnums("table.foreign_key.on_update", specutil.ReferenceVars...),
+		schemahcl.WithScopedEnums("table.foreign_key.on_delete", specutil.ReferenceVars...),
 	)
 	// UnmarshalHCL unmarshals an Atlas HCL DDL document into v.
 	UnmarshalHCL = schemaspec.UnmarshalerFunc(func(bytes []byte, i interface{}) error {
@@ -203,7 +205,7 @@ func convertIdentity(r *schemaspec.Resource) (*Identity, error) {
 	if err := r.As(&spec); err != nil {
 		return nil, err
 	}
-	id := &Identity{Generation: fromVar(spec.Generation), Sequence: &Sequence{}}
+	id := &Identity{Generation: specutil.FromVar(spec.Generation), Sequence: &Sequence{}}
 	if spec.Start != 0 {
 		id.Sequence.Start = spec.Start
 	}
@@ -361,7 +363,8 @@ func indexSpec(idx *schema.Index) (*sqlspec.Index, error) {
 	if err != nil {
 		return nil, err
 	}
-	if i := (IndexType{}); sqlx.Has(idx.Attrs, &i) {
+	// Avoid printing the index type if it is the default.
+	if i := (IndexType{}); sqlx.Has(idx.Attrs, &i) && i.T != IndexTypeBTree {
 		spec.Extra.Attrs = append(spec.Extra.Attrs, specutil.VarAttr("type", strings.ToUpper(i.T)))
 	}
 	return spec, nil
@@ -384,7 +387,7 @@ func fromIdentity(i *Identity) *schemaspec.Resource {
 	id := &schemaspec.Resource{
 		Type: "identity",
 		Attrs: []*schemaspec.Attr{
-			specutil.VarAttr("generated", strings.ToUpper(asVar(i.Generation))),
+			specutil.VarAttr("generated", strings.ToUpper(specutil.Var(i.Generation))),
 		},
 	}
 	if s := i.Sequence; s != nil {
@@ -525,8 +528,3 @@ func attr(typ *schemaspec.Type, key string) (*schemaspec.Attr, bool) {
 	}
 	return nil, false
 }
-
-// asVar and fromVar formats a string as variable to make it HCL compatible.
-// The result is simple, replace each space with underscore.
-func asVar(s string) string   { return strings.ReplaceAll(s, " ", "_") }
-func fromVar(s string) string { return strings.ReplaceAll(s, "_", " ") }
diff --git a/sql/sqlite/migrate.go b/sql/sqlite/migrate.go
index 682e741c..3ac352e7 100644
--- a/sql/sqlite/migrate.go
+++ b/sql/sqlite/migrate.go
@@ -344,7 +344,7 @@ func (s *state) copyRows(from *schema.Table, to *schema.Table, changes []schema.
 			fromC = append(fromC, column.Name)
 		}
 	}
-	stmt := fmt.Sprintf("INSERT INTO %s (%s) SELECT %s FROM %s", to.Name, strings.Join(toC, ", "), strings.Join(fromC, ", "), from.Name)
+	stmt := fmt.Sprintf("INSERT INTO `%s` (%s) SELECT %s FROM `%s`", to.Name, identComma(toC), identComma(fromC), from.Name)
 	s.append(&migrate.Change{
 		Cmd:     stmt,
 		Args:    args,
@@ -470,3 +470,15 @@ func defaultValue(c *schema.Column) (string, error) {
 		return "", fmt.Errorf("unexpected default value type: %T", x)
 	}
 }
+
+func identComma(c []string) string {
+	b := &sqlx.Builder{QuoteChar: '`'}
+	b.MapComma(c, func(i int, b *sqlx.Builder) {
+		if strings.ContainsRune(c[i], '`') {
+			b.WriteString(c[i])
+		} else {
+			b.Ident(c[i])
+		}
+	})
+	return b.String()
+}
diff --git a/sql/sqlite/sqlspec.go b/sql/sqlite/sqlspec.go
index 8e364ff2..8b178b9d 100644
--- a/sql/sqlite/sqlspec.go
+++ b/sql/sqlite/sqlspec.go
@@ -127,7 +127,11 @@ var TypeRegistry = specutil.NewRegistry(
 )
 
 var (
-	hclState = schemahcl.New(schemahcl.WithTypes(TypeRegistry.Specs()))
+	hclState = schemahcl.New(
+		schemahcl.WithTypes(TypeRegistry.Specs()),
+		schemahcl.WithScopedEnums("table.foreign_key.on_update", specutil.ReferenceVars...),
+		schemahcl.WithScopedEnums("table.foreign_key.on_delete", specutil.ReferenceVars...),
+	)
 	// UnmarshalHCL unmarshals an Atlas HCL DDL document into v.
 	UnmarshalHCL = schemaspec.UnmarshalerFunc(func(bytes []byte, i interface{}) error {
 		return UnmarshalSpec(bytes, hclState, i)
diff --git a/sql/sqlspec/sqlspec.go b/sql/sqlspec/sqlspec.go
index a3bd7eda..18142754 100644
--- a/sql/sqlspec/sqlspec.go
+++ b/sql/sqlspec/sqlspec.go
@@ -2,7 +2,6 @@ package sqlspec
 
 import (
 	"ariga.io/atlas/schema/schemaspec"
-	"ariga.io/atlas/sql/schema"
 )
 
 type (
@@ -66,11 +65,11 @@ type (
 
 	// ForeignKey holds a specification for the Foreign key of a table.
 	ForeignKey struct {
-		Symbol     string                 `spec:",name"`
-		Columns    []*schemaspec.Ref      `spec:"columns"`
-		RefColumns []*schemaspec.Ref      `spec:"ref_columns"`
-		OnUpdate   schema.ReferenceOption `spec:"on_update"`
-		OnDelete   schema.ReferenceOption `spec:"on_delete"`
+		Symbol     string            `spec:",name"`
+		Columns    []*schemaspec.Ref `spec:"columns"`
+		RefColumns []*schemaspec.Ref `spec:"ref_columns"`
+		OnUpdate   *schemaspec.Ref   `spec:"on_update"`
+		OnDelete   *schemaspec.Ref   `spec:"on_delete"`
 		schemaspec.DefaultExtension
 	}
 


Test patch:
diff --git a/internal/integration/hclsqlspec/hclsqlspec_test.go b/internal/integration/hclsqlspec/hclsqlspec_test.go
index 89138ed5..033f93e9 100644
--- a/internal/integration/hclsqlspec/hclsqlspec_test.go
+++ b/internal/integration/hclsqlspec/hclsqlspec_test.go
@@ -185,7 +185,7 @@ table "accounts" {
 								V: "$table.accounts.$column.active",
 							},
 						},
-						OnDelete: schema.SetNull,
+						OnDelete: &schemaspec.Ref{V: string(schema.SetNull)},
 					},
 				},
 			},
@@ -258,7 +258,7 @@ table "accounts" {
 								V: "$table.users.$column.active",
 							},
 						},
-						OnDelete: schema.SetNull,
+						OnDelete: &schemaspec.Ref{V: string(schema.SetNull)},
 					},
 				},
 			},
diff --git a/internal/integration/testdata/mysql/foreign-key-add.txt b/internal/integration/testdata/mysql/foreign-key-add.txt
index 368da449..88368a22 100644
--- a/internal/integration/testdata/mysql/foreign-key-add.txt
+++ b/internal/integration/testdata/mysql/foreign-key-add.txt
@@ -63,7 +63,7 @@ table "posts" {
     foreign_key "owner_id" {
         columns = [table.posts.column.author_id]
         ref_columns = [table.users.column.id]
-        on_delete = "SET NULL"
+        on_delete = SET_NULL
     }
 }
 
@@ -97,7 +97,7 @@ table "posts" {
     foreign_key "owner_id" {
     	columns = [table.posts.column.author_id]
     	ref_columns = [table.users.column.id]
-    	on_update = "SET NULL"
+    	on_update = SET_NULL
     }
 }
 
@@ -132,7 +132,7 @@ table "posts" {
     foreign_key "owner_id" {
     	columns = [table.posts.column.author_id]
     	ref_columns = [table.users.column.id]
-    	on_delete = "SET NULL"
+    	on_delete = SET_NULL
     }
 }
 
diff --git a/internal/integration/testdata/mysql/foreign-key-modify-action.txt b/internal/integration/testdata/mysql/foreign-key-modify-action.txt
index 8b07051d..7e45dd6a 100644
--- a/internal/integration/testdata/mysql/foreign-key-modify-action.txt
+++ b/internal/integration/testdata/mysql/foreign-key-modify-action.txt
@@ -40,7 +40,7 @@ table "posts" {
     foreign_key "owner_id" {
     	columns = [table.posts.column.author_id]
     	ref_columns = [table.users.column.id]
-    	on_update = "SET NULL"
+    	on_update = SET_NULL
     }
 }
 
@@ -101,8 +101,8 @@ table "posts" {
     foreign_key "owner_id" {
     	columns = [table.posts.column.author_id]
     	ref_columns = [table.users.column.id]
-    	on_update = "NO ACTION"
-    	on_delete = "CASCADE"
+    	on_update = NO_ACTION
+    	on_delete = CASCADE
     }
 }
 
diff --git a/sql/internal/specutil/convert_test.go b/sql/internal/specutil/convert_test.go
index cf6a7bff..814d636e 100644
--- a/sql/internal/specutil/convert_test.go
+++ b/sql/internal/specutil/convert_test.go
@@ -61,6 +61,8 @@ func TestFromForeignKey(t *testing.T) {
 		Columns:    tbl.Columns[1:],
 		RefTable:   tbl,
 		RefColumns: tbl.Columns[:1],
+		OnUpdate:   schema.NoAction,
+		OnDelete:   schema.Cascade,
 	}
 	key, err := FromForeignKey(fk)
 	require.NoError(t, err)
@@ -72,5 +74,7 @@ func TestFromForeignKey(t *testing.T) {
 		RefColumns: []*schemaspec.Ref{
 			{V: "$column.id"},
 		},
+		OnUpdate: &schemaspec.Ref{V: "NO_ACTION"},
+		OnDelete: &schemaspec.Ref{V: "CASCADE"},
 	}, key)
 }
diff --git a/sql/mysql/sqlspec_test.go b/sql/mysql/sqlspec_test.go
index 7d9530ab..3df3add7 100644
--- a/sql/mysql/sqlspec_test.go
+++ b/sql/mysql/sqlspec_test.go
@@ -59,7 +59,7 @@ table "table" {
 		ref_columns = [
 			table.accounts.column.name,
 		]
-		on_delete = "SET NULL"
+		on_delete = SET_NULL
 	}
 	check "positive price" {
 		expr = "price1 > 0"
diff --git a/sql/postgres/sqlspec_test.go b/sql/postgres/sqlspec_test.go
index 4a4d4769..f27fb1a5 100644
--- a/sql/postgres/sqlspec_test.go
+++ b/sql/postgres/sqlspec_test.go
@@ -65,7 +65,7 @@ table "table" {
 		ref_columns = [
 			table.accounts.column.name,
 		]
-		on_delete = "SET NULL"
+		on_delete = SET_NULL
 	}
 	check "positive price" {
 		expr = "price > 0"
diff --git a/sql/sqlite/migrate_test.go b/sql/sqlite/migrate_test.go
index 073cb3ff..2a659c54 100644
--- a/sql/sqlite/migrate_test.go
+++ b/sql/sqlite/migrate_test.go
@@ -157,7 +157,7 @@ func TestPlanChanges(t *testing.T) {
 				Changes: []*migrate.Change{
 					{Cmd: "PRAGMA foreign_keys = off"},
 					{Cmd: "CREATE TABLE `new_users` (`id` bigint NOT NULL, CHECK (id <> 0))", Reverse: "DROP TABLE `new_users`"},
-					{Cmd: "INSERT INTO new_users (id) SELECT id FROM users"},
+					{Cmd: "INSERT INTO `new_users` (`id`) SELECT `id` FROM `users`"},
 					{Cmd: "DROP TABLE `users`"},
 					{Cmd: "ALTER TABLE `new_users` RENAME TO `users`"},
 					{Cmd: "PRAGMA foreign_keys = on"},
diff --git a/sql/sqlite/sqlspec_test.go b/sql/sqlite/sqlspec_test.go
index 045ff6ca..d4f00715 100644
--- a/sql/sqlite/sqlspec_test.go
+++ b/sql/sqlite/sqlspec_test.go
@@ -49,7 +49,7 @@ table "table" {
 		ref_columns = [
 			table.accounts.column.name,
 		]
-		on_delete = "SET NULL"
+		on_delete = SET_NULL
 	}
 	check "positive price" {
 		expr = "price > 0"