Fix views

Snowflake-Labs · Sep 16, 2024 · c22c035 · c22c035
1 parent 138a288
commit c22c035
Show file tree

Hide file tree

Showing 21 changed files with 182 additions and 125 deletions.
diff --git a/pkg/acceptance/bettertestspoc/assert/resourceassert/row_access_policy_resource_ext.go b/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
 }
diff --git a/pkg/acceptance/bettertestspoc/config/model/row_access_policy_model_ext.go b/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...)

diff --git a/pkg/datasources/row_access_policies_acceptance_test.go b/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"),

diff --git a/pkg/resources/diff_suppressions.go b/pkg/resources/diff_suppressions.go
@@ -221,3 +221,10 @@ func suppressIdentifierQuoting(_, oldValue, newValue string, _ *schema.ResourceD
 	}
 	return slices.Equal(oldId, newId)
 }
+
+func ignoreEmpty(k, oldValue, newValue string, _ *schema.ResourceData) bool {
+	if strings.HasSuffix(k, ".#") && newValue == "0" {
+		return true
+	}
+	return newValue == ""
+}
diff --git a/pkg/resources/row_access_policy.go b/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 {

diff --git a/pkg/resources/row_access_policy_acceptance_test.go b/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,8 +525,8 @@ 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))),
 				),
 			},
 			{
@@ -546,11 +550,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,
 						},
 					}),
 				),

diff --git a/pkg/resources/testdata/TestAcc_View/basic/test.tf b/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"]
     }

diff --git a/pkg/resources/testdata/TestAcc_View/basic/variables.tf b/pkg/resources/testdata/TestAcc_View/basic/variables.tf
@@ -14,6 +14,6 @@ variable "statement" {
   type = string
 }
 
-variable "columns" {
+variable "column" {
   type = set(map(string))
 }
diff --git a/pkg/resources/testdata/TestAcc_View/basic_copy_grants/test.tf b/pkg/resources/testdata/TestAcc_View/basic_copy_grants/test.tf
@@ -7,7 +7,7 @@ resource "snowflake_view" "test" {
   is_secure   = var.is_secure
 
   dynamic "column" {
-    for_each = var.columns
+    for_each = var.column
     content {
       column_name = column.value["column_name"]
     }

diff --git a/pkg/resources/testdata/TestAcc_View/basic_copy_grants/variables.tf b/pkg/resources/testdata/TestAcc_View/basic_copy_grants/variables.tf
@@ -22,6 +22,6 @@ variable "is_secure" {
   type = bool
 }
 
-variable "columns" {
+variable "column" {
   type = set(map(string))
 }
diff --git a/pkg/resources/testdata/TestAcc_View/basic_is_recursive/test.tf b/pkg/resources/testdata/TestAcc_View/basic_is_recursive/test.tf
@@ -6,7 +6,7 @@ resource "snowflake_view" "test" {
   is_recursive = var.is_recursive
 
   dynamic "column" {
-    for_each = var.columns
+    for_each = var.column
     content {
       column_name = column.value["column_name"]
     }

diff --git a/pkg/resources/testdata/TestAcc_View/basic_is_recursive/variables.tf b/pkg/resources/testdata/TestAcc_View/basic_is_recursive/variables.tf
@@ -18,6 +18,6 @@ variable "is_recursive" {
   type = bool
 }
 
-variable "columns" {
+variable "column" {
   type = set(map(string))
 }
diff --git a/pkg/resources/testdata/TestAcc_View/basic_update/test.tf b/pkg/resources/testdata/TestAcc_View/basic_update/test.tf
@@ -4,7 +4,7 @@ resource "snowflake_view" "test" {
   schema   = var.schema
 
   dynamic "column" {
-    for_each = var.columns
+    for_each = var.column
     content {
       column_name = column.value["column_name"]
     }

diff --git a/pkg/resources/testdata/TestAcc_View/basic_update/variables.tf b/pkg/resources/testdata/TestAcc_View/basic_update/variables.tf
@@ -50,6 +50,6 @@ variable "schedule_status" {
   type = string
 }
 
-variable "columns" {
+variable "column" {
   type = set(map(string))
 }
diff --git a/pkg/resources/view.go b/pkg/resources/view.go
@@ -194,7 +194,8 @@ var viewSchema = map[string]*schema.Schema{
 				},
 			},
 		},
-		Description: "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.)",
+		Description:      "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.",
+		DiffSuppressFunc: ignoreEmpty,
 	},
 	"comment": {
 		Type:        schema.TypeString,