fix: Fix views permadiff (#3079)

<!-- Feel free to delete comments as you fill this in -->
- fix permadiff when columns are not specified
- adjust row access policy signature in describe_output and move parsing
to sdk
- adjust the identifier doc - add procedure id 
<!-- summary of changes -->

## Test Plan
<!-- detail ways in which this PR has been tested or needs to be tested
-->
* [x] acceptance tests
<!-- add more below if you think they are relevant -->
* [ ] …

## References
<!-- issues documentation links, etc  -->

#3073
#2994

docs/data-sources/row_access_policies.md

@@ -152,7 +152,16 @@ Read-Only:
 - `body` (String)
 - `name` (String)
 - `return_type` (String)
-- `signature` (String)
+- `signature` (List of Object) (see [below for nested schema](#nestedobjatt--row_access_policies--describe_output--signature))
+
+<a id="nestedobjatt--row_access_policies--describe_output--signature"></a>
+### Nested Schema for `row_access_policies.describe_output.signature`
+
+Read-Only:
+
+- `name` (String)
+- `type` (String)
+
 
 
 <a id="nestedobjatt--row_access_policies--show_output"></a>

docs/guides/identifiers.md

@@ -12,13 +12,19 @@ With the combination of quotes, old parsing methods, and other factors, it was a
 
 For example, instead of writing
 
-```object_name = “\”${snowflake_table.database}\”.\”${snowflake_table.schema}\”.\”${snowflake_table.name}\””```
+```
+object_name = “\”${snowflake_table.database}\”.\”${snowflake_table.schema}\”.\”${snowflake_table.name}\””
+# for procedures
+object_name = “\”${snowflake_procedure.database}\”.\”${snowflake_procedure.schema}\”.\”${snowflake_procedure.name}(NUMBER, VARCHAR)\””
+```
 
 now we can write
 
-```object_name = snowflake_table.fully_qualified_name```
+```
+object_name = snowflake_table.fully_qualified_name
+```
 
-This is our recommended way of referencing other objects. However, if you don't manage table in Terraform, you can construct the proper id yourself like before: `"\"database_name\".\"schema_name\".\"table_name\""` Note that quotes are necessary for correct parsing of an identifier.
+This is our recommended way of referencing other objects. However, if you don't manage the referenced object in Terraform, you can construct the proper id yourself like before: `"\"database_name\".\"schema_name\".\"object_name\""` for schema-level objects, or `"\"database_name\".\"schema_name\".\"procedure_name(NUMBER, VARCHAR)\""` for procedures. Note that quotes are necessary for correct parsing of an identifier.
 
 This change was announced in v0.95.0 [migration guide](https://github.com/Snowflake-Labs/terraform-provider-snowflake/blob/main/MIGRATION_GUIDE.md#new-fully_qualified_name-field-in-the-resources).
 

docs/resources/row_access_policy.md

@@ -76,7 +76,16 @@ Read-Only:
 - `body` (String)
 - `name` (String)
 - `return_type` (String)
-- `signature` (String)
+- `signature` (List of Object) (see [below for nested schema](#nestedobjatt--describe_output--signature))
+
+<a id="nestedobjatt--describe_output--signature"></a>
+### Nested Schema for `describe_output.signature`
+
+Read-Only:
+
+- `name` (String)
+- `type` (String)
+
 
 
 <a id="nestedatt--show_output"></a>

docs/resources/view.md

@@ -98,7 +98,7 @@ SQL
 
 - `aggregation_policy` (Block List, Max: 1) Specifies the aggregation policy to set on a view. (see [below for nested schema](#nestedblock--aggregation_policy))
 - `change_tracking` (String) Specifies to enable or disable change tracking on the table. Available options are: "true" or "false". When the value is not set in the configuration the provider will put "default" there which means to use the Snowflake default for this value.
-- `column` (Block List) If you want to change the name of a column or add a comment to a column in the new view, include a column list that specifies the column names and (if needed) comments about the columns. (You do not need to specify the data types of the columns.) (see [below for nested schema](#nestedblock--column))
+- `column` (Block List) If you want to change the name of a column or add a comment to a column in the new view, include a column list that specifies the column names and (if needed) comments about the columns. You do not need to specify the data types of the columns. If this field is not specified, columns are inferred from the `statement` field by Snowflake. (see [below for nested schema](#nestedblock--column))
 - `comment` (String) Specifies a comment for the view.
 - `copy_grants` (Boolean) Retains the access permissions from the original view when a new view is created using the OR REPLACE clause.
 - `data_metric_function` (Block Set) Data metric functions used for the view. (see [below for nested schema](#nestedblock--data_metric_function))

pkg/acceptance/bettertestspoc/assert/resourceassert/row_access_policy_resource_ext.go

@@ -12,7 +12,7 @@ func (r *RowAccessPolicyResourceAssert) HasArguments(args []sdk.RowAccessPolicyA
 	r.AddAssertion(assert.ValueSet("argument.#", strconv.FormatInt(int64(len(args)), 10)))
 	for i, v := range args {
 		r.AddAssertion(assert.ValueSet(fmt.Sprintf("argument.%d.name", i), v.Name))
-		r.AddAssertion(assert.ValueSet(fmt.Sprintf("argument.%d.type", i), v.Type))
+		r.AddAssertion(assert.ValueSet(fmt.Sprintf("argument.%d.type", i), string(v.Type)))
 	}
 	return r
 }

pkg/acceptance/bettertestspoc/config/model/row_access_policy_model_ext.go

@@ -11,7 +11,7 @@ func (r *RowAccessPolicyModel) WithArgument(argument []sdk.RowAccessPolicyArgume
 	for i, v := range argument {
 		maps[i] = config.MapVariable(map[string]config.Variable{
 			"name": config.StringVariable(v.Name),
-			"type": config.StringVariable(v.Type),
+			"type": config.StringVariable(string(v.Type)),
 		})
 	}
 	r.Argument = tfconfig.SetVariable(maps...)

pkg/datasources/row_access_policies_acceptance_test.go

@@ -27,11 +27,11 @@ func TestAcc_RowAccessPolicies(t *testing.T) {
 	policyModel := model.RowAccessPolicy("test", []sdk.RowAccessPolicyArgument{
 		{
 			Name: "a",
-			Type: string(sdk.DataTypeVARCHAR),
+			Type: sdk.DataTypeVARCHAR,
 		},
 		{
 			Name: "b",
-			Type: string(sdk.DataTypeVARCHAR),
+			Type: sdk.DataTypeVARCHAR,
 		},
 	}, body, id.DatabaseName(), id.Name(), id.SchemaName()).WithComment("foo")
 
@@ -64,7 +64,11 @@ func TestAcc_RowAccessPolicies(t *testing.T) {
 					assert.Check(resource.TestCheckResourceAttr(dsName, "row_access_policies.0.describe_output.0.body", "case when current_role() in ('ANALYST') then true else false end")),
 					assert.Check(resource.TestCheckResourceAttr(dsName, "row_access_policies.0.describe_output.0.name", id.Name())),
 					assert.Check(resource.TestCheckResourceAttr(dsName, "row_access_policies.0.describe_output.0.return_type", "BOOLEAN")),
-					assert.Check(resource.TestCheckResourceAttr(dsName, "row_access_policies.0.describe_output.0.signature", "(a VARCHAR, b VARCHAR)")),
+					assert.Check(resource.TestCheckResourceAttr(dsName, "row_access_policies.0.describe_output.0.signature.#", "2")),
+					assert.Check(resource.TestCheckResourceAttr(dsName, "row_access_policies.0.describe_output.0.signature.0.name", "a")),
+					assert.Check(resource.TestCheckResourceAttr(dsName, "row_access_policies.0.describe_output.0.signature.0.type", string(sdk.DataTypeVARCHAR))),
+					assert.Check(resource.TestCheckResourceAttr(dsName, "row_access_policies.0.describe_output.0.signature.1.name", "b")),
+					assert.Check(resource.TestCheckResourceAttr(dsName, "row_access_policies.0.describe_output.0.signature.1.type", string(sdk.DataTypeVARCHAR))),
 				),
 			},
 			{
@@ -107,7 +111,7 @@ func TestAcc_RowAccessPolicies_Filtering(t *testing.T) {
 		"arguments": config.SetVariable(
 			config.MapVariable(map[string]config.Variable{
 				"name": config.StringVariable("a"),
-				"type": config.StringVariable("VARCHAR"),
+				"type": config.StringVariable(string(sdk.DataTypeVARCHAR)),
 			}),
 		),
 		"body": config.StringVariable("case when current_role() in ('ANALYST') then true else false end"),

pkg/resources/diff_suppressions.go

@@ -221,3 +221,20 @@ func suppressIdentifierQuoting(_, oldValue, newValue string, _ *schema.ResourceD
 	}
 	return slices.Equal(oldId, newId)
 }
+
+// IgnoreNewEmptyListOrSubfields suppresses the diff if `new` list is empty or compared subfield is ignored. Subfields can be nested.
+func IgnoreNewEmptyListOrSubfields(ignoredSubfields ...string) schema.SchemaDiffSuppressFunc {
+	return func(k, old, new string, _ *schema.ResourceData) bool {
+		parts := strings.SplitN(k, ".", 3)
+		if len(parts) < 2 {
+			log.Printf("[DEBUG] invalid resource key: %s", parts)
+			return false
+		}
+		// key is element count
+		if parts[1] == "#" && new == "0" {
+			return true
+		}
+		// key is one of the ignored subfields
+		return len(parts) == 3 && slices.Contains(ignoredSubfields, parts[2]) && new == ""
+	}
+}

pkg/resources/diff_suppressions_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func Test_NormalizeAndCompare(t *testing.T) {
@@ -150,3 +151,56 @@ func Test_NormalizeAndCompareIdentifiersSet(t *testing.T) {
 		// assert.True(t, resources.NormalizeAndCompareIdentifiersInSet("value")("value.doesnt_matter", "", "SCHEMA.OBJECT.IDENTIFIER", resourceData))
 	})
 }
+
+func Test_ignoreNewEmptyList(t *testing.T) {
+	tests := []struct {
+		name      string
+		subfields []string
+		key       string
+		old       string
+		new       string
+		suppress  bool
+	}{
+		{
+			name:     "suppress on zero count",
+			key:      "a.#",
+			old:      "5",
+			new:      "0",
+			suppress: true,
+		},
+		{
+			name:      "suppress on ignored field",
+			key:       "a.0.b",
+			subfields: []string{"b"},
+			suppress:  true,
+		},
+		{
+			name:      "suppress on nested ignored field",
+			key:       "a.0.b.c.d",
+			subfields: []string{"b.c.d"},
+			suppress:  true,
+		},
+		{
+			name:     "do not suppress on non-zero count",
+			key:      "a.#",
+			new:      "5",
+			suppress: false,
+		},
+		{
+			name:      "do not suppress on non-ignored field",
+			key:       "a.0.b",
+			subfields: []string{"c"},
+			suppress:  false,
+		},
+		{
+			name:     "do not suppress on invalid key",
+			key:      "a",
+			suppress: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			require.Equal(t, tt.suppress, resources.IgnoreNewEmptyListOrSubfields(tt.subfields...)(tt.key, tt.old, tt.new, nil))
+		})
+	}
+}

pkg/resources/row_access_policy.go

@@ -156,11 +156,7 @@ func ImportRowAccessPolicy(ctx context.Context, d *schema.ResourceData, meta any
 	if err := d.Set("body", policyDescription.Body); err != nil {
 		return nil, err
 	}
-	args, err := policyDescription.Arguments()
-	if err != nil {
-		return nil, err
-	}
-	if err := d.Set("argument", schemas.RowAccessPolicyArgumentsToSchema(args)); err != nil {
+	if err := d.Set("argument", schemas.RowAccessPolicyArgumentsToSchema(policyDescription.Signature)); err != nil {
 		return nil, err
 	}
 	return []*schema.ResourceData{d}, nil
@@ -233,11 +229,7 @@ func ReadRowAccessPolicy(ctx context.Context, d *schema.ResourceData, meta any)
 	if err := d.Set("body", rowAccessPolicyDescription.Body); err != nil {
 		return diag.FromErr(err)
 	}
-	args, err := rowAccessPolicyDescription.Arguments()
-	if err != nil {
-		return diag.FromErr(err)
-	}
-	if err := d.Set("argument", schemas.RowAccessPolicyArgumentsToSchema(args)); err != nil {
+	if err := d.Set("argument", schemas.RowAccessPolicyArgumentsToSchema(rowAccessPolicyDescription.Signature)); err != nil {
 		return diag.FromErr(err)
 	}
 	if err = d.Set(ShowOutputAttributeName, []map[string]any{schemas.RowAccessPolicyToSchema(rowAccessPolicy)}); err != nil {

pkg/resources/row_access_policy_acceptance_test.go

@@ -29,21 +29,21 @@ func TestAcc_RowAccessPolicy(t *testing.T) {
 	argument := []sdk.RowAccessPolicyArgument{
 		{
 			Name: "A",
-			Type: string(sdk.DataTypeVARCHAR),
+			Type: sdk.DataTypeVARCHAR,
 		},
 		{
 			Name: "B",
-			Type: string(sdk.DataTypeVARCHAR),
+			Type: sdk.DataTypeVARCHAR,
 		},
 	}
 	changedArgument := []sdk.RowAccessPolicyArgument{
 		{
 			Name: "C",
-			Type: string(sdk.DataTypeBoolean),
+			Type: sdk.DataTypeBoolean,
 		},
 		{
 			Name: "D",
-			Type: string(sdk.DataTypeTimestampNTZ),
+			Type: sdk.DataTypeTimestampNTZ,
 		},
 	}
 	policyModel := model.RowAccessPolicy("test", argument, body, id.DatabaseName(), id.Name(), id.SchemaName()).WithComment("Terraform acceptance test")
@@ -80,7 +80,11 @@ func TestAcc_RowAccessPolicy(t *testing.T) {
 					assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.body", body)),
 					assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.name", id.Name())),
 					assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.return_type", "BOOLEAN")),
-					assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.signature", "(A VARCHAR, B VARCHAR)")),
+					assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.signature.#", "2")),
+					assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.signature.0.name", "A")),
+					assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.signature.0.type", string(sdk.DataTypeVARCHAR))),
+					assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.signature.1.name", "B")),
+					assert.Check(resource.TestCheckResourceAttr(resourceName, "describe_output.0.signature.1.type", string(sdk.DataTypeVARCHAR))),
 				),
 			},
 			// change comment and expression
@@ -194,7 +198,7 @@ func TestAcc_RowAccessPolicy_Issue2053(t *testing.T) {
 	policyModel := model.RowAccessPolicy("test", []sdk.RowAccessPolicyArgument{
 		{
 			Name: "A",
-			Type: string(sdk.DataTypeVARCHAR),
+			Type: sdk.DataTypeVARCHAR,
 		},
 	}, body, id.DatabaseName(), id.Name(), id.SchemaName())
 	resource.Test(t, resource.TestCase{
@@ -252,7 +256,7 @@ func TestAcc_RowAccessPolicy_Rename(t *testing.T) {
 	policyModel := model.RowAccessPolicy("test", []sdk.RowAccessPolicyArgument{
 		{
 			Name: "a",
-			Type: string(sdk.DataTypeVARCHAR),
+			Type: sdk.DataTypeVARCHAR,
 		},
 	}, body, id.DatabaseName(), id.Name(), id.SchemaName())
 
@@ -390,7 +394,7 @@ func TestAcc_RowAccessPolicy_DataTypeAliases(t *testing.T) {
 					HasArguments([]sdk.RowAccessPolicyArgument{
 						{
 							Name: "A",
-							Type: string(sdk.DataTypeVARCHAR),
+							Type: sdk.DataTypeVARCHAR,
 						},
 					}),
 				),
@@ -406,11 +410,11 @@ func TestAcc_view_migrateFromVersion_0_95_0_LowercaseArgName(t *testing.T) {
 	policyModel := model.RowAccessPolicy("test", []sdk.RowAccessPolicyArgument{
 		{
 			Name: "A",
-			Type: string(sdk.DataTypeVARCHAR),
+			Type: sdk.DataTypeVARCHAR,
 		},
 		{
 			Name: "b",
-			Type: string(sdk.DataTypeVARCHAR),
+			Type: sdk.DataTypeVARCHAR,
 		},
 	}, body, id.DatabaseName(), id.Name(), id.SchemaName())
 
@@ -441,8 +445,8 @@ func TestAcc_view_migrateFromVersion_0_95_0_LowercaseArgName(t *testing.T) {
 					HasSchemaString(id.SchemaName()).
 					HasFullyQualifiedNameString(id.FullyQualifiedName()),
 					assert.Check(resource.TestCheckResourceAttr(resourceName, "row_access_expression", body)),
-					assert.Check(resource.TestCheckResourceAttr(resourceName, "signature.A", "VARCHAR")),
-					assert.Check(resource.TestCheckResourceAttr(resourceName, "signature.B", "VARCHAR")),
+					assert.Check(resource.TestCheckResourceAttr(resourceName, "signature.A", string(sdk.DataTypeVARCHAR))),
+					assert.Check(resource.TestCheckResourceAttr(resourceName, "signature.B", string(sdk.DataTypeVARCHAR))),
 				),
 			},
 			{
@@ -466,11 +470,11 @@ func TestAcc_view_migrateFromVersion_0_95_0_LowercaseArgName(t *testing.T) {
 					HasArguments([]sdk.RowAccessPolicyArgument{
 						{
 							Name: "A",
-							Type: string(sdk.DataTypeVARCHAR),
+							Type: sdk.DataTypeVARCHAR,
 						},
 						{
 							Name: "b",
-							Type: string(sdk.DataTypeVARCHAR),
+							Type: sdk.DataTypeVARCHAR,
 						},
 					}),
 				),
@@ -486,11 +490,11 @@ func TestAcc_view_migrateFromVersion_0_95_0_UppercaseArgName(t *testing.T) {
 	policyModel := model.RowAccessPolicy("test", []sdk.RowAccessPolicyArgument{
 		{
 			Name: "A",
-			Type: string(sdk.DataTypeVARCHAR),
+			Type: sdk.DataTypeVARCHAR,
 		},
 		{
 			Name: "B",
-			Type: string(sdk.DataTypeVARCHAR),
+			Type: sdk.DataTypeVARCHAR,
 		},
 	}, body, id.DatabaseName(), id.Name(), id.SchemaName())
 
@@ -521,18 +525,15 @@ func TestAcc_view_migrateFromVersion_0_95_0_UppercaseArgName(t *testing.T) {
 					HasSchemaString(id.SchemaName()).
 					HasFullyQualifiedNameString(id.FullyQualifiedName()),
 					assert.Check(resource.TestCheckResourceAttr(resourceName, "row_access_expression", body)),
-					assert.Check(resource.TestCheckResourceAttr(resourceName, "signature.A", "VARCHAR")),
-					assert.Check(resource.TestCheckResourceAttr(resourceName, "signature.B", "VARCHAR")),
+					assert.Check(resource.TestCheckResourceAttr(resourceName, "signature.A", string(sdk.DataTypeVARCHAR))),
+					assert.Check(resource.TestCheckResourceAttr(resourceName, "signature.B", string(sdk.DataTypeVARCHAR))),
 				),
 			},
 			{
 				ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
 				ConfigDirectory:          acc.ConfigurationDirectory("TestAcc_RowAccessPolicy/basic"),
 				ConfigVariables:          tfconfig.ConfigVariablesFromModel(t, policyModel),
 				ConfigPlanChecks: resource.ConfigPlanChecks{
-					PreApply: []plancheck.PlanCheck{
-						plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionNoop),
-					},
 					PostApplyPostRefresh: []plancheck.PlanCheck{
 						plancheck.ExpectResourceAction(resourceName, plancheck.ResourceActionNoop),
 					},
@@ -546,11 +547,11 @@ func TestAcc_view_migrateFromVersion_0_95_0_UppercaseArgName(t *testing.T) {
 					HasArguments([]sdk.RowAccessPolicyArgument{
 						{
 							Name: "A",
-							Type: string(sdk.DataTypeVARCHAR),
+							Type: sdk.DataTypeVARCHAR,
 						},
 						{
 							Name: "B",
-							Type: string(sdk.DataTypeVARCHAR),
+							Type: sdk.DataTypeVARCHAR,
 						},
 					}),
 				),

pkg/resources/testdata/TestAcc_View/basic/test.tf

@@ -5,7 +5,7 @@ resource "snowflake_view" "test" {
   statement = var.statement
 
   dynamic "column" {
-    for_each = var.columns
+    for_each = var.column
     content {
       column_name = column.value["column_name"]
     }