From 579080de541f32ce29ffcabe258898aa04aa6bf0 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Fri, 2 Feb 2024 11:31:06 +0100 Subject: [PATCH 1/7] Migrate table constraint create --- pkg/helpers/helpers.go | 9 -- pkg/resources/table_constraint.go | 104 +++++++++++++------- pkg/sdk/common_table_types.go | 55 ++++++++++- pkg/sdk/common_table_types_test.go | 148 +++++++++++++++++++++++++++++ pkg/snowflake/table_constraint.go | 57 +---------- 5 files changed, 274 insertions(+), 99 deletions(-) create mode 100644 pkg/sdk/common_table_types_test.go diff --git a/pkg/helpers/helpers.go b/pkg/helpers/helpers.go index 5893b97f2a..d5a04ac4ba 100644 --- a/pkg/helpers/helpers.go +++ b/pkg/helpers/helpers.go @@ -29,15 +29,6 @@ func ListToSnowflakeString(list []string) string { return fmt.Sprintf("%v", strings.Join(list, ", ")) } -// IPListToString formats a list of IPs into a Snowflake-DDL friendly string, e.g. ('192.168.1.0', '192.168.1.100'). -func IPListToSnowflakeString(ips []string) string { - for index, element := range ips { - ips[index] = fmt.Sprintf(`'%v'`, element) - } - - return fmt.Sprintf("(%v)", strings.Join(ips, ", ")) -} - // ListContentToString strips list elements of double quotes or brackets. func ListContentToString(listString string) string { re := regexp.MustCompile(`[\"\[\]]`) diff --git a/pkg/resources/table_constraint.go b/pkg/resources/table_constraint.go index 5cbb79d8a3..caf4f2a7c4 100644 --- a/pkg/resources/table_constraint.go +++ b/pkg/resources/table_constraint.go @@ -1,13 +1,16 @@ package resources import ( + "context" "database/sql" "fmt" - "log" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers" "strings" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/snowflake" snowflakeValidation "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/validation" + + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/snowflake" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" ) @@ -21,9 +24,9 @@ var tableConstraintSchema = map[string]*schema.Schema{ "type": { Type: schema.TypeString, Required: true, - Description: "Type of constraint, one of 'UNIQUE', 'PRIMARY KEY', 'FOREIGN KEY', or 'NOT NULL'", + Description: "Type of constraint, one of 'UNIQUE', 'PRIMARY KEY', or 'FOREIGN KEY'", ForceNew: true, - ValidateFunc: validation.StringInSlice([]string{"UNIQUE", "PRIMARY KEY", "FOREIGN KEY", "NOT NULL"}, false), + ValidateFunc: validation.StringInSlice([]string{"UNIQUE", "PRIMARY KEY", "FOREIGN KEY"}, false), DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { return strings.EqualFold(old, new) }, @@ -35,7 +38,7 @@ var tableConstraintSchema = map[string]*schema.Schema{ Type: schema.TypeString, Required: true, ForceNew: true, - Description: "Idenfifier for table to create constraint on. Must be of the form Note: format must follow: \"\".\"\".\"\" or \"..\" or \"|.\" (snowflake_table.my_table.id)", + Description: "Identifier for table to create constraint on. Format must follow: \"\"\".\"\".\"\"\" or \"..\" (snowflake_table.my_table.id)", }, "columns": { Type: schema.TypeList, @@ -93,8 +96,8 @@ var tableConstraintSchema = map[string]*schema.Schema{ "comment": { Type: schema.TypeString, Optional: true, - ForceNew: true, Description: "Comment for the table constraint", + Deprecated: "Not used. Will be removed.", }, "foreign_key_properties": { Type: schema.TypeList, @@ -204,79 +207,114 @@ func (v *tableConstraintID) parse(s string) { // CreateTableConstraint implements schema.CreateFunc. func CreateTableConstraint(d *schema.ResourceData, meta interface{}) error { db := meta.(*sql.DB) + ctx := context.Background() + client := sdk.NewClientFromDB(db) + name := d.Get("name").(string) - constraintType := d.Get("type").(string) + cType := d.Get("type").(string) tableID := d.Get("table_id").(string) - formattedTableID := snowflakeValidation.ParseAndFormatFullyQualifiedObectID(tableID) - builder := snowflake.NewTableConstraintBuilder(name, constraintType, formattedTableID) + objectIdentifier, err := helpers.DecodeSnowflakeParameterID(tableID) + if err != nil { + return fmt.Errorf("table id is incorrect: %s, err: %s", objectIdentifier, err) + } + tableIdentifier, ok := objectIdentifier.(sdk.SchemaObjectIdentifier) + if !ok { + return fmt.Errorf("table id is incorrect: %s", objectIdentifier) + } + + constraintType, err := sdk.ToColumnConstraintType(cType) + if err != nil { + return err + } + constraintRequest := sdk.NewOutOfLineConstraintRequest(constraintType) cc := d.Get("columns").([]interface{}) columns := make([]string, 0, len(cc)) for _, c := range cc { columns = append(columns, c.(string)) } - builder.WithColumns(columns) + constraintRequest.WithColumns(columns) - // set optionals if v, ok := d.GetOk("enforced"); ok { - builder.WithEnforced(v.(bool)) + constraintRequest.WithEnforced(sdk.Bool(v.(bool))) } if v, ok := d.GetOk("deferrable"); ok { - builder.WithDeferrable(v.(bool)) + constraintRequest.WithDeferrable(sdk.Bool(v.(bool))) } if v, ok := d.GetOk("initially"); ok { - builder.WithInitially(v.(string)) + if v.(string) == "DEFERRED" { + constraintRequest.WithInitiallyDeferred(sdk.Bool(true)) + } else { + constraintRequest.WithInitiallyImmediate(sdk.Bool(true)) + } } if v, ok := d.GetOk("enable"); ok { - builder.WithEnable(v.(bool)) + constraintRequest.WithEnable(sdk.Bool(v.(bool))) } if v, ok := d.GetOk("validate"); ok { - builder.WithValidate(v.(bool)) + constraintRequest.WithValidate(sdk.Bool(v.(bool))) } if v, ok := d.GetOk("rely"); ok { - builder.WithRely(v.(bool)) - } - - if v, ok := d.GetOk("comment"); ok { - builder.WithComment(v.(string)) + constraintRequest.WithRely(sdk.Bool(v.(bool))) } // set foreign key specific settings if v, ok := d.GetOk("foreign_key_properties"); ok { foreignKeyProperties := v.([]interface{})[0].(map[string]interface{}) - builder.WithMatch(foreignKeyProperties["match"].(string)) - builder.WithUpdate(foreignKeyProperties["on_update"].(string)) - builder.WithDelete(foreignKeyProperties["on_delete"].(string)) references := foreignKeyProperties["references"].([]interface{})[0].(map[string]interface{}) fkTableID := references["table_id"].(string) - formattedFkTableID := snowflakeValidation.ParseAndFormatFullyQualifiedObectID(fkTableID) - builder.WithReferenceTableID(formattedFkTableID) - log.Printf("reference table id : %s", formattedFkTableID) + fkId, err := helpers.DecodeSnowflakeParameterID(fkTableID) + if err != nil { + return fmt.Errorf("table id is incorrect: %s, err: %s", objectIdentifier, err) + } + referencedTableIdentifier, ok := fkId.(sdk.SchemaObjectIdentifier) + if !ok { + return fmt.Errorf("table id is incorrect: %s", objectIdentifier) + } + cols := references["columns"].([]interface{}) var fkColumns []string for _, c := range cols { fkColumns = append(fkColumns, c.(string)) } - builder.WithReferenceColumns(fkColumns) + foreignKeyRequest := sdk.NewOutOfLineForeignKeyRequest(referencedTableIdentifier, fkColumns) + + matchType, err := sdk.ToMatchType(foreignKeyProperties["match"].(string)) + if err != nil { + return err + } + foreignKeyRequest.WithMatch(&matchType) + + onUpdate, err := sdk.ToForeignKeyAction(foreignKeyProperties["on_update"].(string)) + if err != nil { + return err + } + onDelete, err := sdk.ToForeignKeyAction(foreignKeyProperties["on_delete"].(string)) + if err != nil { + return err + } + foreignKeyRequest.WithOn(sdk.NewForeignKeyOnAction(). + WithOnDelete(&onDelete). + WithOnUpdate(&onUpdate), + ) + constraintRequest.WithForeignKey(foreignKeyRequest) } - stmt := builder.Create() - log.Printf("[DEBUG] create table constraint statement: %v\n", stmt) - result, err := db.Exec(stmt) + alterStatement := sdk.NewAlterTableRequest(tableIdentifier).WithConstraintAction(sdk.NewTableConstraintActionRequest().WithAdd(constraintRequest)) + err = client.Tables.Alter(ctx, alterStatement) if err != nil { return fmt.Errorf("error creating table constraint %v err = %w", name, err) } - log.Printf("[DEBUG] result: %v\n", result) tc := tableConstraintID{ name, - constraintType, + cType, tableID, } d.SetId(tc.String()) diff --git a/pkg/sdk/common_table_types.go b/pkg/sdk/common_table_types.go index 0334095411..f84a2ddef1 100644 --- a/pkg/sdk/common_table_types.go +++ b/pkg/sdk/common_table_types.go @@ -1,6 +1,10 @@ package sdk -import "errors" +import ( + "errors" + "fmt" + "strings" +) type TableRowAccessPolicy struct { rowAccessPolicy bool `ddl:"static" sql:"ROW ACCESS POLICY"` @@ -76,6 +80,21 @@ const ( ColumnConstraintTypeForeignKey ColumnConstraintType = "FOREIGN KEY" ) +func ToColumnConstraintType(s string) (ColumnConstraintType, error) { + cType := strings.ToUpper(s) + + switch cType { + case string(ColumnConstraintTypeUnique): + return ColumnConstraintTypeUnique, nil + case string(ColumnConstraintTypePrimaryKey): + return ColumnConstraintTypePrimaryKey, nil + case string(ColumnConstraintTypeForeignKey): + return ColumnConstraintTypeForeignKey, nil + } + + return "", fmt.Errorf("invalid column constraint type: %s", s) +} + type InlineForeignKey struct { TableName string `ddl:"keyword" sql:"REFERENCES"` ColumnName []string `ddl:"keyword,parentheses"` @@ -99,6 +118,21 @@ var ( PartialMatchType MatchType = "PARTIAL" ) +func ToMatchType(s string) (MatchType, error) { + cType := strings.ToUpper(s) + + switch cType { + case string(FullMatchType): + return FullMatchType, nil + case string(SimpleMatchType): + return SimpleMatchType, nil + case string(PartialMatchType): + return PartialMatchType, nil + } + + return "", fmt.Errorf("invalid match type: %s", s) +} + type ForeignKeyOnAction struct { OnUpdate *ForeignKeyAction `ddl:"parameter,no_equals" sql:"ON UPDATE"` OnDelete *ForeignKeyAction `ddl:"parameter,no_equals" sql:"ON DELETE"` @@ -113,3 +147,22 @@ const ( ForeignKeyRestrictAction ForeignKeyAction = "RESTRICT" ForeignKeyNoAction ForeignKeyAction = "NO ACTION" ) + +func ToForeignKeyAction(s string) (ForeignKeyAction, error) { + cType := strings.ToUpper(s) + + switch cType { + case string(ForeignKeyCascadeAction): + return ForeignKeyCascadeAction, nil + case string(ForeignKeySetNullAction): + return ForeignKeySetNullAction, nil + case string(ForeignKeySetDefaultAction): + return ForeignKeySetDefaultAction, nil + case string(ForeignKeyRestrictAction): + return ForeignKeyRestrictAction, nil + case string(ForeignKeyNoAction): + return ForeignKeyNoAction, nil + } + + return "", fmt.Errorf("invalid column constraint type: %s", s) +} diff --git a/pkg/sdk/common_table_types_test.go b/pkg/sdk/common_table_types_test.go new file mode 100644 index 0000000000..70d2289d86 --- /dev/null +++ b/pkg/sdk/common_table_types_test.go @@ -0,0 +1,148 @@ +package sdk + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_ToColumnConstraintType(t *testing.T) { + type test struct { + input string + want ColumnConstraintType + } + + positiveTests := []test{ + {input: "UNIQUE", want: ColumnConstraintTypeUnique}, + {input: "unique", want: ColumnConstraintTypeUnique}, + {input: "uNiQuE", want: ColumnConstraintTypeUnique}, + {input: "PRIMARY KEY", want: ColumnConstraintTypePrimaryKey}, + {input: "primary key", want: ColumnConstraintTypePrimaryKey}, + {input: "PRIMARY key", want: ColumnConstraintTypePrimaryKey}, + {input: "FOREIGN KEY", want: ColumnConstraintTypeForeignKey}, + {input: "foreign key", want: ColumnConstraintTypeForeignKey}, + {input: "foreign KEY", want: ColumnConstraintTypeForeignKey}, + } + + negativeTests := []test{ + {input: "foreign key "}, + {input: "not null"}, + {input: "NOT NULL"}, + {input: "abc"}, + {input: ""}, + } + + for _, tc := range positiveTests { + tc := tc + t.Run(tc.input, func(t *testing.T) { + got, err := ToColumnConstraintType(tc.input) + require.NoError(t, err) + require.Equal(t, tc.want, got) + }) + } + + for _, tc := range negativeTests { + tc := tc + t.Run(tc.input, func(t *testing.T) { + got, err := ToColumnConstraintType(tc.input) + require.Error(t, err) + require.Empty(t, got) + }) + } +} + +func Test_ToMatchType(t *testing.T) { + type test struct { + input string + want MatchType + } + + positiveTests := []test{ + {input: string(FullMatchType), want: FullMatchType}, + {input: "FULL", want: FullMatchType}, + {input: "full", want: FullMatchType}, + {input: string(SimpleMatchType), want: SimpleMatchType}, + {input: "SIMPLE", want: SimpleMatchType}, + {input: "simple", want: SimpleMatchType}, + {input: string(PartialMatchType), want: PartialMatchType}, + {input: "PARTIAL", want: PartialMatchType}, + {input: "partial", want: PartialMatchType}, + } + + negativeTests := []test{ + {input: "full "}, + {input: " PARTIAL"}, + {input: "NOT NULL"}, + {input: "abc"}, + {input: ""}, + } + + for _, tc := range positiveTests { + tc := tc + t.Run(tc.input, func(t *testing.T) { + got, err := ToMatchType(tc.input) + require.NoError(t, err) + require.Equal(t, tc.want, got) + }) + } + + for _, tc := range negativeTests { + tc := tc + t.Run(tc.input, func(t *testing.T) { + got, err := ToMatchType(tc.input) + require.Error(t, err) + require.Empty(t, got) + }) + } +} + +func Test_ToForeignKeyAction(t *testing.T) { + type test struct { + input string + want ForeignKeyAction + } + + positiveTests := []test{ + {input: string(ForeignKeyCascadeAction), want: ForeignKeyCascadeAction}, + {input: "CASCADE", want: ForeignKeyCascadeAction}, + {input: "cascade", want: ForeignKeyCascadeAction}, + {input: string(ForeignKeySetNullAction), want: ForeignKeySetNullAction}, + {input: "SET NULL", want: ForeignKeySetNullAction}, + {input: "set null", want: ForeignKeySetNullAction}, + {input: string(ForeignKeySetDefaultAction), want: ForeignKeySetDefaultAction}, + {input: "SET DEFAULT", want: ForeignKeySetDefaultAction}, + {input: "set default", want: ForeignKeySetDefaultAction}, + {input: string(ForeignKeyRestrictAction), want: ForeignKeyRestrictAction}, + {input: "RESTRICT", want: ForeignKeyRestrictAction}, + {input: "restrict", want: ForeignKeyRestrictAction}, + {input: string(ForeignKeyNoAction), want: ForeignKeyNoAction}, + {input: "NO ACTION", want: ForeignKeyNoAction}, + {input: "no action", want: ForeignKeyNoAction}, + } + + negativeTests := []test{ + {input: "no action "}, + {input: " RESTRICT"}, + {input: "not null"}, + {input: "abc"}, + {input: ""}, + } + + for _, tc := range positiveTests { + tc := tc + t.Run(tc.input, func(t *testing.T) { + got, err := ToForeignKeyAction(tc.input) + require.NoError(t, err) + require.Equal(t, tc.want, got) + }) + } + + for _, tc := range negativeTests { + tc := tc + t.Run(tc.input, func(t *testing.T) { + got, err := ToForeignKeyAction(tc.input) + require.Error(t, err) + require.Empty(t, got) + }) + } +} diff --git a/pkg/snowflake/table_constraint.go b/pkg/snowflake/table_constraint.go index 3827859f5d..54ee63a2a9 100644 --- a/pkg/snowflake/table_constraint.go +++ b/pkg/snowflake/table_constraint.go @@ -4,10 +4,8 @@ import ( "database/sql" "errors" "fmt" - "log" - "strings" - "github.com/jmoiron/sqlx" + "log" ) // TableConstraintBuilder abstracts the creation of SQL queries for a Snowflake table constraint. @@ -132,59 +130,6 @@ func (b *TableConstraintBuilder) formattedColumns() []string { return formattedColumns } -// Create returns the SQL query that will create a new table constraint. -func (b *TableConstraintBuilder) Create() string { - q := strings.Builder{} - q.WriteString(fmt.Sprintf(`ALTER TABLE %v ADD CONSTRAINT %v %v`, b.tableID, b.name, b.constraintType)) - if b.columns != nil { - q.WriteString(fmt.Sprintf(` (%v)`, strings.Join(b.formattedColumns(), ", "))) - } - - if b.constraintType == "FOREIGN KEY" { - q.WriteString(fmt.Sprintf(` REFERENCES %v (%v)`, b.referenceTableID, strings.Join(b.formattedReferenceColumns(), ", "))) - - if b.match != "" { - q.WriteString(fmt.Sprintf(` MATCH %v`, b.match)) - } - if b.update != "" { - q.WriteString(fmt.Sprintf(` ON UPDATE %v`, b.update)) - } - if b.delete != "" { - q.WriteString(fmt.Sprintf(` ON DELETE %v`, b.delete)) - } - } - - if b.enforced { - q.WriteString(` ENFORCED`) - } - - if !b.deferrable { - q.WriteString(` NOT DEFERRABLE`) - } - - if b.initially != "DEFERRED" { - q.WriteString(fmt.Sprintf(` INITIALLY %v`, b.initially)) - } - - if !b.enable { - q.WriteString(` DISABLE`) - } - - if b.validate { - q.WriteString(` VALIDATE`) - } - - if !b.rely { - q.WriteString(` NORELY`) - } - - if b.comment != "" { - q.WriteString(fmt.Sprintf(` COMMENT '%v'`, EscapeString(b.comment))) - } - - return q.String() -} - // Rename returns the SQL query that will rename the table constraint. func (b *TableConstraintBuilder) Rename(newName string) string { return fmt.Sprintf(`ALTER TABLE %v RENAME CONSTRAINT %v TO %v`, b.tableID, b.name, newName) From 99def8560e028cd7fb3a05947f65439db72a2b10 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Fri, 2 Feb 2024 11:44:19 +0100 Subject: [PATCH 2/7] Refined validations to remove repetition --- pkg/resources/table_constraint.go | 8 ++++---- pkg/sdk/common_table_types.go | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/pkg/resources/table_constraint.go b/pkg/resources/table_constraint.go index caf4f2a7c4..920ac653dc 100644 --- a/pkg/resources/table_constraint.go +++ b/pkg/resources/table_constraint.go @@ -26,7 +26,7 @@ var tableConstraintSchema = map[string]*schema.Schema{ Required: true, Description: "Type of constraint, one of 'UNIQUE', 'PRIMARY KEY', or 'FOREIGN KEY'", ForceNew: true, - ValidateFunc: validation.StringInSlice([]string{"UNIQUE", "PRIMARY KEY", "FOREIGN KEY"}, false), + ValidateFunc: validation.StringInSlice(sdk.AsStringList(sdk.AllColumnConstraintTypes), false), DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { return strings.EqualFold(old, new) }, @@ -137,7 +137,7 @@ var tableConstraintSchema = map[string]*schema.Schema{ Optional: true, ForceNew: true, Default: "FULL", - ValidateFunc: validation.StringInSlice([]string{"FULL", "PARTIAL", "SIMPLE"}, true), + ValidateFunc: validation.StringInSlice(sdk.AsStringList(sdk.AllMatchTypes), true), Description: "The match type for the foreign key. Not applicable for primary/unique keys", }, "on_update": { @@ -145,7 +145,7 @@ var tableConstraintSchema = map[string]*schema.Schema{ Optional: true, ForceNew: true, Default: "NO ACTION", - ValidateFunc: validation.StringInSlice([]string{"NO ACTION", "CASCADE", "SET NULL", "SET DEFAULT", "RESTRICT"}, true), + ValidateFunc: validation.StringInSlice(sdk.AsStringList(sdk.AllForeignKeyActions), true), Description: "Specifies the action performed when the primary/unique key for the foreign key is updated. Not applicable for primary/unique keys", DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { return strings.EqualFold(old, new) @@ -159,7 +159,7 @@ var tableConstraintSchema = map[string]*schema.Schema{ Optional: true, ForceNew: true, Default: "NO ACTION", - ValidateFunc: validation.StringInSlice([]string{"NO ACTION", "CASCADE", "SET NULL", "SET DEFAULT", "RESTRICT"}, true), + ValidateFunc: validation.StringInSlice(sdk.AsStringList(sdk.AllForeignKeyActions), true), Description: "Specifies the action performed when the primary/unique key for the foreign key is deleted. Not applicable for primary/unique keys", DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool { return strings.EqualFold(old, new) diff --git a/pkg/sdk/common_table_types.go b/pkg/sdk/common_table_types.go index f84a2ddef1..0bbde889da 100644 --- a/pkg/sdk/common_table_types.go +++ b/pkg/sdk/common_table_types.go @@ -80,6 +80,8 @@ const ( ColumnConstraintTypeForeignKey ColumnConstraintType = "FOREIGN KEY" ) +var AllColumnConstraintTypes = []ColumnConstraintType{ColumnConstraintTypeUnique, ColumnConstraintTypePrimaryKey, ColumnConstraintTypeForeignKey} + func ToColumnConstraintType(s string) (ColumnConstraintType, error) { cType := strings.ToUpper(s) @@ -118,6 +120,8 @@ var ( PartialMatchType MatchType = "PARTIAL" ) +var AllMatchTypes = []MatchType{FullMatchType, SimpleMatchType, PartialMatchType} + func ToMatchType(s string) (MatchType, error) { cType := strings.ToUpper(s) @@ -148,6 +152,8 @@ const ( ForeignKeyNoAction ForeignKeyAction = "NO ACTION" ) +var AllForeignKeyActions = []ForeignKeyAction{ForeignKeyCascadeAction, ForeignKeySetNullAction, ForeignKeySetDefaultAction, ForeignKeyRestrictAction, ForeignKeyNoAction} + func ToForeignKeyAction(s string) (ForeignKeyAction, error) { cType := strings.ToUpper(s) @@ -166,3 +172,11 @@ func ToForeignKeyAction(s string) (ForeignKeyAction, error) { return "", fmt.Errorf("invalid column constraint type: %s", s) } + +func AsStringList[T ~string](input []T) []string { + output := make([]string, len(input)) + for i, element := range input { + output[i] = string(element) + } + return output +} From 3a0c6979905d8179c188d8357bb2932a5b637e6a Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Fri, 2 Feb 2024 12:28:16 +0100 Subject: [PATCH 3/7] Migrate constraint resource --- .../snowflake_table_constraint/resource.tf | 8 +- pkg/resources/table_constraint.go | 86 ++++---- .../table_constraint_acceptance_test.go | 40 ++-- pkg/snowflake/table_constraint.go | 186 ------------------ 4 files changed, 70 insertions(+), 250 deletions(-) delete mode 100644 pkg/snowflake/table_constraint.go diff --git a/examples/resources/snowflake_table_constraint/resource.tf b/examples/resources/snowflake_table_constraint/resource.tf index 78956193c9..58d2747dfe 100644 --- a/examples/resources/snowflake_table_constraint/resource.tf +++ b/examples/resources/snowflake_table_constraint/resource.tf @@ -52,7 +52,7 @@ resource "snowflake_table" "fk_t" { resource "snowflake_table_constraint" "primary_key" { name = "myconstraint" type = "PRIMARY KEY" - table_id = snowflake_table.t.id + table_id = snowflake_table.t.qualified_name columns = ["col1"] comment = "hello world" } @@ -60,11 +60,11 @@ resource "snowflake_table_constraint" "primary_key" { resource "snowflake_table_constraint" "foreign_key" { name = "myconstraintfk" type = "FOREIGN KEY" - table_id = snowflake_table.t.id + table_id = snowflake_table.t.qualified_name columns = ["col2"] foreign_key_properties { references { - table_id = snowflake_table.fk_t.id + table_id = snowflake_table.fk_t.qualified_name columns = ["fk_col1"] } } @@ -77,7 +77,7 @@ resource "snowflake_table_constraint" "foreign_key" { resource "snowflake_table_constraint" "unique" { name = "unique" type = "UNIQUE" - table_id = snowflake_table.t.id + table_id = snowflake_table.t.qualified_name columns = ["col3"] comment = "hello unique" } diff --git a/pkg/resources/table_constraint.go b/pkg/resources/table_constraint.go index 920ac653dc..a365dd6007 100644 --- a/pkg/resources/table_constraint.go +++ b/pkg/resources/table_constraint.go @@ -4,17 +4,19 @@ import ( "context" "database/sql" "fmt" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers" "strings" - snowflakeValidation "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/validation" - + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/snowflake" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation" ) +// TODO [SNOW-867235]: refine this resource during redesign: +// - read (from the existing comment it seems that active warehouse is needed (it should be probably added to the resource as required) +// - drop (in tests it's not dropped correctly, probably also because missing warehouse) +// - do we need it? var tableConstraintSchema = map[string]*schema.Schema{ "name": { Type: schema.TypeString, @@ -204,6 +206,18 @@ func (v *tableConstraintID) parse(s string) { v.tableID = parts[2] } +func getTableIdentifier(s string) (*sdk.SchemaObjectIdentifier, error) { + objectIdentifier, err := helpers.DecodeSnowflakeParameterID(s) + if err != nil { + return nil, fmt.Errorf("table id is incorrect: %s, err: %s", objectIdentifier, err) + } + tableIdentifier, ok := objectIdentifier.(sdk.SchemaObjectIdentifier) + if !ok { + return nil, fmt.Errorf("table id is incorrect: %s", objectIdentifier) + } + return &tableIdentifier, nil +} + // CreateTableConstraint implements schema.CreateFunc. func CreateTableConstraint(d *schema.ResourceData, meta interface{}) error { db := meta.(*sql.DB) @@ -214,27 +228,23 @@ func CreateTableConstraint(d *schema.ResourceData, meta interface{}) error { cType := d.Get("type").(string) tableID := d.Get("table_id").(string) - objectIdentifier, err := helpers.DecodeSnowflakeParameterID(tableID) + tableIdentifier, err := getTableIdentifier(tableID) if err != nil { - return fmt.Errorf("table id is incorrect: %s, err: %s", objectIdentifier, err) - } - tableIdentifier, ok := objectIdentifier.(sdk.SchemaObjectIdentifier) - if !ok { - return fmt.Errorf("table id is incorrect: %s", objectIdentifier) + return err } constraintType, err := sdk.ToColumnConstraintType(cType) if err != nil { return err } - constraintRequest := sdk.NewOutOfLineConstraintRequest(constraintType) + constraintRequest := sdk.NewOutOfLineConstraintRequest(constraintType).WithName(&name) cc := d.Get("columns").([]interface{}) columns := make([]string, 0, len(cc)) for _, c := range cc { columns = append(columns, c.(string)) } - constraintRequest.WithColumns(columns) + constraintRequest.WithColumns(snowflake.QuoteStringList(columns)) if v, ok := d.GetOk("enforced"); ok { constraintRequest.WithEnforced(sdk.Bool(v.(bool))) @@ -271,11 +281,11 @@ func CreateTableConstraint(d *schema.ResourceData, meta interface{}) error { fkTableID := references["table_id"].(string) fkId, err := helpers.DecodeSnowflakeParameterID(fkTableID) if err != nil { - return fmt.Errorf("table id is incorrect: %s, err: %s", objectIdentifier, err) + return fmt.Errorf("table id is incorrect: %s, err: %s", fkTableID, err) } referencedTableIdentifier, ok := fkId.(sdk.SchemaObjectIdentifier) if !ok { - return fmt.Errorf("table id is incorrect: %s", objectIdentifier) + return fmt.Errorf("table id is incorrect: %s", fkId) } cols := references["columns"].([]interface{}) @@ -283,7 +293,7 @@ func CreateTableConstraint(d *schema.ResourceData, meta interface{}) error { for _, c := range cols { fkColumns = append(fkColumns, c.(string)) } - foreignKeyRequest := sdk.NewOutOfLineForeignKeyRequest(referencedTableIdentifier, fkColumns) + foreignKeyRequest := sdk.NewOutOfLineForeignKeyRequest(referencedTableIdentifier, snowflake.QuoteStringList(fkColumns)) matchType, err := sdk.ToMatchType(foreignKeyProperties["match"].(string)) if err != nil { @@ -306,7 +316,7 @@ func CreateTableConstraint(d *schema.ResourceData, meta interface{}) error { constraintRequest.WithForeignKey(foreignKeyRequest) } - alterStatement := sdk.NewAlterTableRequest(tableIdentifier).WithConstraintAction(sdk.NewTableConstraintActionRequest().WithAdd(constraintRequest)) + alterStatement := sdk.NewAlterTableRequest(*tableIdentifier).WithConstraintAction(sdk.NewTableConstraintActionRequest().WithAdd(constraintRequest)) err = client.Tables.Alter(ctx, alterStatement) if err != nil { return fmt.Errorf("error creating table constraint %v err = %w", name, err) @@ -345,26 +355,24 @@ func ReadTableConstraint(_ *schema.ResourceData, _ interface{}) error { // UpdateTableConstraint implements schema.UpdateFunc. func UpdateTableConstraint(d *schema.ResourceData, meta interface{}) error { db := meta.(*sql.DB) + ctx := context.Background() + client := sdk.NewClientFromDB(db) + tc := tableConstraintID{} tc.parse(d.Id()) - formattedTableID := snowflakeValidation.ParseAndFormatFullyQualifiedObectID(tc.tableID) - - builder := snowflake.NewTableConstraintBuilder(tc.name, tc.constraintType, formattedTableID) - /* "unsupported feature comment error message" - if d.HasChange("comment") { - _, new := d.GetChange("comment") - _, err := db.Exec(builder.SetComment(new.(string))) - if err != nil { - return fmt.Errorf("error setting comment for table constraint %v", tc.name) - } - }*/ + tableIdentifier, err := getTableIdentifier(tc.tableID) + if err != nil { + return err + } if d.HasChange("name") { _, n := d.GetChange("name") - _, err := db.Exec(builder.Rename(n.(string))) + constraintRequest := sdk.NewTableConstraintRenameActionRequest().WithOldName(tc.name).WithNewName(n.(string)) + alterStatement := sdk.NewAlterTableRequest(*tableIdentifier).WithConstraintAction(sdk.NewTableConstraintActionRequest().WithRename(constraintRequest)) + err = client.Tables.Alter(ctx, alterStatement) if err != nil { - return fmt.Errorf("error renaming table constraint %v err = %w", tc.name, err) + return fmt.Errorf("error renaming table constraint %v err = %w", &tc.name, err) } } @@ -374,28 +382,30 @@ func UpdateTableConstraint(d *schema.ResourceData, meta interface{}) error { // DeleteTableConstraint implements schema.DeleteFunc. func DeleteTableConstraint(d *schema.ResourceData, meta interface{}) error { db := meta.(*sql.DB) + ctx := context.Background() + client := sdk.NewClientFromDB(db) + tc := tableConstraintID{} tc.parse(d.Id()) - formattedTableID := snowflakeValidation.ParseAndFormatFullyQualifiedObectID(tc.tableID) - builder := snowflake.NewTableConstraintBuilder(tc.name, tc.constraintType, formattedTableID) - cc := d.Get("columns").([]interface{}) - columns := make([]string, 0, len(cc)) - for _, c := range cc { - columns = append(columns, c.(string)) + + tableIdentifier, err := getTableIdentifier(tc.tableID) + if err != nil { + return err } - builder.WithColumns(columns) - stmt := builder.Drop() - _, err := db.Exec(stmt) + dropRequest := sdk.NewTableConstraintDropActionRequest().WithConstraintName(&tc.name) + alterStatement := sdk.NewAlterTableRequest(*tableIdentifier).WithConstraintAction(sdk.NewTableConstraintActionRequest().WithDrop(dropRequest)) + err = client.Tables.Alter(ctx, alterStatement) if err != nil { // if the table constraint does not exist, then remove from state file if strings.Contains(err.Error(), "does not exist") { d.SetId("") return nil } - return fmt.Errorf("error deleting table constraint %v err = %w", tc.name, err) + return fmt.Errorf("error dropping table constraint %v err = %w", tc.name, err) } d.SetId("") + return nil } diff --git a/pkg/resources/table_constraint_acceptance_test.go b/pkg/resources/table_constraint_acceptance_test.go index 2c75531474..efdf4b5d08 100644 --- a/pkg/resources/table_constraint_acceptance_test.go +++ b/pkg/resources/table_constraint_acceptance_test.go @@ -5,16 +5,21 @@ import ( "testing" acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" + "github.com/hashicorp/terraform-plugin-testing/helper/acctest" "github.com/hashicorp/terraform-plugin-testing/helper/resource" + "github.com/hashicorp/terraform-plugin-testing/tfversion" ) func TestAcc_TableConstraint_fk(t *testing.T) { name := acctest.RandStringFromCharSet(10, acctest.CharSetAlpha) - resource.ParallelTest(t, resource.TestCase{ - Providers: acc.TestAccProviders(), - PreCheck: func() { acc.TestAccPreCheck(t) }, + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, CheckDestroy: nil, Steps: []resource.TestStep{ { @@ -58,11 +63,11 @@ resource "snowflake_table" "fk_t" { resource "snowflake_table_constraint" "fk" { name="%s" type= "FOREIGN KEY" - table_id = snowflake_table.t.id + table_id = snowflake_table.t.qualified_name columns = ["col1"] foreign_key_properties { references { - table_id = snowflake_table.fk_t.id + table_id = snowflake_table.fk_t.qualified_name columns = ["fk_col1"] } } @@ -78,9 +83,12 @@ resource "snowflake_table_constraint" "fk" { func TestAcc_TableConstraint_unique(t *testing.T) { name := acctest.RandStringFromCharSet(10, acctest.CharSetAlpha) - resource.ParallelTest(t, resource.TestCase{ - Providers: acc.TestAccProviders(), - PreCheck: func() { acc.TestAccPreCheck(t) }, + resource.Test(t, resource.TestCase{ + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, CheckDestroy: nil, Steps: []resource.TestStep{ { @@ -110,22 +118,10 @@ resource "snowflake_table" "t" { } } -resource "snowflake_table" "unique_t" { - name = "unique_%s" - database = "%s" - schema = "%s" - - column { - name = "unique_col1" - type = "text" - nullable = false - } -} - resource "snowflake_table_constraint" "unique" { name="%s" type= "UNIQUE" - table_id = snowflake_table.t.id + table_id = snowflake_table.t.qualified_name columns = ["col1"] enforced = true deferrable = false @@ -133,5 +129,5 @@ resource "snowflake_table_constraint" "unique" { comment = "hello unique" } -`, n, databaseName, schemaName, n, databaseName, schemaName, n) +`, n, databaseName, schemaName, n) } diff --git a/pkg/snowflake/table_constraint.go b/pkg/snowflake/table_constraint.go deleted file mode 100644 index 54ee63a2a9..0000000000 --- a/pkg/snowflake/table_constraint.go +++ /dev/null @@ -1,186 +0,0 @@ -package snowflake - -import ( - "database/sql" - "errors" - "fmt" - "github.com/jmoiron/sqlx" - "log" -) - -// TableConstraintBuilder abstracts the creation of SQL queries for a Snowflake table constraint. -type TableConstraintBuilder struct { - name string - columns []string - constraintType string - tableID string - enforced bool - deferrable bool - initially string - enable bool - validate bool - rely bool - referenceTableID string - referenceColumns []string - match string - update string - delete string - comment string -} - -func NewTableConstraintBuilder(name string, constraintType string, tableID string) *TableConstraintBuilder { - return &TableConstraintBuilder{ - name: name, - constraintType: constraintType, - tableID: tableID, - } -} - -// WithComment sets comment. -func (b *TableConstraintBuilder) WithComment(comment string) *TableConstraintBuilder { - b.comment = comment - return b -} - -// WithColumns sets columns. -func (b *TableConstraintBuilder) WithColumns(columns []string) *TableConstraintBuilder { - b.columns = columns - return b -} - -// WithEnforced sets enforced. -func (b *TableConstraintBuilder) WithEnforced(enforced bool) *TableConstraintBuilder { - b.enforced = enforced - return b -} - -// WithDeferrable sets deferrable. -func (b *TableConstraintBuilder) WithDeferrable(deferrable bool) *TableConstraintBuilder { - b.deferrable = deferrable - return b -} - -// WithInitially sets initially. -func (b *TableConstraintBuilder) WithInitially(initially string) *TableConstraintBuilder { - b.initially = initially - return b -} - -// WithEnable sets enable. -func (b *TableConstraintBuilder) WithEnable(enable bool) *TableConstraintBuilder { - b.enable = enable - return b -} - -// WithValidated sets validated. -func (b *TableConstraintBuilder) WithValidate(validate bool) *TableConstraintBuilder { - b.validate = validate - return b -} - -// WithRely sets rely. -func (b *TableConstraintBuilder) WithRely(rely bool) *TableConstraintBuilder { - b.rely = rely - return b -} - -// WithReferenceTableID sets referenceTableID. -func (b *TableConstraintBuilder) WithReferenceTableID(referenceTableID string) *TableConstraintBuilder { - b.referenceTableID = referenceTableID - return b -} - -// WithReferenceColumns sets referenceColumns. -func (b *TableConstraintBuilder) WithReferenceColumns(referenceColumns []string) *TableConstraintBuilder { - b.referenceColumns = referenceColumns - return b -} - -// WithMatch sets match. -func (b *TableConstraintBuilder) WithMatch(match string) *TableConstraintBuilder { - b.match = match - return b -} - -// WithUpdate sets update. -func (b *TableConstraintBuilder) WithUpdate(onUpdate string) *TableConstraintBuilder { - b.update = onUpdate - return b -} - -// WithDelete sets delete. -func (b *TableConstraintBuilder) WithDelete(onDelete string) *TableConstraintBuilder { - b.delete = onDelete - return b -} - -func (b *TableConstraintBuilder) formattedReferenceColumns() []string { - formattedColumns := make([]string, len(b.referenceColumns)) - for i, c := range b.referenceColumns { - formattedColumns[i] = fmt.Sprintf(`"%v"`, EscapeString(c)) - } - return formattedColumns -} - -func (b *TableConstraintBuilder) formattedColumns() []string { - formattedColumns := make([]string, len(b.columns)) - for i, c := range b.columns { - formattedColumns[i] = fmt.Sprintf(`"%v"`, EscapeString(c)) - } - return formattedColumns -} - -// Rename returns the SQL query that will rename the table constraint. -func (b *TableConstraintBuilder) Rename(newName string) string { - return fmt.Sprintf(`ALTER TABLE %v RENAME CONSTRAINT %v TO %v`, b.tableID, b.name, newName) -} - -// SetComment returns the SQL query that will update the comment on the table constraint. -func (b *TableConstraintBuilder) SetComment(c string) string { - return fmt.Sprintf(`COMMENT ON CONSTRAINT %v IS '%v'`, b.name, EscapeString(c)) -} - -// Drop returns the SQL query that will drop a table constraint. -func (b *TableConstraintBuilder) Drop() string { - s := fmt.Sprintf(`ALTER TABLE %v DROP CONSTRAINT %v`, b.tableID, b.name) - /*if b.columns != nil { - s += fmt.Sprintf(` (%v)`, strings.Join(b.formattedColumns(), ", ")) - }*/ - s += " CASCADE" - return s -} - -type TableConstraint struct { - ConstraintCatalog sql.NullString `db:"CONSTRAINT_CATALOG"` - ConstraintSchema sql.NullString `db:"CONSTRAINT_SCHEMA"` - ConstraintName sql.NullString `db:"CONSTRAINT_NAME"` - TableCatalog sql.NullString `db:"TABLE_CATALOG"` - TableSchema sql.NullString `db:"TABLE_SCHEMA"` - TableName sql.NullString `db:"TABLE_NAME"` - ConstraintType sql.NullString `db:"CONSTRAINT_TYPE"` - IsDeferrable sql.NullString `db:"IS_DEFERRABLE"` - InitiallyDeferred sql.NullString `db:"INITIALLY_DEFERRED"` - Enforced sql.NullString `db:"ENFORCED"` - Comment sql.NullString `db:"COMMENT"` -} - -// Show returns the SQL query that will show a table constraint by ID. -func ShowTableConstraint(name, tableDB, tableSchema, tableName string, db *sql.DB) (*TableConstraint, error) { - stmt := `SELECT * FROM SNOWFLAKE.INFORMATION_SCHEMA.TABLE_CONSTRAINTS WHERE TABLE_NAME = '?' AND TABLE_SCHEMA = '?' AND TABLE_CATALOG = '?' AND CONSTRAINT_NAME = '?'` - rows, err := db.Query(stmt, - tableName, tableSchema, tableDB, name) - if err != nil { - return nil, err - } - defer rows.Close() - tableConstraints := []TableConstraint{} - log.Printf("[DEBUG] tableConstraints is %v", tableConstraints) - if err := sqlx.StructScan(rows, &tableConstraints); err != nil { - if errors.Is(err, sql.ErrNoRows) { - log.Printf("[DEBUG] no tableConstraints found for constraint %s", name) - return nil, err - } - return nil, fmt.Errorf("unable to scan row for %s err = %w", stmt, err) - } - return &tableConstraints[0], nil -} From c6ce92cddd587bda4c7b00e37088207cacdb582d Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Tue, 6 Feb 2024 15:50:38 +0100 Subject: [PATCH 4/7] Generate docs # Conflicts: # pkg/resources/network_policy_attachment.go --- docs/resources/table_constraint.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/resources/table_constraint.md b/docs/resources/table_constraint.md index 874e1b9d05..340e800145 100644 --- a/docs/resources/table_constraint.md +++ b/docs/resources/table_constraint.md @@ -67,7 +67,7 @@ resource "snowflake_table" "fk_t" { resource "snowflake_table_constraint" "primary_key" { name = "myconstraint" type = "PRIMARY KEY" - table_id = snowflake_table.t.id + table_id = snowflake_table.t.qualified_name columns = ["col1"] comment = "hello world" } @@ -75,11 +75,11 @@ resource "snowflake_table_constraint" "primary_key" { resource "snowflake_table_constraint" "foreign_key" { name = "myconstraintfk" type = "FOREIGN KEY" - table_id = snowflake_table.t.id + table_id = snowflake_table.t.qualified_name columns = ["col2"] foreign_key_properties { references { - table_id = snowflake_table.fk_t.id + table_id = snowflake_table.fk_t.qualified_name columns = ["fk_col1"] } } @@ -92,7 +92,7 @@ resource "snowflake_table_constraint" "foreign_key" { resource "snowflake_table_constraint" "unique" { name = "unique" type = "UNIQUE" - table_id = snowflake_table.t.id + table_id = snowflake_table.t.qualified_name columns = ["col3"] comment = "hello unique" } @@ -105,12 +105,12 @@ resource "snowflake_table_constraint" "unique" { - `columns` (List of String) Columns to use in constraint key - `name` (String) Name of constraint -- `table_id` (String) Idenfifier for table to create constraint on. Must be of the form Note: format must follow: "".""."" or ".." or "|." (snowflake_table.my_table.id) -- `type` (String) Type of constraint, one of 'UNIQUE', 'PRIMARY KEY', 'FOREIGN KEY', or 'NOT NULL' +- `table_id` (String) Identifier for table to create constraint on. Format must follow: """."".""" or ".." (snowflake_table.my_table.id) +- `type` (String) Type of constraint, one of 'UNIQUE', 'PRIMARY KEY', or 'FOREIGN KEY' ### Optional -- `comment` (String) Comment for the table constraint +- `comment` (String, Deprecated) Comment for the table constraint - `deferrable` (Boolean) Whether the constraint is deferrable - `enable` (Boolean) Specifies whether the constraint is enabled or disabled. These properties are provided for compatibility with Oracle. - `enforced` (Boolean) Whether the constraint is enforced From c660f64f71b9a718f86af29eba0533f383ce19cd Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Tue, 6 Feb 2024 15:52:16 +0100 Subject: [PATCH 5/7] Fix linter complaints --- pkg/resources/table_constraint.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/resources/table_constraint.go b/pkg/resources/table_constraint.go index a365dd6007..c6d0958630 100644 --- a/pkg/resources/table_constraint.go +++ b/pkg/resources/table_constraint.go @@ -209,7 +209,7 @@ func (v *tableConstraintID) parse(s string) { func getTableIdentifier(s string) (*sdk.SchemaObjectIdentifier, error) { objectIdentifier, err := helpers.DecodeSnowflakeParameterID(s) if err != nil { - return nil, fmt.Errorf("table id is incorrect: %s, err: %s", objectIdentifier, err) + return nil, fmt.Errorf("table id is incorrect: %s, err: %w", objectIdentifier, err) } tableIdentifier, ok := objectIdentifier.(sdk.SchemaObjectIdentifier) if !ok { @@ -281,7 +281,7 @@ func CreateTableConstraint(d *schema.ResourceData, meta interface{}) error { fkTableID := references["table_id"].(string) fkId, err := helpers.DecodeSnowflakeParameterID(fkTableID) if err != nil { - return fmt.Errorf("table id is incorrect: %s, err: %s", fkTableID, err) + return fmt.Errorf("table id is incorrect: %s, err: %w", fkTableID, err) } referencedTableIdentifier, ok := fkId.(sdk.SchemaObjectIdentifier) if !ok { From 078de2c6e7aa01a95e8fdcdeabadec2778e0a4be Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Tue, 6 Feb 2024 16:05:13 +0100 Subject: [PATCH 6/7] Fix docs --- docs/resources/table_constraint.md | 2 +- pkg/resources/table_constraint.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/resources/table_constraint.md b/docs/resources/table_constraint.md index 340e800145..a149e609d9 100644 --- a/docs/resources/table_constraint.md +++ b/docs/resources/table_constraint.md @@ -105,7 +105,7 @@ resource "snowflake_table_constraint" "unique" { - `columns` (List of String) Columns to use in constraint key - `name` (String) Name of constraint -- `table_id` (String) Identifier for table to create constraint on. Format must follow: """."".""" or ".." (snowflake_table.my_table.id) +- `table_id` (String) Identifier for table to create constraint on. Format must follow: "\"\".\"\".\"\"" or ".." (snowflake_table.my_table.id) - `type` (String) Type of constraint, one of 'UNIQUE', 'PRIMARY KEY', or 'FOREIGN KEY' ### Optional diff --git a/pkg/resources/table_constraint.go b/pkg/resources/table_constraint.go index c6d0958630..b4602abb9c 100644 --- a/pkg/resources/table_constraint.go +++ b/pkg/resources/table_constraint.go @@ -40,7 +40,7 @@ var tableConstraintSchema = map[string]*schema.Schema{ Type: schema.TypeString, Required: true, ForceNew: true, - Description: "Identifier for table to create constraint on. Format must follow: \"\"\".\"\".\"\"\" or \"..\" (snowflake_table.my_table.id)", + Description: `Identifier for table to create constraint on. Format must follow: "\"\".\"\".\"\"" or ".." (snowflake_table.my_table.id)`, }, "columns": { Type: schema.TypeList, From 96d04dfcb3c93d57e11c8f7e578b00b27f522fb7 Mon Sep 17 00:00:00 2001 From: Artur Sawicki Date: Wed, 7 Feb 2024 10:21:52 +0100 Subject: [PATCH 7/7] Add NOT NULL removal note in the migration guide --- MIGRATION_GUIDE.md | 10 ++++++++++ pkg/resources/table_constraint.go | 1 + pkg/sdk/testint/tables_integration_test.go | 20 ++++++++++++++++++++ 3 files changed, 31 insertions(+) diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 127365e85e..c204f3ec7d 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -4,6 +4,16 @@ This document is meant to help you migrate your Terraform config to the new newe describe deprecations or breaking changes and help you to change your configuration to keep the same (or similar) behavior across different versions. +## v0.85.0 ➞ v0.86.0 +### snowflake_table_constraint resource changes + +#### *(behavior change)* NOT NULL removed from possible types +The `type` of the constraint was limited back to `UNIQUE`, `PRIMARY KEY`, and `FOREIGN KEY`. +The reason for that is, that syntax for Out-of-Line constraint ([docs](https://docs.snowflake.com/en/sql-reference/sql/create-table-constraint#out-of-line-unique-primary-foreign-key)) does not contain `NOT NULL`. +It is noted as a behavior change but in some way it is not; with the previous implementation it did not work at all with `type` set to `NOT NULL` because the generated statement was not a valid Snowflake statement. + +We will consider adding `NOT NULL` back because it can be set by `ALTER COLUMN columnX SET NOT NULL`, but first we want to revisit the whole resource design. + ## vX.XX.X -> v0.85.0 ### Migration from old (grant) resources to new ones diff --git a/pkg/resources/table_constraint.go b/pkg/resources/table_constraint.go index b4602abb9c..3a48018816 100644 --- a/pkg/resources/table_constraint.go +++ b/pkg/resources/table_constraint.go @@ -17,6 +17,7 @@ import ( // - read (from the existing comment it seems that active warehouse is needed (it should be probably added to the resource as required) // - drop (in tests it's not dropped correctly, probably also because missing warehouse) // - do we need it? +// - not null cannot be set as a named constraint but it can be set by alter column statement - should it be added back here? var tableConstraintSchema = map[string]*schema.Schema{ "name": { Type: schema.TypeString, diff --git a/pkg/sdk/testint/tables_integration_test.go b/pkg/sdk/testint/tables_integration_test.go index 212087d1f9..ff621e1bb0 100644 --- a/pkg/sdk/testint/tables_integration_test.go +++ b/pkg/sdk/testint/tables_integration_test.go @@ -681,6 +681,26 @@ func TestInt_Table(t *testing.T) { require.NoError(t, err) }) + t.Run("add constraint: not null", func(t *testing.T) { + name := random.String() + id := sdk.NewSchemaObjectIdentifier(database.Name, schema.Name, name) + columns := []sdk.TableColumnRequest{ + *sdk.NewTableColumnRequest("COLUMN_1", sdk.DataTypeVARCHAR), + } + + err := client.Tables.Create(ctx, sdk.NewCreateTableRequest(id, columns)) + require.NoError(t, err) + t.Cleanup(cleanupTableProvider(id)) + + alterRequest := sdk.NewAlterTableRequest(id). + WithColumnAction(sdk.NewTableColumnActionRequest().WithAlter([]sdk.TableColumnAlterActionRequest{ + *sdk.NewTableColumnAlterActionRequest("COLUMN_1"). + WithNotNullConstraint(sdk.NewTableColumnNotNullConstraintRequest().WithSet(sdk.Bool(true))), + })) + err = client.Tables.Alter(ctx, alterRequest) + require.NoError(t, err) + }) + // TODO [SNOW-1007542]: check renamed constraint t.Run("alter constraint: rename", func(t *testing.T) { name := random.String()