diff --git a/docs/data-sources/row_access_policies.md b/docs/data-sources/row_access_policies.md index 076c8b87f9..b6ccb31cd8 100644 --- a/docs/data-sources/row_access_policies.md +++ b/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)) + + +### Nested Schema for `row_access_policies.describe_output.signature` + +Read-Only: + +- `name` (String) +- `type` (String) + diff --git a/docs/guides/identifiers.md b/docs/guides/identifiers.md index 94def58363..1813be44e0 100644 --- a/docs/guides/identifiers.md +++ b/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). diff --git a/docs/resources/row_access_policy.md b/docs/resources/row_access_policy.md index c11557428e..5e0c651176 100644 --- a/docs/resources/row_access_policy.md +++ b/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)) + + +### Nested Schema for `describe_output.signature` + +Read-Only: + +- `name` (String) +- `type` (String) + diff --git a/docs/resources/view.md b/docs/resources/view.md index ab2682b498..ba07139c62 100644 --- a/docs/resources/view.md +++ b/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)) 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 index 725e0012ea..ec54107493 100644 --- 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 index fc11800ba3..848c07ee5c 100644 --- 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 index eff2a614f9..66352d1a71 100644 --- 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 index 7f60139b5e..3e23be3ee0 100644 --- a/pkg/resources/diff_suppressions.go +++ b/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 == "" + } +} diff --git a/pkg/resources/diff_suppressions_test.go b/pkg/resources/diff_suppressions_test.go index f7ff8cc131..7c54f03938 100644 --- a/pkg/resources/diff_suppressions_test.go +++ b/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)) + }) + } +} diff --git a/pkg/resources/row_access_policy.go b/pkg/resources/row_access_policy.go index afe4d7ba12..3c07409f32 100644 --- 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 index 796f1df167..f8e7b89812 100644 --- 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))), ), }, { @@ -530,9 +534,6 @@ func TestAcc_view_migrateFromVersion_0_95_0_UppercaseArgName(t *testing.T) { 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, }, }), ), diff --git a/pkg/resources/testdata/TestAcc_View/basic/test.tf b/pkg/resources/testdata/TestAcc_View/basic/test.tf index 905a79328a..e61457cfac 100644 --- 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 index 2219f130a5..dc7d599d87 100644 --- 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 index cdc7295f9d..7586548757 100644 --- 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 index 86e63c4564..71a25def0e 100644 --- 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 index 42308b4d48..54dc35e75e 100644 --- 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 index b38898bbcf..e3c42edcee 100644 --- 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 index 2df97a8f62..fd96e26315 100644 --- 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 index 4b814dd06d..2a299e3bdb 100644 --- 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 index 0d66abadea..d9f2f6bb51 100644 --- 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: IgnoreNewEmptyListOrSubfields("column_name"), }, "comment": { Type: schema.TypeString, diff --git a/pkg/resources/view_acceptance_test.go b/pkg/resources/view_acceptance_test.go index cf29e6b78f..9c00b77092 100644 --- a/pkg/resources/view_acceptance_test.go +++ b/pkg/resources/view_acceptance_test.go @@ -5,21 +5,20 @@ import ( "regexp" "testing" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/objectassert" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections" - - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" - acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" accconfig "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config" - "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testenvs" + tfconfig "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/objectassert" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/resourceassert" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/assert/resourceshowoutputassert" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/bettertestspoc/config/model" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/importchecks" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/testenvs" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers" + "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/snowflakeroles" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/provider/resources" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk" @@ -71,7 +70,7 @@ func TestAcc_View_basic(t *testing.T) { "database": config.StringVariable(id.DatabaseName()), "schema": config.StringVariable(id.SchemaName()), "statement": config.StringVariable(configStatement), - "columns": config.SetVariable( + "column": config.SetVariable( config.MapVariable(map[string]config.Variable{ "column_name": config.StringVariable("ID"), }), @@ -100,7 +99,7 @@ func TestAcc_View_basic(t *testing.T) { "data_metric_schedule_using_cron": config.StringVariable(cron), "comment": config.StringVariable(comment), "schedule_status": config.StringVariable(string(scheduleStatus)), - "columns": config.SetVariable( + "column": config.SetVariable( config.MapVariable(map[string]config.Variable{ "column_name": config.StringVariable("ID"), }), @@ -427,7 +426,7 @@ func TestAcc_View_recursive(t *testing.T) { "schema": config.StringVariable(id.SchemaName()), "statement": config.StringVariable(statement), "is_recursive": config.BoolVariable(true), - "columns": config.SetVariable( + "column": config.SetVariable( config.MapVariable(map[string]config.Variable{ "column_name": config.StringVariable("ROLE_NAME"), }), @@ -705,7 +704,7 @@ end;; "database": config.StringVariable(id.DatabaseName()), "schema": config.StringVariable(id.SchemaName()), "statement": config.StringVariable(statement), - "columns": config.SetVariable( + "column": config.SetVariable( collections.Map(columns, func(columnName string) config.Variable { return config.MapVariable(map[string]config.Variable{ "column_name": config.StringVariable(columnName), @@ -717,7 +716,7 @@ end;; basicViewWithPolicies := func() config.Variables { conf := basicView("ID", "FOO") - delete(conf, "columns") + delete(conf, "column") conf["projection_name"] = config.StringVariable(projectionPolicy.FullyQualifiedName()) conf["masking_name"] = config.StringVariable(maskingPolicy.ID().FullyQualifiedName()) conf["masking_using"] = config.ListVariable(config.StringVariable("ID")) @@ -820,7 +819,7 @@ func TestAcc_View_Rename(t *testing.T) { "database": config.StringVariable(identifier.DatabaseName()), "schema": config.StringVariable(identifier.SchemaName()), "statement": config.StringVariable(statement), - "columns": config.SetVariable( + "column": config.SetVariable( config.MapVariable(map[string]config.Variable{ "column_name": config.StringVariable("ROLE_NAME"), }), @@ -865,6 +864,67 @@ func TestAcc_View_Rename(t *testing.T) { }) } +func TestAcc_View_Issue3073(t *testing.T) { + t.Setenv(string(testenvs.ConfigureClientOnce), "") + statement := "SELECT ROLE_NAME, ROLE_OWNER FROM INFORMATION_SCHEMA.APPLICABLE_ROLES" + id := acc.TestClient().Ids.RandomSchemaObjectIdentifier() + viewModel := model.View("test", id.DatabaseName(), id.Name(), id.SchemaName(), statement) + viewModelWithColumns := model.View("test", id.DatabaseName(), id.Name(), id.SchemaName(), statement).WithColumnValue(config.SetVariable( + config.MapVariable(map[string]config.Variable{ + "column_name": config.StringVariable("ROLE_NAME"), + }), + config.MapVariable(map[string]config.Variable{ + "column_name": config.StringVariable("ROLE_OWNER"), + }), + )) + + 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.View), + Steps: []resource.TestStep{ + { + Config: accconfig.FromModel(t, viewModel), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPostRefresh: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("snowflake_view.test", plancheck.ResourceActionNoop), + }, + }, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_view.test", "name", id.Name()), + ), + }, + // specify the columns + { + ConfigDirectory: acc.ConfigurationDirectory("TestAcc_View/basic"), + ConfigVariables: tfconfig.ConfigVariablesFromModel(t, viewModelWithColumns), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPostRefresh: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("snowflake_view.test", plancheck.ResourceActionNoop), + }, + }, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_view.test", "name", id.Name()), + ), + }, + { + Config: accconfig.FromModel(t, viewModel), + ConfigPlanChecks: resource.ConfigPlanChecks{ + PostApplyPostRefresh: []plancheck.PlanCheck{ + plancheck.ExpectResourceAction("snowflake_view.test", plancheck.ResourceActionNoop), + }, + }, + Check: resource.ComposeAggregateTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_view.test", "name", id.Name()), + ), + }, + }, + }) +} + func TestAcc_ViewChangeCopyGrants(t *testing.T) { t.Setenv(string(testenvs.ConfigureClientOnce), "") id := acc.TestClient().Ids.RandomSchemaObjectIdentifier() @@ -878,7 +938,7 @@ func TestAcc_ViewChangeCopyGrants(t *testing.T) { "statement": config.StringVariable(statement), "copy_grants": config.BoolVariable(copyGrants), "is_secure": config.BoolVariable(true), - "columns": config.SetVariable( + "column": config.SetVariable( config.MapVariable(map[string]config.Variable{ "column_name": config.StringVariable("ID"), }), @@ -946,7 +1006,7 @@ func TestAcc_ViewChangeCopyGrantsReversed(t *testing.T) { "statement": config.StringVariable(statement), "copy_grants": config.BoolVariable(copyGrants), "is_secure": config.BoolVariable(true), - "columns": config.SetVariable( + "column": config.SetVariable( config.MapVariable(map[string]config.Variable{ "column_name": config.StringVariable("ID"), }), @@ -1098,7 +1158,7 @@ func TestAcc_view_migrateFromVersion_0_94_1(t *testing.T) { "database": config.StringVariable(id.DatabaseName()), "schema": config.StringVariable(id.SchemaName()), "statement": config.StringVariable(statement), - "columns": config.SetVariable( + "column": config.SetVariable( config.MapVariable(map[string]config.Variable{ "column_name": config.StringVariable("ROLE_NAME"), }), diff --git a/pkg/schemas/row_access_policy.go b/pkg/schemas/row_access_policy.go index ddb04f49aa..578f74c830 100644 --- a/pkg/schemas/row_access_policy.go +++ b/pkg/schemas/row_access_policy.go @@ -11,7 +11,19 @@ var RowAccessPolicyDescribeSchema = map[string]*schema.Schema{ Computed: true, }, "signature": { - Type: schema.TypeString, + Type: schema.TypeList, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "name": { + Type: schema.TypeString, + Computed: true, + }, + "type": { + Type: schema.TypeString, + Computed: true, + }, + }, + }, Computed: true, }, "return_type": { @@ -25,9 +37,16 @@ var RowAccessPolicyDescribeSchema = map[string]*schema.Schema{ } func RowAccessPolicyDescriptionToSchema(description sdk.RowAccessPolicyDescription) map[string]any { + signatureElem := make([]map[string]any, len(description.Signature)) + for i, v := range description.Signature { + signatureElem[i] = map[string]any{ + "name": v.Name, + "type": string(v.Type), + } + } return map[string]any{ "name": description.Name, - "signature": description.Signature, + "signature": signatureElem, "return_type": description.ReturnType, "body": description.Body, } diff --git a/pkg/sdk/row_access_policies_gen.go b/pkg/sdk/row_access_policies_gen.go index df9d720e11..64ba7c4096 100644 --- a/pkg/sdk/row_access_policies_gen.go +++ b/pkg/sdk/row_access_policies_gen.go @@ -3,8 +3,6 @@ package sdk import ( "context" "database/sql" - "fmt" - "strings" ) type RowAccessPolicies interface { @@ -109,33 +107,11 @@ type describeRowAccessPolicyDBRow struct { type RowAccessPolicyDescription struct { Name string - Signature string + Signature []RowAccessPolicyArgument ReturnType string Body string } type RowAccessPolicyArgument struct { Name string - Type string -} - -// TODO(SNOW-1596962): Fully support VECTOR data type -// TODO(SNOW-1660588): Use ParseFunctionArgumentsFromString -func (d *RowAccessPolicyDescription) Arguments() ([]RowAccessPolicyArgument, error) { - // Format in database is `(column )` - plainSignature := strings.ReplaceAll(d.Signature, "(", "") - plainSignature = strings.ReplaceAll(plainSignature, ")", "") - signatureParts := strings.Split(plainSignature, ", ") - arguments := make([]RowAccessPolicyArgument, len(signatureParts)) - - for i, e := range signatureParts { - parts := strings.Split(e, " ") - if len(parts) < 2 { - return nil, fmt.Errorf("parsing policy arguments: expected argument name and type, got %s", e) - } - arguments[i] = RowAccessPolicyArgument{ - Name: strings.Join(parts[:len(parts)-1], " "), - Type: parts[len(parts)-1], - } - } - return arguments, nil + Type DataType } diff --git a/pkg/sdk/row_access_policies_gen_test.go b/pkg/sdk/row_access_policies_gen_test.go index f4590f3b9e..c45d5da68b 100644 --- a/pkg/sdk/row_access_policies_gen_test.go +++ b/pkg/sdk/row_access_policies_gen_test.go @@ -297,35 +297,11 @@ func TestRowAccessPolicyDescription_Arguments(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - d := &RowAccessPolicyDescription{ + d := &describeRowAccessPolicyDBRow{ Signature: tt.signature, } - got, err := d.Arguments() - require.NoError(t, err) - require.Equal(t, tt.want, got) - }) - } -} - -func TestRowAccessPolicyDescription_ArgumentsInvalid(t *testing.T) { - tests := []struct { - name string - signature string - err string - }{ - { - name: "signature without data type", - signature: "(A)", - err: "parsing policy arguments: expected argument name and type, got A", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - d := &RowAccessPolicyDescription{ - Signature: tt.signature, - } - _, err := d.Arguments() - require.ErrorContains(t, err, tt.err) + got := d.convert() + require.Equal(t, tt.want, got.Signature) }) } } diff --git a/pkg/sdk/row_access_policies_impl_gen.go b/pkg/sdk/row_access_policies_impl_gen.go index e008b5a62c..d02092e9cd 100644 --- a/pkg/sdk/row_access_policies_impl_gen.go +++ b/pkg/sdk/row_access_policies_impl_gen.go @@ -2,6 +2,8 @@ package sdk import ( "context" + "log" + "strings" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections" ) @@ -133,9 +135,34 @@ func (r *DescribeRowAccessPolicyRequest) toOpts() *DescribeRowAccessPolicyOption func (r describeRowAccessPolicyDBRow) convert() *RowAccessPolicyDescription { rowAccessPolicyDescription := &RowAccessPolicyDescription{ Name: r.Name, - Signature: r.Signature, ReturnType: r.ReturnType, Body: r.Body, } + // Format in database is `(column )` + // TODO(SNOW-1596962): Fully support VECTOR data type + // TODO(SNOW-1660588): Use ParseFunctionArgumentsFromString + plainSignature := strings.ReplaceAll(r.Signature, "(", "") + plainSignature = strings.ReplaceAll(plainSignature, ")", "") + signatureParts := strings.Split(plainSignature, ", ") + arguments := make([]RowAccessPolicyArgument, len(signatureParts)) + + for i, e := range signatureParts { + parts := strings.Split(e, " ") + if len(parts) < 2 { + log.Printf("[DEBUG] parsing policy arguments: expected argument name and type, got %s", e) + continue + } + dataType, err := ToDataType(parts[len(parts)-1]) + if err != nil { + log.Printf("[DEBUG] converting row access policy db row: invalid data type %s", dataType) + continue + } + arguments[i] = RowAccessPolicyArgument{ + Name: strings.Join(parts[:len(parts)-1], " "), + Type: dataType, + } + } + rowAccessPolicyDescription.Signature = arguments + return rowAccessPolicyDescription } diff --git a/pkg/sdk/testint/row_access_policies_gen_integration_test.go b/pkg/sdk/testint/row_access_policies_gen_integration_test.go index 87d1eb79fe..516b5c4787 100644 --- a/pkg/sdk/testint/row_access_policies_gen_integration_test.go +++ b/pkg/sdk/testint/row_access_policies_gen_integration_test.go @@ -1,7 +1,6 @@ package testint import ( - "fmt" "testing" "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance/helpers/random" @@ -28,11 +27,11 @@ func TestInt_RowAccessPolicies(t *testing.T) { assert.Equal(t, "ROLE", rowAccessPolicy.OwnerRoleType) } - assertRowAccessPolicyDescription := func(t *testing.T, rowAccessPolicyDescription *sdk.RowAccessPolicyDescription, id sdk.SchemaObjectIdentifier, expectedSignature string, expectedBody string) { + assertRowAccessPolicyDescription := func(t *testing.T, rowAccessPolicyDescription *sdk.RowAccessPolicyDescription, id sdk.SchemaObjectIdentifier, signature []sdk.RowAccessPolicyArgument, expectedBody string) { t.Helper() assert.Equal(t, sdk.RowAccessPolicyDescription{ Name: id.Name(), - Signature: expectedSignature, + Signature: signature, ReturnType: "BOOLEAN", Body: expectedBody, }, *rowAccessPolicyDescription) @@ -251,7 +250,10 @@ func TestInt_RowAccessPolicies(t *testing.T) { returnedRowAccessPolicyDescription, err := client.RowAccessPolicies.Describe(ctx, rowAccessPolicy.ID()) require.NoError(t, err) - assertRowAccessPolicyDescription(t, returnedRowAccessPolicyDescription, rowAccessPolicy.ID(), fmt.Sprintf("(%s %s)", argName, argType), body) + assertRowAccessPolicyDescription(t, returnedRowAccessPolicyDescription, rowAccessPolicy.ID(), []sdk.RowAccessPolicyArgument{{ + Name: argName, + Type: argType, + }}, body) }) t.Run("describe row access policy: with timestamp data type normalization", func(t *testing.T) { @@ -267,7 +269,10 @@ func TestInt_RowAccessPolicies(t *testing.T) { returnedRowAccessPolicyDescription, err := client.RowAccessPolicies.Describe(ctx, rowAccessPolicy.ID()) require.NoError(t, err) - assertRowAccessPolicyDescription(t, returnedRowAccessPolicyDescription, rowAccessPolicy.ID(), fmt.Sprintf("(%s %s)", argName, sdk.DataTypeTimestampNTZ), body) + assertRowAccessPolicyDescription(t, returnedRowAccessPolicyDescription, rowAccessPolicy.ID(), []sdk.RowAccessPolicyArgument{{ + Name: argName, + Type: sdk.DataTypeTimestampNTZ, + }}, body) }) t.Run("describe row access policy: with varchar data type normalization", func(t *testing.T) { @@ -283,7 +288,10 @@ func TestInt_RowAccessPolicies(t *testing.T) { returnedRowAccessPolicyDescription, err := client.RowAccessPolicies.Describe(ctx, rowAccessPolicy.ID()) require.NoError(t, err) - assertRowAccessPolicyDescription(t, returnedRowAccessPolicyDescription, rowAccessPolicy.ID(), fmt.Sprintf("(%s %s)", argName, sdk.DataTypeVARCHAR), body) + assertRowAccessPolicyDescription(t, returnedRowAccessPolicyDescription, rowAccessPolicy.ID(), []sdk.RowAccessPolicyArgument{{ + Name: argName, + Type: sdk.DataTypeVARCHAR, + }}, body) }) t.Run("describe row access policy: non-existing", func(t *testing.T) { @@ -368,7 +376,6 @@ func TestInt_RowAccessPoliciesDescribe(t *testing.T) { assert.Equal(t, "true", policyDetails.Body) assert.Equal(t, id.Name(), policyDetails.Name) assert.Equal(t, "BOOLEAN", policyDetails.ReturnType) - gotArgs, err := policyDetails.Arguments() require.NoError(t, err) wantArgs := make([]sdk.RowAccessPolicyArgument, len(args)) for i, arg := range args { @@ -376,9 +383,9 @@ func TestInt_RowAccessPoliciesDescribe(t *testing.T) { require.NoError(t, err) wantArgs[i] = sdk.RowAccessPolicyArgument{ Name: arg.Name, - Type: string(dataType), + Type: dataType, } } - assert.Equal(t, wantArgs, gotArgs) + assert.Equal(t, wantArgs, policyDetails.Signature) }) } diff --git a/templates/guides/identifiers.md.tmpl b/templates/guides/identifiers.md.tmpl index 94def58363..1813be44e0 100644 --- a/templates/guides/identifiers.md.tmpl +++ b/templates/guides/identifiers.md.tmpl @@ -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).