fix: renames in update operations (#2702)

This change fixes all of the Updates that were performing re-names to
always setId at the end of the rename operation as well as updating id
that was used in the later operations. Also, the `ForceNew`s were
removed in cases where rename was also handled in the Update. Test cases
were added to show that rename is going through Update and not
DeleteAndCreate operation + additional param is always changed to see if
other operations are affected by the previous rename operation. The
table constraint was adjusted and cannot support the rename for now
(until the Read operation is working #2683). Migration guide updated for
behavioral changes.

---------

Co-authored-by: Artur Sawicki <artur.sawicki@snowflake.com>

MIGRATION_GUIDE.md

@@ -4,6 +4,13 @@ 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.88.0 ➞ v0.89.0
+#### *(behavior change)* ForceNew removed
+The `ForceNew` field was removed in favor of in-place Update for `name` parameter in:
+- `snowflake_file_format`
+- `snowflake_masking_policy`
+So from now, these objects won't be re-created when the `name` changes, but instead only the name will be updated with `ALTER .. RENAME TO` statements.
+
 ## v0.87.0 ➞ v0.88.0
 ### snowflake_procedure resource changes
 #### *(behavior change)* Execute as validation added

pkg/resources/file_format.go

@@ -7,6 +7,8 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
+
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/provider"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
@@ -93,7 +95,6 @@ var fileFormatSchema = map[string]*schema.Schema{
 		Type:        schema.TypeString,
 		Required:    true,
 		Description: "Specifies the identifier for the file format; must be unique for the database and schema in which the file format is created.",
-		ForceNew:    true,
 	},
 	"database": {
 		Type:        schema.TypeString,
@@ -767,6 +768,7 @@ func UpdateFileFormat(d *schema.ResourceData, meta interface{}) error {
 
 	if d.HasChange("name") {
 		newId := sdk.NewSchemaObjectIdentifier(id.DatabaseName(), id.SchemaName(), d.Get("name").(string))
+
 		err := client.FileFormats.Alter(ctx, id, &sdk.AlterFileFormatOptions{
 			Rename: &sdk.AlterFileFormatRenameOptions{
 				NewName: newId,
@@ -775,6 +777,8 @@ func UpdateFileFormat(d *schema.ResourceData, meta interface{}) error {
 		if err != nil {
 			return fmt.Errorf("error renaming file format: %w", err)
 		}
+
+		d.SetId(helpers.EncodeSnowflakeID(newId))
 		id = newId
 	}
 

pkg/resources/file_format_acceptance_test.go

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"testing"
 
+	"github.com/hashicorp/terraform-plugin-testing/plancheck"
+
 	acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
 
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources"
@@ -448,6 +450,47 @@ func TestAcc_FileFormat_issue1947(t *testing.T) {
 	})
 }
 
+func TestAcc_FileFormat_Rename(t *testing.T) {
+	name := acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)
+	newName := acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)
+	comment := acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)
+	newComment := acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)
+	resourceName := "snowflake_file_format.test"
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: acc.CheckDestroy(t, resources.FileFormat),
+		Steps: []resource.TestStep{
+			{
+				Config: fileFormatConfigWithComment(name, acc.TestDatabaseName, acc.TestSchemaName, comment),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "name", name),
+					resource.TestCheckResourceAttr(resourceName, "comment", comment),
+				),
+			},
+			{
+				Config: fileFormatConfigWithComment(newName, acc.TestDatabaseName, acc.TestSchemaName, newComment),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "name", newName),
+					resource.TestCheckResourceAttr(resourceName, "comment", newComment),
+				),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
+					},
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectEmptyPlan(),
+					},
+				},
+			},
+		},
+	})
+}
+
 func fileFormatConfigCSV(n string, databaseName string, schemaName string, fieldDelimiter string, fieldOptionallyEnclosedBy string, comment string) string {
 	return fmt.Sprintf(`
 resource "snowflake_file_format" "test" {
@@ -581,6 +624,18 @@ resource "snowflake_file_format" "test" {
 `, n, databaseName, schemaName, formatType)
 }
 
