fix: Fix functions and small other fixes (#2503)

- change parallel test in task
- run pre-push (because main was not pre-pushed: docs and reformat)
- fix materialized view datasource test
- add state upgrader to function resource's id
- add test utilizing older provider version

References: #2490

docs/resources/dynamic_table.md

@@ -3,7 +3,7 @@
 page_title: "snowflake_dynamic_table Resource - terraform-provider-snowflake"
 subcategory: ""
 description: |-
-
+  
 ---
 
 # snowflake_dynamic_table (Resource)
@@ -42,15 +42,16 @@ resource "snowflake_dynamic_table" "dt" {
 ### Optional
 
 - `comment` (String) Specifies a comment for the dynamic table.
+- `initialize` (String) Initialize trigger for the dynamic table. Can only be set on creation. Available options are ON_CREATE and ON_SCHEDULE.
 - `or_replace` (Boolean) Specifies whether to replace the dynamic table if it already exists.
-- `refresh_mode` (String) INCREMENTAL if the dynamic table will use incremental refreshes, or FULL if it will recompute the whole table on every refresh. Specify AUTO to let Snowflake decide. The default is AUTO.
-- `initialize` (String) Specifies the behavior of the initial refresh of the dynamic table. This property cannot be altered after you create the dynamic table. Specify ON_CREATE to initialize the dynamic table immeidately, or ON_SCHEDULE to have it initialize at the next tick after creation. The default os ON_CREATE.
+- `refresh_mode` (String) INCREMENTAL to use incremental refreshes, FULL to recompute the whole table on every refresh, or AUTO to let Snowflake decide.
 
 ### Read-Only
 
 - `automatic_clustering` (Boolean) Whether auto-clustering is enabled on the dynamic table. Not currently supported for dynamic tables.
 - `bytes` (Number) Number of bytes that will be scanned if the entire dynamic table is scanned in a query.
 - `cluster_by` (String) The clustering key for the dynamic table.
+- `created_on` (String) Time when this dynamic table was created.
 - `data_timestamp` (String) Timestamp of the data in the base object(s) that is included in the dynamic table.
 - `id` (String) The ID of this resource.
 - `is_clone` (Boolean) TRUE if the dynamic table has been cloned, else FALSE.

pkg/datasources/materialized_views_acceptance_test.go

@@ -13,11 +13,9 @@ import (
 )
 
 func TestAcc_MaterializedViews(t *testing.T) {
-	warehouseName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
-	databaseName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
-	schemaName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
 	tableName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
 	viewName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+
 	resource.Test(t, resource.TestCase{
 		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
 		PreCheck:                 func() { acc.TestAccPreCheck(t) },
@@ -27,10 +25,10 @@ func TestAcc_MaterializedViews(t *testing.T) {
 		CheckDestroy: nil,
 		Steps: []resource.TestStep{
 			{
-				Config: materializedViews(warehouseName, databaseName, schemaName, tableName, viewName),
+				Config: materializedViews(acc.TestWarehouseName, acc.TestDatabaseName, acc.TestSchemaName, tableName, viewName),
 				Check: resource.ComposeTestCheckFunc(
-					resource.TestCheckResourceAttr("data.snowflake_materialized_views.v", "database", databaseName),
-					resource.TestCheckResourceAttr("data.snowflake_materialized_views.v", "schema", schemaName),
+					resource.TestCheckResourceAttr("data.snowflake_materialized_views.v", "database", acc.TestDatabaseName),
+					resource.TestCheckResourceAttr("data.snowflake_materialized_views.v", "schema", acc.TestSchemaName),
 					resource.TestCheckResourceAttrSet("data.snowflake_materialized_views.v", "materialized_views.#"),
 					resource.TestCheckResourceAttr("data.snowflake_materialized_views.v", "materialized_views.#", "1"),
 					resource.TestCheckResourceAttr("data.snowflake_materialized_views.v", "materialized_views.0.name", viewName),
@@ -42,44 +40,30 @@ func TestAcc_MaterializedViews(t *testing.T) {
 
 func materializedViews(warehouseName string, databaseName string, schemaName string, tableName string, viewName string) string {
 	return fmt.Sprintf(`
-	resource "snowflake_warehouse" "w" {
-		name                = "%v"
-		initially_suspended = false
-	}
-
-	resource snowflake_database "d" {
-		name = "%v"
-	}
-
-	resource snowflake_schema "s"{
-		name 	 = "%v"
-		database = snowflake_database.d.name
-	}
-
 	resource snowflake_table "t"{
-		name 	 = "%v"
-		database = snowflake_schema.s.database
-		schema 	 = snowflake_schema.s.name
+		name 	 = "%[4]v"
+		database = "%[2]s"
+		schema 	 = "%[3]s"
 		column {
 			name = "column2"
 			type = "VARCHAR(16)"
 		}
 	}
 
 	resource snowflake_materialized_view "v"{
-		name 	   = "%v"
+		name 	   = "%[5]v"
 		comment    = "Terraform test resource"
-		database   = snowflake_schema.s.database
-		schema 	   = snowflake_schema.s.name
+		database   = "%[2]s"
+		schema 	   = "%[3]s"
 		is_secure  = true
 		or_replace = false
 		statement  = "SELECT * FROM ${snowflake_table.t.name}"
-		warehouse  = snowflake_warehouse.w.name
+		warehouse  = "%[1]s"
 	}
 
 	data snowflake_materialized_views "v" {
-		database = snowflake_materialized_view.v.database
-		schema = snowflake_materialized_view.v.schema
+		database = "%[2]s"
+		schema = "%[3]s"
 		depends_on = [snowflake_materialized_view.v]
 	}
 	`, warehouseName, databaseName, schemaName, tableName, viewName)

pkg/resources/dynamic_table.go

@@ -91,7 +91,7 @@ var dynamicTableSchema = map[string]*schema.Schema{
 		Type:         schema.TypeString,
 		Optional:     true,
 		Default:      sdk.DynamicTableInitializeOnCreate,
-		Description:  "Initialize trigger for the dynamic table. Can only be set on creation.",
+		Description:  "Initialize trigger for the dynamic table. Can only be set on creation. Available options are ON_CREATE and ON_SCHEDULE.",
 		ValidateFunc: validation.StringInSlice(sdk.AsStringList(sdk.AllDynamicTableInitializes), true),
 		ForceNew:     true,
 	},

pkg/resources/function.go

@@ -9,6 +9,7 @@ import (
 
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/snowflake"
+	"github.com/hashicorp/go-cty/cty"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
@@ -159,6 +160,8 @@ var functionSchema = map[string]*schema.Schema{
 // Function returns a pointer to the resource representing a stored function.
 func Function() *schema.Resource {
 	return &schema.Resource{
+		SchemaVersion: 1,
+
 		CreateContext: CreateContextFunction,
 		ReadContext:   ReadContextFunction,
 		UpdateContext: UpdateContextFunction,
@@ -168,6 +171,15 @@ func Function() *schema.Resource {
 		Importer: &schema.ResourceImporter{
 			StateContext: schema.ImportStatePassthroughContext,
 		},
+
+		StateUpgraders: []schema.StateUpgrader{
+			{
+				Version: 0,
+				// setting type to cty.EmptyObject is a bit hacky here but following https://developer.hashicorp.com/terraform/plugin/framework/migrating/resources/state-upgrade#sdkv2-1 would require lots of repetitive code; this should work with cty.EmptyObject
+				Type:    cty.EmptyObject,
+				Upgrade: v085FunctionIdStateUpgrader,
+			},
+		},
 	}
 }
 

pkg/resources/function_acceptance_test.go

@@ -1,13 +1,19 @@
 package resources_test
 
 import (
+	"context"
+	"errors"
+	"fmt"
 	"strings"
 	"testing"
 
 	acc "github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/acceptance"
+
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
 	"github.com/hashicorp/terraform-plugin-testing/config"
 	"github.com/hashicorp/terraform-plugin-testing/helper/acctest"
 	"github.com/hashicorp/terraform-plugin-testing/helper/resource"
+	"github.com/hashicorp/terraform-plugin-testing/terraform"
 	"github.com/hashicorp/terraform-plugin-testing/tfversion"
 )
 
@@ -33,7 +39,7 @@ func testAccFunction(t *testing.T, configDirectory string) {
 		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
 			tfversion.RequireAbove(tfversion.Version1_5_0),
 		},
-		CheckDestroy: testAccCheckDynamicTableDestroy,
+		CheckDestroy: testAccCheckFunctionDestroy,
 		Steps: []resource.TestStep{
 			{
 				ConfigDirectory: acc.ConfigurationDirectory(configDirectory),
@@ -124,7 +130,7 @@ func TestAcc_Function_complex(t *testing.T) {
 		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
 			tfversion.RequireAbove(tfversion.Version1_5_0),
 		},
-		CheckDestroy: testAccCheckDynamicTableDestroy,
+		CheckDestroy: testAccCheckFunctionDestroy,
 		Steps: []resource.TestStep{
 			{
 				ConfigDirectory: acc.ConfigurationDirectory("TestAcc_Function/complex"),
@@ -176,3 +182,85 @@ func TestAcc_Function_complex(t *testing.T) {
 		},
 	})
 }
+
+// proves issue https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2490
+func TestAcc_Function_migrateFromVersion085(t *testing.T) {
+	name := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+	resourceName := "snowflake_function.f"
+
+	resource.Test(t, resource.TestCase{
+		PreCheck: func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: testAccCheckFunctionDestroy,
+
+		// Using the string config because of the validation in teststep_validate.go:
+		// teststep.Config.HasConfigurationFiles() returns true both for ConfigFile and ConfigDirectory.
+		// It returns false for Config. I don't understand why they have such a validation, but we will work around it later.
+		// Added as subtask SNOW-1057066 to SNOW-926148.
+		Steps: []resource.TestStep{
+			{
+				ExternalProviders: map[string]resource.ExternalProvider{
+					"snowflake": {
+						VersionConstraint: "=0.85.0",
+						Source:            "Snowflake-Labs/snowflake",
+					},
+				},
+				Config: functionConfig(acc.TestDatabaseName, acc.TestSchemaName, name),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "name", name),
+					resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName),
+					resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName),
+				),
+			},
+			{
+				ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+				Config:                   functionConfig(acc.TestDatabaseName, acc.TestSchemaName, name),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "name", name),
+					resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName),
+					resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName),
+				),
+			},
+		},
+	})
+}
+
+func functionConfig(database string, schema string, name string) string {
+	return fmt.Sprintf(`
+resource "snowflake_function" "f" {
+  database        = "%[1]s"
+  schema          = "%[2]s"
+  name            = "%[3]s"
+  return_type     = "VARCHAR"
+  return_behavior = "IMMUTABLE"
+  statement       = "SELECT PARAM"
+
+  arguments {
+    name = "PARAM"
+    type = "VARCHAR"
+  }
+}
+`, database, schema, name)
+}
+
+func testAccCheckFunctionDestroy(s *terraform.State) error {
+	client, err := sdk.NewDefaultClient()
+	if err != nil {
+		return errors.New("client could not be instantiated")
+	}
+
+	for _, rs := range s.RootModule().Resources {
+		if rs.Type != "snowflake_function" {
+			continue
+		}
+		ctx := context.Background()
+		id := sdk.NewSchemaObjectIdentifier(rs.Primary.Attributes["database"], rs.Primary.Attributes["schema"], rs.Primary.Attributes["name"])
+		function, err := client.Functions.ShowByID(ctx, id)
+		if err == nil {
+			return fmt.Errorf("function %v still exists", function.Name)
+		}
+	}
+	return nil
+}

pkg/resources/function_state_upgraders.go

@@ -0,0 +1,56 @@
+package resources
+
+import (
+	"context"
+	"fmt"
+	"strings"
+
+	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
+)
+
+type v085FunctionId struct {
+	DatabaseName string
+	SchemaName   string
+	FunctionName string
+	ArgTypes     []string
+}
+
+func parseV085FunctionId(v string) (*v085FunctionId, error) {
+	arr := strings.Split(v, "|")
+	if len(arr) != 4 {
+		return nil, fmt.Errorf("ID %v is invalid", v)
+	}
+
+	return &v085FunctionId{
+		DatabaseName: arr[0],
+		SchemaName:   arr[1],
+		FunctionName: arr[2],
+		ArgTypes:     strings.Split(arr[3], "-"),
+	}, nil
+}
+
+func v085FunctionIdStateUpgrader(ctx context.Context, rawState map[string]interface{}, meta interface{}) (map[string]interface{}, error) {
+	if rawState == nil {
+		return rawState, nil
+	}
+
+	oldId := rawState["id"].(string)
+	parsedV085FunctionId, err := parseV085FunctionId(oldId)
+	if err != nil {
+		return nil, err
+	}
+
+	argDataTypes := make([]sdk.DataType, len(parsedV085FunctionId.ArgTypes))
+	for i, argType := range parsedV085FunctionId.ArgTypes {
+		argDataType, err := sdk.ToDataType(argType)
+		if err != nil {
+			return nil, err
+		}
+		argDataTypes[i] = argDataType
+	}
+
+	schemaObjectIdentifierWithArguments := sdk.NewSchemaObjectIdentifierWithArguments(parsedV085FunctionId.DatabaseName, parsedV085FunctionId.SchemaName, parsedV085FunctionId.FunctionName, argDataTypes)
+	rawState["id"] = schemaObjectIdentifierWithArguments.FullyQualifiedName()
+
+	return rawState, nil
+}

pkg/resources/task_acceptance_test.go

@@ -543,9 +543,12 @@ func TestAcc_Task_SwitchScheduled(t *testing.T) {
 	accName := "tst-terraform-" + strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
 	taskRootName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
 
-	resource.ParallelTest(t, resource.TestCase{
-		Providers:    acc.TestAccProviders(),
-		PreCheck:     func() { acc.TestAccPreCheck(t) },
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
 		CheckDestroy: nil,
 		Steps: []resource.TestStep{
 			{

pkg/resources/testdata/TestAcc_DynamicTable_basic/3/test.tf

@@ -19,8 +19,8 @@ resource "snowflake_dynamic_table" "dt" {
   target_lag {
     downstream = true
   }
-  warehouse    = var.warehouse
-  query        = var.query
-  comment      = var.comment
-  initialize   = var.initialize
+  warehouse  = var.warehouse
+  query      = var.query
+  comment    = var.comment
+  initialize = var.initialize
 }

pkg/resources/testdata/TestAcc_GrantPrivilegesToShare/OnAllTablesInSchema/test.tf

@@ -1,10 +1,10 @@
 resource "snowflake_share" "test" {
-  name = var.to_share
+  name       = var.to_share
   depends_on = [snowflake_database.test]
 }
 
 resource "snowflake_database" "test" {
-  name       = var.database
+  name = var.database
 }
 
 resource "snowflake_schema" "test" {

pkg/resources/testdata/TestAcc_GrantPrivilegesToShare/OnAllTablesInSchema_NoGrant/test.tf

@@ -1,10 +1,10 @@
 resource "snowflake_share" "test" {
   depends_on = [snowflake_database.test]
-  name = var.to_share
+  name       = var.to_share
 }
 
 resource "snowflake_database" "test" {
-  name       = var.database
+  name = var.database
 }
 
 resource "snowflake_schema" "test" {

pkg/resources/testdata/TestAcc_GrantPrivilegesToShare/OnDatabase/test.tf

@@ -1,10 +1,10 @@
 resource "snowflake_share" "test" {
   depends_on = [snowflake_database.test]
-  name = var.to_share
+  name       = var.to_share
 }
 
 resource "snowflake_database" "test" {
-  name       = var.database
+  name = var.database
 }
 
 resource "snowflake_grant_privileges_to_share" "test" {