fix: Prove problems with procedure resource data types (#2782)

Introduce a fix for arguments but not for the return_type
Snowflake-Labs · May 7, 2024 · 68d0318 · 68d0318
1 parent efde48d
commit 68d0318
Show file tree

Hide file tree

Showing 3 changed files with 140 additions and 8 deletions.
diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md
@@ -9,6 +9,10 @@ across different versions.
 #### *(behavior change)* Validation to column type added
 While solving issue [#2733](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2733) we have introduced diff suppression for `column.type`. To make it work correctly we have also added a validation to it. It should not cause any problems, but it's worth noting in case of any data types used that the provider is not aware of.
 
+### snowflake_procedure resource changes
+#### *(behavior change)* Validation to arguments type added
+Diff suppression for `arguments.type` is needed for the same reason as above for `snowflake_table` resource.
+
 ## 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:

diff --git a/pkg/resources/procedure.go b/pkg/resources/procedure.go
@@ -55,14 +55,11 @@ var procedureSchema = map[string]*schema.Schema{
 					Description: "The argument name",
 				},
 				"type": {
-					Type:     schema.TypeString,
-					Required: true,
-					// Suppress the diff shown if the values are equal when both compared in lower case.
-					DiffSuppressFunc: func(k, old, new string, d *schema.ResourceData) bool {
-						return strings.EqualFold(old, new)
-					},
-					ValidateFunc: IsDataType(),
-					Description:  "The argument type",
+					Type:             schema.TypeString,
+					Required:         true,
+					ValidateFunc:     dataTypeValidateFunc,
+					DiffSuppressFunc: dataTypeDiffSuppressFunc,
+					Description:      "The argument type",
 				},
 			},
 		},

diff --git a/pkg/resources/procedure_acceptance_test.go b/pkg/resources/procedure_acceptance_test.go
@@ -255,3 +255,134 @@ resource "snowflake_procedure" "p" {
 }
 `, database, schema, name)
 }
+
+func TestAcc_Procedure_proveArgsPermanentDiff(t *testing.T) {
+	name := acc.TestClient().Ids.Alpha()
+	resourceName := "snowflake_procedure.p"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck: func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: acc.CheckDestroy(t, resources.Procedure),
+		Steps: []resource.TestStep{
+			{
+				ExternalProviders: map[string]resource.ExternalProvider{
+					"snowflake": {
+						VersionConstraint: "=0.89.0",
+						Source:            "Snowflake-Labs/snowflake",
+					},
+				},
+				Config: sqlProcedureConfigArgsPermanentDiff(acc.TestDatabaseName, acc.TestSchemaName, name),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "id", sdk.NewSchemaObjectIdentifierWithArguments(acc.TestDatabaseName, acc.TestSchemaName, name, []sdk.DataType{sdk.DataTypeVARCHAR, sdk.DataTypeNumber}).FullyQualifiedName()),
+				),
+				ExpectNonEmptyPlan: true,
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PostApplyPreRefresh: []plancheck.PlanCheck{plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionUpdate)},
+				},
+			},
+			{
+				ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+				Config:                   sqlProcedureConfigArgsPermanentDiff(acc.TestDatabaseName, acc.TestSchemaName, name),
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PostApplyPreRefresh: []plancheck.PlanCheck{plancheck.ExpectEmptyPlan()},
+				},
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "id", sdk.NewSchemaObjectIdentifierWithArguments(acc.TestDatabaseName, acc.TestSchemaName, name, []sdk.DataType{sdk.DataTypeVARCHAR, sdk.DataTypeNumber}).FullyQualifiedName()),
+				),
+			},
+		},
+	})
+}
+
+// TODO [SNOW-1348106]: diff suppression for the return type (the same with functions); finish this test
+func TestAcc_Procedure_returnTypePermanentDiff(t *testing.T) {
+	name := acc.TestClient().Ids.Alpha()
+	resourceName := "snowflake_procedure.p"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck: func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: acc.CheckDestroy(t, resources.Procedure),
+		Steps: []resource.TestStep{
+			{
+				ExternalProviders: map[string]resource.ExternalProvider{
+					"snowflake": {
+						VersionConstraint: "=0.89.0",
+						Source:            "Snowflake-Labs/snowflake",
+					},
+				},
+				Config: sqlProcedureConfigReturnTypePermanentDiff(acc.TestDatabaseName, acc.TestSchemaName, name),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "id", sdk.NewSchemaObjectIdentifierWithArguments(acc.TestDatabaseName, acc.TestSchemaName, name, []sdk.DataType{sdk.DataTypeVARCHAR}).FullyQualifiedName()),
+				),
+				ExpectNonEmptyPlan: true,
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PostApplyPreRefresh: []plancheck.PlanCheck{plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionDestroyBeforeCreate)},
+				},
+			},
+			{
+				ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+				Config:                   sqlProcedureConfigReturnTypePermanentDiff(acc.TestDatabaseName, acc.TestSchemaName, name),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "id", sdk.NewSchemaObjectIdentifierWithArguments(acc.TestDatabaseName, acc.TestSchemaName, name, []sdk.DataType{sdk.DataTypeVARCHAR}).FullyQualifiedName()),
+				),
+				// should be empty after SNOW-1348106
+				ExpectNonEmptyPlan: true,
+				ConfigPlanChecks: resource.ConfigPlanChecks{
+					PostApplyPreRefresh: []plancheck.PlanCheck{plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionDestroyBeforeCreate)},
+				},
+			},
+		},
+	})
+}
+
+func sqlProcedureConfigArgsPermanentDiff(database string, schema string, name string) string {
+	return fmt.Sprintf(`
+resource "snowflake_procedure" "p" {
+  database    = "%[1]s"
+  schema      = "%[2]s"
+  name        = "%[3]s"
+  language    = "SQL"
+  return_type = "NUMBER(38,0)"
+  arguments {
+    name = "arg1"
+    type = "VARCHAR"
+  }
+  arguments {
+    name = "MY_INT"
+    type = "int"
+  }
+  statement   = <<EOT
+BEGIN
+  RETURN 13.4;
+END;
+  EOT
+}
+`, database, schema, name)
+}
+
+func sqlProcedureConfigReturnTypePermanentDiff(database string, schema string, name string) string {
+	return fmt.Sprintf(`
+resource "snowflake_procedure" "p" {
+  database    = "%[1]s"
+  schema      = "%[2]s"
+  name        = "%[3]s"
+  language    = "SQL"
+  return_type = "TABLE (NUM1 NUMBER(10,2))"
+  arguments {
+    name = "ARG1"
+    type = "VARCHAR"
+  }
+  statement   = <<EOT
+BEGIN
+  RETURN 13.4;
+END;
+  EOT
+}
+`, database, schema, name)
+}