+func fileFormatConfigWithComment(n string, databaseName string, schemaName string, comment string) string {
+	return fmt.Sprintf(`
+resource "snowflake_file_format" "test" {
+	name = "%v"
+	database = "%s"
+	schema = "%s"
+	format_type = "XML"
+	comment = "%s"
+}
+`, n, databaseName, schemaName, comment)
+}
+
 func fileFormatConfigFullDefaultsWithAdditionalParam(n string, formatType string, databaseName string, schemaName string) string {
 	return fmt.Sprintf(`
 resource "snowflake_file_format" "test" {

pkg/resources/function.go

@@ -657,15 +657,17 @@ func UpdateContextFunction(ctx context.Context, d *schema.ResourceData, meta int
 
 	id := sdk.NewSchemaObjectIdentifierFromFullyQualifiedName(d.Id())
 	if d.HasChange("name") {
-		name := d.Get("name")
-		if err := client.Functions.Alter(ctx, sdk.NewAlterFunctionRequest(id.WithoutArguments(), id.Arguments()).WithRenameTo(sdk.Pointer(sdk.NewSchemaObjectIdentifier(id.DatabaseName(), id.SchemaName(), name.(string))))); err != nil {
-			return diag.FromErr(err)
-		}
-		id = sdk.NewSchemaObjectIdentifierWithArguments(id.DatabaseName(), id.SchemaName(), name.(string), id.Arguments())
-		if err := d.Set("name", name); err != nil {
+		name := d.Get("name").(string)
+		newId := sdk.NewSchemaObjectIdentifierWithArguments(id.DatabaseName(), id.SchemaName(), name, id.Arguments())
+
+		if err := client.Functions.Alter(ctx, sdk.NewAlterFunctionRequest(id.WithoutArguments(), id.Arguments()).WithRenameTo(sdk.Pointer(newId.WithoutArguments()))); err != nil {
 			return diag.FromErr(err)
 		}
+
+		d.SetId(newId.FullyQualifiedName())
+		id = newId
 	}
+
 	if d.HasChange("is_secure") {
 		secure := d.Get("is_secure")
 		if secure.(bool) {
@@ -678,6 +680,7 @@ func UpdateContextFunction(ctx context.Context, d *schema.ResourceData, meta int
 			}
 		}
 	}
+
 	if d.HasChange("comment") {
 		comment := d.Get("comment")
 		if comment != "" {
@@ -690,6 +693,7 @@ func UpdateContextFunction(ctx context.Context, d *schema.ResourceData, meta int
 			}
 		}
 	}
+
 	return ReadContextFunction(ctx, d, meta)
 }
 

pkg/resources/function_acceptance_test.go

@@ -5,6 +5,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/hashicorp/terraform-plugin-testing/plancheck"
+
 	acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
 
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources"
@@ -185,6 +187,7 @@ func TestAcc_Function_complex(t *testing.T) {
 // proves issue https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2490
 func TestAcc_Function_migrateFromVersion085(t *testing.T) {
 	name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+	comment := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
 	resourceName := "snowflake_function.f"
 
 	resource.Test(t, resource.TestCase{
@@ -206,7 +209,7 @@ func TestAcc_Function_migrateFromVersion085(t *testing.T) {
 						Source:            "Snowflake-Labs/snowflake",
 					},
 				},
-				Config: functionConfig(acc.TestDatabaseName, acc.TestSchemaName, name),
+				Config: functionConfig(acc.TestDatabaseName, acc.TestSchemaName, name, comment),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(resourceName, "id", fmt.Sprintf("%s|%s|%s|VARCHAR", acc.TestDatabaseName, acc.TestSchemaName, name)),
 					resource.TestCheckResourceAttr(resourceName, "name", name),
@@ -216,7 +219,7 @@ func TestAcc_Function_migrateFromVersion085(t *testing.T) {
 			},
 			{
 				ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
-				Config:                   functionConfig(acc.TestDatabaseName, acc.TestSchemaName, name),
+				Config:                   functionConfig(acc.TestDatabaseName, acc.TestSchemaName, name, comment),
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr(resourceName, "id", sdk.NewSchemaObjectIdentifierWithArguments(acc.TestDatabaseName, acc.TestSchemaName, name, []sdk.DataType{sdk.DataTypeVARCHAR}).FullyQualifiedName()),
 					resource.TestCheckResourceAttr(resourceName, "name", name),
@@ -228,12 +231,54 @@ func TestAcc_Function_migrateFromVersion085(t *testing.T) {
 	})
 }
 
-func functionConfig(database string, schema string, name string) string {
+func TestAcc_Function_Rename(t *testing.T) {
+	name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+	newName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+	comment := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+	newComment := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+	resourceName := "snowflake_function.f"
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: acc.CheckDestroy(t, resources.Function),
+		Steps: []resource.TestStep{
+			{
+				Config: functionConfig(acc.TestDatabaseName, acc.TestSchemaName, name, comment),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "name", name),
+					resource.TestCheckResourceAttr(resourceName, "comment", comment),
+				),
+			},
+			{
+				Config: functionConfig(acc.TestDatabaseName, acc.TestSchemaName, newName, newComment),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "name", newName),
+					resource.TestCheckResourceAttr(resourceName, "comment", newComment),
+				),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate),
+					},
+					PostApplyPostRefresh: []plancheck.PlanCheck{
+						plancheck.ExpectEmptyPlan(),
+					},
+				},
+			},
+		},
+	})
+}
+
+func functionConfig(database string, schema string, name string, comment string) string {
 	return fmt.Sprintf(`
 resource "snowflake_function" "f" {
   database        = "%[1]s"
   schema          = "%[2]s"
   name            = "%[3]s"
+  comment         = "%[4]s"
   return_type     = "VARCHAR"
   return_behavior = "IMMUTABLE"
   statement       = "SELECT PARAM"
@@ -243,5 +288,5 @@ resource "snowflake_function" "f" {
     type = "VARCHAR"
   }
 }
-`, database, schema, name)
+`, database, schema, name, comment)
 }

