diff --git a/MIGRATION_GUIDE.md b/MIGRATION_GUIDE.md index 7eb2a326fb..a2c42bb525 100644 --- a/MIGRATION_GUIDE.md +++ b/MIGRATION_GUIDE.md @@ -41,6 +41,11 @@ It is noted as a behavior change but in some way it is not; with the previous im We will consider adding `NOT NULL` back because it can be set by `ALTER COLUMN columnX SET NOT NULL`, but first we want to revisit the whole resource design. +#### *(behavior change)* table_id reference +The docs were inconsistent. Example prior to 0.86.0 version showed using the `table.id` as the `table_id` reference. The description of the `table_id` parameter never allowed such a value (`table.id` is a `|`-delimited identifier representation and only the `.`-separated values were listed in the docs: https://registry.terraform.io/providers/Snowflake-Labs/snowflake/0.85.0/docs/resources/table_constraint#required. The misuse of `table.id` parameter will result in error after migrating to 0.86.0. To make the config work, please remove and reimport the constraint resource from the state as described in [resource migration doc](./docs/technical-documentation/resource_migration.md). + +After discussions in [#2535](https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2535) we decided to provide a temporary workaround in 0.87.0 version, so that the manual migration is not necessary. It allows skipping the migration and jumping straight to 0.87.0 version. However, the temporary workaround will be gone in one of the future versions. Please adjust to the newly suggested reference with the new resources you create. + ### snowflake_external_function resource changes #### *(behavior change)* return_null_allowed default is now true diff --git a/Makefile b/Makefile index a1a64c9adf..a0a9292bd7 100644 --- a/Makefile +++ b/Makefile @@ -64,7 +64,7 @@ test: test-client ## run unit and integration tests go test -v -cover -timeout=30m ./... test-acceptance: ## run acceptance tests - TF_ACC=1 go test -run "^TestAcc_" -v -cover -timeout=45m ./... + TF_ACC=1 go test -run "^TestAcc_" -v -cover -timeout=60m ./... test-integration: ## run SDK integration tests go test -run "^TestInt_" -v -cover -timeout=30m ./... diff --git a/docs/resources/table_constraint.md b/docs/resources/table_constraint.md index a149e609d9..171febc02c 100644 --- a/docs/resources/table_constraint.md +++ b/docs/resources/table_constraint.md @@ -105,7 +105,7 @@ resource "snowflake_table_constraint" "unique" { - `columns` (List of String) Columns to use in constraint key - `name` (String) Name of constraint -- `table_id` (String) Identifier for table to create constraint on. Format must follow: "\"\".\"\".\"\"" or ".." (snowflake_table.my_table.id) +- `table_id` (String) Identifier for table to create constraint on. Format must follow: "\"<db_name>\".\"<schema_name>\".\"<table_name>\"" or "<db_name>.<schema_name>.<table_name>" (snowflake_table.my_table.id) - `type` (String) Type of constraint, one of 'UNIQUE', 'PRIMARY KEY', or 'FOREIGN KEY' ### Optional diff --git a/pkg/resources/table_constraint.go b/pkg/resources/table_constraint.go index abc9ceb489..6a2de92c94 100644 --- a/pkg/resources/table_constraint.go +++ b/pkg/resources/table_constraint.go @@ -41,7 +41,7 @@ var tableConstraintSchema = map[string]*schema.Schema{ Type: schema.TypeString, Required: true, ForceNew: true, - Description: `Identifier for table to create constraint on. Format must follow: "\"\".\"\".\"\"" or ".." (snowflake_table.my_table.id)`, + Description: `Identifier for table to create constraint on. Format must follow: "\"<db_name>\".\"<schema_name>\".\"<table_name>\"" or "<db_name>.<schema_name>.<table_name>" (snowflake_table.my_table.id)`, }, "columns": { Type: schema.TypeList, @@ -208,7 +208,15 @@ func (v *tableConstraintID) parse(s string) { } func getTableIdentifier(s string) (*sdk.SchemaObjectIdentifier, error) { - objectIdentifier, err := helpers.DecodeSnowflakeParameterID(s) + var objectIdentifier sdk.ObjectIdentifier + var err error + // TODO [SNOW-999049]: Fallback for old implementations using table.id instead of table.qualified_name - probably will be removed later. + if strings.Contains(s, "|") { + objectIdentifier = helpers.DecodeSnowflakeID(s) + } else { + objectIdentifier, err = helpers.DecodeSnowflakeParameterID(s) + } + if err != nil { return nil, fmt.Errorf("table id is incorrect: %s, err: %w", objectIdentifier, err) } diff --git a/pkg/resources/table_constraint_acceptance_test.go b/pkg/resources/table_constraint_acceptance_test.go index efdf4b5d08..dc3499108c 100644 --- a/pkg/resources/table_constraint_acceptance_test.go +++ b/pkg/resources/table_constraint_acceptance_test.go @@ -2,6 +2,8 @@ package resources_test import ( "fmt" + "regexp" + "strings" "testing" acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance" @@ -131,3 +133,126 @@ resource "snowflake_table_constraint" "unique" { `, n, databaseName, schemaName, n) } + +// proves issue https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2535 +func TestAcc_Table_issue2535_newConstraint(t *testing.T) { + accName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: nil, + Steps: []resource.TestStep{ + { + ExternalProviders: map[string]resource.ExternalProvider{ + "snowflake": { + VersionConstraint: "=0.86.0", + Source: "Snowflake-Labs/snowflake", + }, + }, + Config: tableConstraintUniqueConfigUsingTableId(accName, acc.TestDatabaseName, acc.TestSchemaName), + ExpectError: regexp.MustCompile(`.*table id is incorrect.*`), + }, + { + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + Config: tableConstraintUniqueConfigUsingTableId(accName, acc.TestDatabaseName, acc.TestSchemaName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_table_constraint.unique", "type", "UNIQUE"), + ), + }, + }, + }) +} + +// proves issue https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2535 +func TestAcc_Table_issue2535_existingTable(t *testing.T) { + accName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha)) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { acc.TestAccPreCheck(t) }, + TerraformVersionChecks: []tfversion.TerraformVersionCheck{ + tfversion.RequireAbove(tfversion.Version1_5_0), + }, + CheckDestroy: nil, + Steps: []resource.TestStep{ + // reference done by table.id in 0.85.0 + { + ExternalProviders: map[string]resource.ExternalProvider{ + "snowflake": { + VersionConstraint: "=0.85.0", + Source: "Snowflake-Labs/snowflake", + }, + }, + Config: tableConstraintUniqueConfigUsingTableId(accName, acc.TestDatabaseName, acc.TestSchemaName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_table_constraint.unique", "type", "UNIQUE"), + ), + }, + // switched to qualified_name in 0.86.0 + { + ExternalProviders: map[string]resource.ExternalProvider{ + "snowflake": { + VersionConstraint: "=0.86.0", + Source: "Snowflake-Labs/snowflake", + }, + }, + Config: tableConstraintUniqueConfigUsingFullyQualifiedName(accName, acc.TestDatabaseName, acc.TestSchemaName), + ExpectError: regexp.MustCompile(`.*table id is incorrect.*`), + }, + // fixed in the current version + { + ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories, + Config: tableConstraintUniqueConfigUsingFullyQualifiedName(accName, acc.TestDatabaseName, acc.TestSchemaName), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("snowflake_table_constraint.unique", "type", "UNIQUE"), + ), + }, + }, + }) +} + +func tableConstraintUniqueConfigUsingFullyQualifiedName(n string, databaseName string, schemaName string) string { + return fmt.Sprintf(` +resource "snowflake_table" "t" { + name = "%s" + database = "%s" + schema = "%s" + + column { + name = "col1" + type = "NUMBER(38,0)" + } +} + +resource "snowflake_table_constraint" "unique" { + name = "%s" + type = "UNIQUE" + table_id = snowflake_table.t.qualified_name + columns = ["col1"] +} +`, n, databaseName, schemaName, n) +} + +func tableConstraintUniqueConfigUsingTableId(n string, databaseName string, schemaName string) string { + return fmt.Sprintf(` +resource "snowflake_table" "t" { + name = "%s" + database = "%s" + schema = "%s" + + column { + name = "col1" + type = "NUMBER(38,0)" + } +} + +resource "snowflake_table_constraint" "unique" { + name = "%s" + type = "UNIQUE" + table_id = snowflake_table.t.id + columns = ["col1"] +} +`, n, databaseName, schemaName, n) +}