changes after review

MIGRATION_GUIDE.md

@@ -66,6 +66,37 @@ As part of the [preparation for v1](https://github.com/Snowflake-Labs/terraform-
 - Shared database - can be used as `snowflake_shared_database` (used to create databases from externally defined shares)
 - Secondary database - can be used as `snowflake_secondary_database` (used to create replicas of databases from external sources)
 
+All the field changes in comparison to the previous database resource are:
+- `is_transient`
+    - in `snowflake_shared_database`
+        - removed: the field is removed from `snowflake_shared_database` as it doesn't have any effect on shared databases.
+- `from_database` - database cloning was entirely removed and is not possible by any of the new database resources.
+- `from_share` - the parameter was moved to the dedicated resource for databases created from shares `snowflake_shared_database`. Right now, it's a text field instead of a map. Additionally, instead of legacy account identifier format we're expecting the new one that with share looks like this: `<organization_name>.<account_name>.<share_name>`. For more information on account identifiers, visit the [official documentation](https://docs.snowflake.com/en/user-guide/admin-account-identifier).
+- p, 
+- `from_replication` - the parameter was moved to the dedicated resource for databases created from primary databases `snowflake_secondary_database`
+- `replication_configuration` - renamed: was renamed to `configuration` and is only available in the `snowflake_database`. Its internal schema changed that instead of list of accounts, we expect a list of nested objects with accounts for which replication (and optionally failover) should be enabled. More information about converting between both versions [here](#resource-renamed-snowflake_database---snowflake_database_old). Additionally, instead of legacy account identifier format we're expecting the new one that looks like this: `<organization_name>.<account_name>`. For more information on account identifiers, visit the [official documentation](https://docs.snowflake.com/en/user-guide/admin-account-identifier).
+- `data_retention_time_in_days` 
+  - in `snowflake_shared_database`
+      - removed: the field is removed from `snowflake_shared_database` as it doesn't have any effect on shared databases.
+  - in `snowflake_database` and `snowflake_secondary_database`
+    - adjusted: now, it uses different approach that won't set it to -1 as a default value, but rather fills the field with the current value from Snowflake (this still can change).
+- added: The following set of [parameters](https://docs.snowflake.com/en/sql-reference/parameters) was added to every database type:
+    - `max_data_extension_time_in_days`
+    - `external_volume`
+    - `catalog`
+    - `replace_invalid_characters`
+    - `default_ddl_collation`
+    - `storage_serialization_policy`
+    - `log_level`
+    - `trace_level`
+    - `suspend_task_after_num_failures`
+    - `task_auto_retry_attempts`
+    - `user_task_managed_initial_warehouse_size`
+    - `user_task_timeout_ms`
+    - `user_task_minimum_trigger_interval_in_seconds`
+    - `quoted_identifiers_ignore_case`
+    - `enable_console_output`
+
 The split was done (and will be done for several objects during the refactor) to simplify the resource on maintainability and usage level.
 Its purpose was also to divide the resources by their specific purpose rather than cramping every use case of an object into one resource.
 
@@ -125,7 +156,9 @@ The only difference would be that instead of writing/generating new configuratio
 - `pattern` was replaced by `like` field.
 - Additional filtering options added (`limit`).
 - Added missing fields returned by SHOW DATABASES.
-- Added outputs from DESC DATABASE and SHOW PARAMETERS IN DATABASE (they can be turned off by declaring `with_describe = false` and `with_parameters = false`, **they're turned on by default**).
+- Added outputs from **DESC DATABASE** and **SHOW PARAMETERS IN DATABASE** (they can be turned off by declaring `with_describe = false` and `with_parameters = false`, **they're turned on by default**). 
+The additional parameters call **DESC DATABASE** (with `with_describe` turned on) and **SHOW PARAMETERS IN DATABASE** (with `with_parameters` turned on) **per database** returned by **SHOW DATABASES**. 
+It's important to limit the records and calls to Snowflake to the minimum. That's why we recommend assessing which information you need from the data source and then providing strong filters and turning off additional fields for better plan performance.
 
 ## v0.89.0 ➞ v0.90.0
 ### snowflake_table resource changes

docs/resources/database_old.md

@@ -81,5 +81,5 @@ Optional:
 Import is supported using the following syntax:
 
 ```shell
-terraform import snowflake_database_old.example name
+terraform import snowflake_database_old.example 'database_name'
 ```

examples/resources/snowflake_database_old/import.sh

@@ -1 +1 @@
-terraform import snowflake_database_old.example name
+terraform import snowflake_database_old.example 'database_name'

pkg/acceptance/asserts.go

@@ -15,7 +15,7 @@ func IsGreaterOrEqualTo(greaterOrEqualValue int) resource.CheckResourceAttrWithF
 		}
 
 		if intValue < greaterOrEqualValue {
-			return fmt.Errorf("expected value %d greater or equal to %d", intValue, greaterOrEqualValue)
+			return fmt.Errorf("expected value %d to be greater or equal to %d", intValue, greaterOrEqualValue)
 		}
 
 		return nil

pkg/acceptance/asserts_test.go

@@ -17,7 +17,25 @@ func TestIsGreaterOrEqualTo(t *testing.T) {
 			Name:             "validation: smaller than expected",
 			GreaterOrEqualTo: 20,
 			Actual:           "10",
-			Error:            "expected value 10 greater or equal to 20",
+			Error:            "expected value 10 to be greater or equal to 20",
+		},
+		{
+			Name:             "validation: zero actual value",
+			GreaterOrEqualTo: 20,
+			Actual:           "0",
+			Error:            "expected value 0 to be greater or equal to 20",
+		},
+		{
+			Name:             "validation: zero greater value",
+			GreaterOrEqualTo: 0,
+			Actual:           "-10",
+			Error:            "expected value -10 to be greater or equal to 0",
+		},
+		{
+			Name:             "validation: negative value",
+			GreaterOrEqualTo: -20,
+			Actual:           "-30",
+			Error:            "expected value -30 to be greater or equal to -20",
 		},
 		{
 			Name:             "validation: not int value",
@@ -35,6 +53,11 @@ func TestIsGreaterOrEqualTo(t *testing.T) {
 			GreaterOrEqualTo: 20,
 			Actual:           "30",
 		},
+		{
+			Name:             "validation: greater value with expected negative value",
+			GreaterOrEqualTo: -20,
+			Actual:           "30",
+		},
 	}
 
 	for _, testCase := range testCases {

pkg/acceptance/check_destroy.go

@@ -70,6 +70,9 @@ var showByIdFunctions = map[resources.Resource]showByIdFunc{
 	resources.ApiIntegration: func(ctx context.Context, client *sdk.Client, id sdk.ObjectIdentifier) error {
 		return runShowById(ctx, id, client.ApiIntegrations.ShowByID)
 	},
+	resources.Database: func(ctx context.Context, client *sdk.Client, id sdk.ObjectIdentifier) error {
+		return runShowById(ctx, id, client.Databases.ShowByID)
+	},
 	resources.DatabaseOld: func(ctx context.Context, client *sdk.Client, id sdk.ObjectIdentifier) error {
 		return runShowById(ctx, id, client.Databases.ShowByID)
 	},
@@ -154,9 +157,6 @@ var showByIdFunctions = map[resources.Resource]showByIdFunc{
 	resources.Stage: func(ctx context.Context, client *sdk.Client, id sdk.ObjectIdentifier) error {
 		return runShowById(ctx, id, client.Stages.ShowByID)
 	},
-	resources.Database: func(ctx context.Context, client *sdk.Client, id sdk.ObjectIdentifier) error {
-		return runShowById(ctx, id, client.Databases.ShowByID)
-	},
 	resources.StorageIntegration: func(ctx context.Context, client *sdk.Client, id sdk.ObjectIdentifier) error {
 		return runShowById(ctx, id, client.StorageIntegrations.ShowByID)
 	},

pkg/acceptance/helpers/parameter_client.go

@@ -3,9 +3,10 @@ package helpers
 import (
 	"context"
 	"fmt"
-	"slices"
 	"testing"
 
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections"
+
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
 	"github.com/stretchr/testify/require"
 )
@@ -46,13 +47,6 @@ func (c *ParameterClient) ShowDatabaseParameters(t *testing.T, id sdk.AccountObj
 	return params
 }
 
-func (c *ParameterClient) GetAccountParameter(t *testing.T, parameter sdk.AccountParameter) *sdk.Parameter {
-	t.Helper()
-	param, err := c.client().ShowAccountParameter(context.Background(), parameter)
-	require.NoError(t, err)
-	return param
-}
-
 func (c *ParameterClient) UpdateAccountParameterTemporarily(t *testing.T, parameter sdk.AccountParameter, newValue string) func() {
 	t.Helper()
 	ctx := context.Background()
@@ -95,9 +89,7 @@ func (c *ParameterClient) UnsetAccountParameter(t *testing.T, parameter sdk.Acco
 
 func FindParameter(t *testing.T, parameters []*sdk.Parameter, parameter sdk.AccountParameter) *sdk.Parameter {
 	t.Helper()
-	idx := slices.IndexFunc(parameters, func(p *sdk.Parameter) bool {
-		return p.Key == string(parameter)
-	})
-	require.NotEqual(t, -1, idx, fmt.Sprintf("parameter %s not found", string(parameter)))
-	return parameters[idx]
+	param, err := collections.FindOne(parameters, func(p *sdk.Parameter) bool { return p.Key == string(parameter) })
+	require.NoError(t, err)
+	return *param
 }

pkg/acceptance/snowflakechecks/database.go

@@ -12,16 +12,16 @@ import (
 	"github.com/hashicorp/terraform-plugin-testing/terraform"
 )
 
-func CheckDatabaseDataRetentionTimeInDays(t *testing.T, databaseId sdk.AccountObjectIdentifier, level string, value string) resource.TestCheckFunc {
+func CheckDatabaseDataRetentionTimeInDays(t *testing.T, databaseId sdk.AccountObjectIdentifier, expectedLevel sdk.ParameterType, expectedValue string) resource.TestCheckFunc {
 	t.Helper()
 	return func(state *terraform.State) error {
 		param := helpers.FindParameter(t, acc.TestClient().Parameter.ShowDatabaseParameters(t, databaseId), sdk.AccountParameterDataRetentionTimeInDays)
 		var errs []error
-		if param.Level != sdk.ParameterType(level) {
-			errs = append(errs, fmt.Errorf("expected parameter level %s, got %s", sdk.ParameterType(level), param.Level))
+		if param.Level != expectedLevel {
+			errs = append(errs, fmt.Errorf("expected parameter level %s, got %s", expectedLevel, param.Level))
 		}
-		if param.Value != value {
-			errs = append(errs, fmt.Errorf("expected parameter value %s, got %s", sdk.ParameterType(level), param.Level))
+		if param.Value != expectedValue {
+			errs = append(errs, fmt.Errorf("expected parameter value %s, got %s", expectedLevel, param.Level))
 		}
 		return errors.Join(errs...)
 	}

pkg/datasources/databases_acceptance_test.go

@@ -93,7 +93,7 @@ func TestAcc_Databases_Complete(t *testing.T) {
 }
 
 func TestAcc_Databases_DifferentFiltering(t *testing.T) {
-	prefix := acc.TestClient().Ids.Alpha()
+	prefix := random.String()
 	idOne := acc.TestClient().Ids.RandomAccountObjectIdentifierWithPrefix(prefix)
 	idTwo := acc.TestClient().Ids.RandomAccountObjectIdentifierWithPrefix(prefix)
 	idThree := acc.TestClient().Ids.RandomAccountObjectIdentifier()

pkg/resources/custom_diffs.go

@@ -5,6 +5,8 @@ import (
 	"log"
 	"strconv"
 
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/internal/collections"
+
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
 
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
@@ -13,58 +15,43 @@ import (
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 )
 
-func AccountObjectStringValueComputedIf(key string, params []*sdk.Parameter, parameterLevel sdk.ParameterType, parameter sdk.AccountParameter) schema.CustomizeDiffFunc {
-	return ValueComputedIf(key, params, parameterLevel, parameter, func(value any) string { return value.(string) })
+func StringParameterValueComputedIf(key string, params []*sdk.Parameter, parameterLevel sdk.ParameterType, parameter sdk.AccountParameter) schema.CustomizeDiffFunc {
+	return ParameterValueComputedIf(key, params, parameterLevel, parameter, func(value any) string { return value.(string) })
 }
 
-func AccountObjectIntValueComputedIf(key string, params []*sdk.Parameter, parameterLevel sdk.ParameterType, parameter sdk.AccountParameter) schema.CustomizeDiffFunc {
-	return ValueComputedIf(key, params, parameterLevel, parameter, func(value any) string { return strconv.Itoa(value.(int)) })
+func IntParameterValueComputedIf(key string, params []*sdk.Parameter, parameterLevel sdk.ParameterType, parameter sdk.AccountParameter) schema.CustomizeDiffFunc {
+	return ParameterValueComputedIf(key, params, parameterLevel, parameter, func(value any) string { return strconv.Itoa(value.(int)) })
 }
 
-func AccountObjectBoolValueComputedIf(key string, params []*sdk.Parameter, parameterLevel sdk.ParameterType, parameter sdk.AccountParameter) schema.CustomizeDiffFunc {
-	return ValueComputedIf(key, params, parameterLevel, parameter, func(value any) string { return strconv.FormatBool(value.(bool)) })
+func BoolParameterValueComputedIf(key string, params []*sdk.Parameter, parameterLevel sdk.ParameterType, parameter sdk.AccountParameter) schema.CustomizeDiffFunc {
+	return ParameterValueComputedIf(key, params, parameterLevel, parameter, func(value any) string { return strconv.FormatBool(value.(bool)) })
 }
 
-func ValueComputedIf(key string, parameters []*sdk.Parameter, objectParameterLevel sdk.ParameterType, accountParameter sdk.AccountParameter, valueToString func(v any) string) schema.CustomizeDiffFunc {
-	var parameterValue *string
-	var parameterLevel *sdk.ParameterType
-
-	for _, parameter := range parameters {
-		if parameter.Key == string(accountParameter) {
-			parameterLevel = &parameter.Level
-			parameterValue = &parameter.Value
-			break
+func ParameterValueComputedIf(key string, parameters []*sdk.Parameter, objectParameterLevel sdk.ParameterType, accountParameter sdk.AccountParameter, valueToString func(v any) string) schema.CustomizeDiffFunc {
+	return func(ctx context.Context, d *schema.ResourceDiff, meta any) error {
+		foundParameter, err := collections.FindOne(parameters, func(parameter *sdk.Parameter) bool { return parameter.Key == string(accountParameter) })
+		if err != nil {
+			log.Printf("[WARN] failed to find account parameter: %s", accountParameter)
+			return nil
 		}
-	}
+		parameter := *foundParameter
 
-	condition := func(ctx context.Context, d *schema.ResourceDiff, meta any) bool {
 		configValue, ok := d.GetRawConfig().AsValueMap()[key]
 
-		if parameterLevel == nil || parameterValue == nil {
-			log.Printf("[ERROR] ValueComputedIf, parameter %s not found", accountParameter)
-			return false
-		}
-
 		// For cases where currently set value (in the config) is equal to the parameter, but not set on the right level.
 		// The parameter is set somewhere higher in the hierarchy, and we need to "forcefully" set the value to
 		// perform the actual set on Snowflake (and set the parameter on the correct level).
-		if *parameterLevel != objectParameterLevel && !configValue.IsNull() && *parameterValue == valueToString(d.Get(key)) {
-			return true
+		if ok && !configValue.IsNull() && parameter.Level != objectParameterLevel && parameter.Value == valueToString(d.Get(key)) {
+			return d.SetNewComputed(key)
 		}
 
 		// For all other cases, if a parameter is set in the configuration, we can ignore parts needed for Computed fields.
 		if ok && !configValue.IsNull() {
-			return false
+			return nil
 		}
 
-		// If the configuration is not set, perform SetNewComputed for cases like:
-		// 1. Check if the parameter value differs from the one saved in state (if they differ, we'll update the computed value).
-		// 2. Check if the parameter level is set on the same level for the given object (if they're the same, we'll trigger the unset as it has been updated in Snowflake).
-		return *parameterValue != valueToString(d.Get(key)) || *parameterLevel == objectParameterLevel
-	}
-
-	return func(ctx context.Context, d *schema.ResourceDiff, meta any) error {
-		if condition(ctx, d, meta) {
+		// Check if the parameter is set on the object level (if so, it means that it was set externally, and we have to unset it).
+		if parameter.Level == objectParameterLevel {
 			return d.SetNewComputed(key)
 		}
 

pkg/resources/custom_diffs_test.go

@@ -16,9 +16,9 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-func TestValueComputedIf(t *testing.T) {
+func TestParameterValueComputedIf(t *testing.T) {
 	createProviderConfig := func(parameterLevel sdk.ParameterType, parameterValue sdk.LogLevel) *schema.Provider {
-		customDiff := resources.ValueComputedIf(
+		customDiff := resources.ParameterValueComputedIf(
 			"value",
 			[]*sdk.Parameter{
 				{

pkg/resources/database.go

@@ -121,9 +121,13 @@ func CreateDatabase(ctx context.Context, d *schema.ResourceData, meta any) diag.
 		userTaskTimeoutMs,
 		userTaskMinimumTriggerIntervalInSeconds,
 		quotedIdentifiersIgnoreCase,
-		enableConsoleOutput := GetAllDatabaseParameters(d)
+		enableConsoleOutput,
+		err := GetAllDatabaseParameters(d)
+	if err != nil {
+		return diag.FromErr(err)
+	}
 
-	err := client.Databases.Create(ctx, id, &sdk.CreateDatabaseOptions{
+	err = client.Databases.Create(ctx, id, &sdk.CreateDatabaseOptions{
 		Transient:                               GetPropertyAsPointer[bool](d, "is_transient"),
 		DataRetentionTimeInDays:                 dataRetentionTimeInDays,
 		MaxDataExtensionTimeInDays:              maxDataExtensionTimeInDays,

pkg/resources/database_acceptance_test.go

@@ -525,7 +525,7 @@ func TestAcc_Database_HierarchicalValues(t *testing.T) {
 		Steps: []resource.TestStep{
 			{
 				PreConfig: func() {
-					*paramDefault = acc.TestClient().Parameter.GetAccountParameter(t, sdk.AccountParameterMaxDataExtensionTimeInDays).Default
+					*paramDefault = acc.TestClient().Parameter.ShowAccountParameter(t, sdk.AccountParameterMaxDataExtensionTimeInDays).Default
 				},
 				ConfigDirectory: acc.ConfigurationDirectory("TestAcc_Database/basic"),
 				ConfigVariables: configVariables(id, comment),
@@ -761,7 +761,7 @@ func TestAcc_Database_IntParameter(t *testing.T) {
 				},
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr("snowflake_database.test", "data_retention_time_in_days", "25"),
-					snowflakechecks.CheckDatabaseDataRetentionTimeInDays(t, id, "DATABASE", "25"),
+					snowflakechecks.CheckDatabaseDataRetentionTimeInDays(t, id, sdk.ParameterTypeDatabase, "25"),
 				),
 			},
 			// remove the param from config
@@ -806,7 +806,7 @@ func TestAcc_Database_IntParameter(t *testing.T) {
 				},
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr("snowflake_database.test", "data_retention_time_in_days", "1"),
-					snowflakechecks.CheckDatabaseDataRetentionTimeInDays(t, id, "DATABASE", "1"),
+					snowflakechecks.CheckDatabaseDataRetentionTimeInDays(t, id, sdk.ParameterTypeDatabase, "1"),
 				),
 			},
 			// remove the param from config
@@ -849,7 +849,7 @@ func TestAcc_Database_IntParameter(t *testing.T) {
 				},
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr("snowflake_database.test", "data_retention_time_in_days", "50"),
-					snowflakechecks.CheckDatabaseDataRetentionTimeInDays(t, id, "ACCOUNT", "50"),
+					snowflakechecks.CheckDatabaseDataRetentionTimeInDays(t, id, sdk.ParameterTypeAccount, "50"),
 				),
 			},
 			// import when param not in config (set on account)
@@ -861,7 +861,7 @@ func TestAcc_Database_IntParameter(t *testing.T) {
 					importchecks.TestCheckResourceAttrInstanceState(id.Name(), "data_retention_time_in_days", "50"),
 				),
 				Check: resource.ComposeTestCheckFunc(
-					snowflakechecks.CheckDatabaseDataRetentionTimeInDays(t, id, "ACCOUNT", "50"),
+					snowflakechecks.CheckDatabaseDataRetentionTimeInDays(t, id, sdk.ParameterTypeAccount, "50"),
 				),
 			},
 			// change param value on database
@@ -880,7 +880,7 @@ func TestAcc_Database_IntParameter(t *testing.T) {
 				},
 				Check: resource.ComposeTestCheckFunc(
 					resource.TestCheckResourceAttr("snowflake_database.test", "data_retention_time_in_days", "50"),
-					snowflakechecks.CheckDatabaseDataRetentionTimeInDays(t, id, "ACCOUNT", "50"),
+					snowflakechecks.CheckDatabaseDataRetentionTimeInDays(t, id, sdk.ParameterTypeAccount, "50"),
 				),
 			},
 			// unset param on account
@@ -907,7 +907,7 @@ func TestAcc_Database_IntParameter(t *testing.T) {
 	})
 }
 
-func TestAcc_Database_BasicUpgrade(t *testing.T) {
+func TestAcc_Database_UpgradeWithTheSameFieldsAsInTheOldOne(t *testing.T) {
 	id := acc.TestClient().Ids.RandomAccountObjectIdentifier()
 	comment := random.Comment()
 	dataRetentionTimeInDays := new(string)