pkg/resources/masking_policy.go

@@ -35,7 +35,6 @@ var maskingPolicySchema = map[string]*schema.Schema{
 		Type:        schema.TypeString,
 		Required:    true,
 		Description: "Specifies the identifier for the masking policy; must be unique for the database and schema in which the masking policy is created.",
-		ForceNew:    true,
 	},
 	"database": {
 		Type:        schema.TypeString,
@@ -250,16 +249,30 @@ func ReadMaskingPolicy(d *schema.ResourceData, meta interface{}) error {
 // UpdateMaskingPolicy implements schema.UpdateFunc.
 func UpdateMaskingPolicy(d *schema.ResourceData, meta interface{}) error {
 	client := meta.(*provider.Context).Client
-	objectIdentifier := helpers.DecodeSnowflakeID(d.Id()).(sdk.SchemaObjectIdentifier)
+	id := helpers.DecodeSnowflakeID(d.Id()).(sdk.SchemaObjectIdentifier)
 	ctx := context.Background()
 
+	if d.HasChange("name") {
+		newID := sdk.NewSchemaObjectIdentifier(id.DatabaseName(), id.SchemaName(), d.Get("name").(string))
+
+		err := client.MaskingPolicies.Alter(ctx, id, &sdk.AlterMaskingPolicyOptions{
+			NewName: &newID,
+		})
+		if err != nil {
+			return err
+		}
+
+		d.SetId(helpers.EncodeSnowflakeID(newID))
+		id = newID
+	}
+
 	if d.HasChange("masking_expression") {
 		alterOptions := &sdk.AlterMaskingPolicyOptions{}
 		_, n := d.GetChange("masking_expression")
 		alterOptions.Set = &sdk.MaskingPolicySet{
 			Body: sdk.String(n.(string)),
 		}
-		err := client.MaskingPolicies.Alter(ctx, objectIdentifier, alterOptions)
+		err := client.MaskingPolicies.Alter(ctx, id, alterOptions)
 		if err != nil {
 			return err
 		}
@@ -276,26 +289,12 @@ func UpdateMaskingPolicy(d *schema.ResourceData, meta interface{}) error {
 				Comment: sdk.Bool(true),
 			}
 		}
-		err := client.MaskingPolicies.Alter(ctx, objectIdentifier, alterOptions)
+		err := client.MaskingPolicies.Alter(ctx, id, alterOptions)
 		if err != nil {
 			return err
 		}
 	}
 
-	if d.HasChange("name") {
-		_, n := d.GetChange("name")
-		newName := n.(string)
-		newID := sdk.NewSchemaObjectIdentifier(objectIdentifier.DatabaseName(), objectIdentifier.SchemaName(), newName)
-		alterOptions := &sdk.AlterMaskingPolicyOptions{
-			NewName: &newID,
-		}
-		err := client.MaskingPolicies.Alter(ctx, objectIdentifier, alterOptions)
-		if err != nil {
-			return err
-		}
-		d.SetId(helpers.EncodeSnowflakeID(newID))
-	}
-
 	return ReadMaskingPolicy(d, meta)
 }
 

pkg/resources/masking_policy_acceptance_test.go

@@ -5,12 +5,13 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/hashicorp/terraform-plugin-testing/plancheck"
+
 	acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
 
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources"
 	"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
-	"github.com/hashicorp/terraform-plugin-testing/plancheck"
 	"github.com/hashicorp/terraform-plugin-testing/tfversion"
 )
 
@@ -42,19 +43,17 @@ func TestAcc_MaskingPolicy(t *testing.T) {
 					resource.TestCheckResourceAttr("snowflake_masking_policy.test", "signature.0.column.0.type", "VARCHAR"),
 				),
 			},
-			// change comment
-			{
-				Config: maskingPolicyConfig(accName, accName, comment2, acc.TestDatabaseName, acc.TestSchemaName),
-				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr("snowflake_masking_policy.test", "name", accName),
-					resource.TestCheckResourceAttr("snowflake_masking_policy.test", "comment", comment2),
-				),
-			},
-			// rename
+			// rename + change comment
 			{
 				Config: maskingPolicyConfig(accName, accName2, comment2, acc.TestDatabaseName, acc.TestSchemaName),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PreApply: []plancheck.PlanCheck{
+						plancheck.ExpectResourceAction("snowflake_masking_policy.test", plancheck.ResourceActionUpdate),
+					},
+				},
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr("snowflake_masking_policy.test", "name", accName2),
+					resource.TestCheckResourceAttr("snowflake_masking_policy.test", "comment", comment2),
 				),
 			},
 			// change body and unset comment

pkg/resources/materialized_view.go

@@ -185,9 +185,7 @@ func UpdateMaterializedView(d *schema.ResourceData, meta interface{}) error {
 	id := helpers.DecodeSnowflakeID(d.Id()).(sdk.SchemaObjectIdentifier)
 
 	if d.HasChange("name") {
-		newName := d.Get("name").(string)
-
-		newId := sdk.NewSchemaObjectIdentifier(id.DatabaseName(), id.SchemaName(), newName)
+		newId := sdk.NewSchemaObjectIdentifier(id.DatabaseName(), id.SchemaName(), d.Get("name").(string))
 
 		err := client.MaterializedViews.Alter(ctx, sdk.NewAlterMaterializedViewRequest(id).WithRenameTo(&newId))
 		if err != nil {