Apply PR feedback on initialze/refresh_mode

* Add type and validations for DynamicTableInitialize options * Update type and add validations for DynamicTableRefreshMode options * Set default values for both initialize and refresh_mode * No longer use DiffSuppressFunc since the default values should match when not specified * Add separate test for initialize and refresh_mode in dynamic table integration test * Add additional steps in dynamic table acceptance test to show what happends when initialize and/or refresh_mode is changed after the dynamic table is already created. * To support the above, read the created_on date from existing dynamic tables as a way to test whether a step has replaced the dynamic table
Snowflake-Labs · Feb 12, 2024 · 9481bff · 9481bff
1 parent e5be319
commit 9481bff
Show file tree

Hide file tree

Showing 12 changed files with 275 additions and 54 deletions.
diff --git a/pkg/resources/dynamic_table.go b/pkg/resources/dynamic_table.go
@@ -5,13 +5,18 @@ import (
 	"database/sql"
 	"errors"
 	"log"
+	"regexp"
 	"strings"
+	"time"
 
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/helpers"
 	"github.com/Snowflake-Labs/terraform-provider-snowflake/pkg/sdk"
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
+	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/validation"
 )
 
+var refreshModePattern = regexp.MustCompile(`refresh_mode = '(\w+)'`)
+
 var dynamicTableSchema = map[string]*schema.Schema{
 	"or_replace": {
 		Type:        schema.TypeBool,
@@ -76,32 +81,25 @@ var dynamicTableSchema = map[string]*schema.Schema{
 		Description: "Specifies a comment for the dynamic table.",
 	},
 	"refresh_mode": {
-		Type:        schema.TypeString,
-		Optional:    true,
-		Description: "INCREMENTAL if the dynamic table will use incremental refreshes, or FULL if it will recompute the whole table on every refresh.",
-		ForceNew:    true,
-		DiffSuppressFunc: func(_, oldValue, newValue string, d *schema.ResourceData) bool {
-			switch {
-			case oldValue == "INCREMENTAL" && newValue == "FULL":
-				return false
-			case oldValue == "FULL" && newValue == "INCREMENTAL":
-				return false
-			case oldValue == "" && newValue != "":
-				return false
-			default:
-				return true
-			}
-		},
-		DiffSuppressOnRefresh: true,
+		Type:         schema.TypeString,
+		Optional:     true,
+		Default:      sdk.DynamicTableRefreshModeAuto,
+		Description:  "INCREMENTAL to use incremental refreshes, FULL to recompute the whole table on every refresh, or AUTO to let Snowflake decide.",
+		ValidateFunc: validation.StringInSlice(sdk.AsStringList(sdk.AllDynamicRefreshModes), true),
+		ForceNew:     true,
 	},
 	"initialize": {
+		Type:         schema.TypeString,
+		Optional:     true,
+		Default:      sdk.DynamicTableInitializeOnCreate,
+		Description:  "Initialize trigger for the dynamic table. Can only be set on creation.",
+		ValidateFunc: validation.StringInSlice(sdk.AsStringList(sdk.AllDynamicTableInitializes), true),
+		ForceNew:     true,
+	},
+	"created_on": {
 		Type:        schema.TypeString,
-		Optional:    true,
-		Description: "Initialize trigger for the dynamic table. Can only be set on creation.",
-		ForceNew:    true,
-		DiffSuppressFunc: func(k, oldValue, newValue string, d *schema.ResourceData) bool {
-			return oldValue == "ON_CREATE" && newValue == ""
-		},
+		Description: "Time when this dynamic table was created.",
+		Computed:    true,
 	},
 	"cluster_by": {
 		Type:        schema.TypeString,
@@ -232,6 +230,15 @@ func ReadDynamicTable(d *schema.ResourceData, meta interface{}) error {
 			return err
 		}
 	}
+	m := refreshModePattern.FindStringSubmatch(dynamicTable.Text)
+	if len(m) > 1 {
+		if err := d.Set("refresh_mode", m[1]); err != nil {
+			return err
+		}
+	}
+	if err := d.Set("created_on", dynamicTable.CreatedOn.Format(time.RFC3339)); err != nil {
+		return err
+	}
 	if err := d.Set("cluster_by", dynamicTable.ClusterBy); err != nil {
 		return err
 	}
@@ -244,9 +251,6 @@ func ReadDynamicTable(d *schema.ResourceData, meta interface{}) error {
 	if err := d.Set("owner", dynamicTable.Owner); err != nil {
 		return err
 	}
-	if err := d.Set("refresh_mode", string(dynamicTable.RefreshMode)); err != nil {
-		return err
-	}
 	if err := d.Set("refresh_mode_reason", dynamicTable.RefreshModeReason); err != nil {
 		return err
 	}

diff --git a/pkg/resources/dynamic_table_acceptance_test.go b/pkg/resources/dynamic_table_acceptance_test.go
@@ -25,20 +25,28 @@ func TestAcc_DynamicTable_basic(t *testing.T) {
 	tableName := name + "_table"
 	m := func() map[string]config.Variable {
 		return map[string]config.Variable{
-			"name":         config.StringVariable(name),
-			"database":     config.StringVariable(acc.TestDatabaseName),
-			"schema":       config.StringVariable(acc.TestSchemaName),
-			"warehouse":    config.StringVariable(acc.TestWarehouseName),
-			"initialize":   config.StringVariable("ON_SCHEDULE"),
-			"refresh_mode": config.StringVariable("FULL"),
-			"query":        config.StringVariable(fmt.Sprintf(`select "id" from "%v"."%v"."%v"`, acc.TestDatabaseName, acc.TestSchemaName, tableName)),
-			"comment":      config.StringVariable("Terraform acceptance test"),
-			"table_name":   config.StringVariable(tableName),
+			"name":       config.StringVariable(name),
+			"database":   config.StringVariable(acc.TestDatabaseName),
+			"schema":     config.StringVariable(acc.TestSchemaName),
+			"warehouse":  config.StringVariable(acc.TestWarehouseName),
+			"query":      config.StringVariable(fmt.Sprintf(`select "id" from "%v"."%v"."%v"`, acc.TestDatabaseName, acc.TestSchemaName, tableName)),
+			"comment":    config.StringVariable("Terraform acceptance test"),
+			"table_name": config.StringVariable(tableName),
 		}
 	}
 	variableSet2 := m()
 	variableSet2["comment"] = config.StringVariable("Terraform acceptance test - updated")
 
+	variableSet3 := m()
+	variableSet3["initialize"] = config.StringVariable("ON_SCHEDULE")
+
+	variableSet4 := m()
+	variableSet4["initialize"] = config.StringVariable("ON_SCHEDULE") // keep the same setting from set 3
+	variableSet4["refresh_mode"] = config.StringVariable("FULL")
+
+	// used to check whether a dynamic table was replaced
+	var createdOn string
+
 	resource.Test(t, resource.TestCase{
 		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
 		PreCheck:                 func() { acc.TestAccPreCheck(t) },
@@ -55,8 +63,8 @@ func TestAcc_DynamicTable_basic(t *testing.T) {
 					resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName),
 					resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName),
 					resource.TestCheckResourceAttr(resourceName, "warehouse", acc.TestWarehouseName),
-					resource.TestCheckResourceAttr(resourceName, "initialize", "ON_SCHEDULE"),
-					resource.TestCheckResourceAttr(resourceName, "refresh_mode", "FULL"),
+					resource.TestCheckResourceAttr(resourceName, "initialize", "ON_CREATE"),
+					resource.TestCheckResourceAttr(resourceName, "refresh_mode", "AUTO"),
 					resource.TestCheckResourceAttr(resourceName, "target_lag.#", "1"),
 					resource.TestCheckResourceAttr(resourceName, "target_lag.0.maximum_duration", "2 minutes"),
 					resource.TestCheckResourceAttr(resourceName, "query", fmt.Sprintf("select \"id\" from \"%v\".\"%v\".\"%v\"", acc.TestDatabaseName, acc.TestSchemaName, tableName)),
@@ -76,6 +84,11 @@ func TestAcc_DynamicTable_basic(t *testing.T) {
 					resource.TestCheckResourceAttrSet(resourceName, "is_clone"),
 					resource.TestCheckResourceAttrSet(resourceName, "is_replica"),
 					resource.TestCheckResourceAttrSet(resourceName, "data_timestamp"),
+
+					resource.TestCheckResourceAttrWith(resourceName, "created_on", func(value string) error {
+						createdOn = value
+						return nil
+					}),
 				),
 			},
 			// test target lag to downstream and change comment
@@ -90,6 +103,58 @@ func TestAcc_DynamicTable_basic(t *testing.T) {
 					resource.TestCheckResourceAttr(resourceName, "target_lag.#", "1"),
 					resource.TestCheckResourceAttr(resourceName, "target_lag.0.downstream", "true"),
 					resource.TestCheckResourceAttr(resourceName, "comment", "Terraform acceptance test - updated"),
+
+					resource.TestCheckResourceAttrWith(resourceName, "created_on", func(value string) error {
+						if value != createdOn {
+							return fmt.Errorf("created_on changed from %v to %v", createdOn, value)
+						}
+						return nil
+					}),
+				),
+			},
+			// test changing initialize setting
+			{
+				ConfigDirectory: config.TestStepDirectory(),
+				ConfigVariables: variableSet3,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "name", name),
+					resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName),
+					resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName),
+					resource.TestCheckResourceAttr(resourceName, "initialize", "ON_SCHEDULE"),
+					resource.TestCheckResourceAttr(resourceName, "refresh_mode", "AUTO"),
+					resource.TestCheckResourceAttr(resourceName, "target_lag.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "target_lag.0.downstream", "true"),
+					resource.TestCheckResourceAttr(resourceName, "comment", "Terraform acceptance test"),
+
+					resource.TestCheckResourceAttrWith(resourceName, "created_on", func(value string) error {
+						if value == createdOn {
+							return fmt.Errorf("expected created_on to change but was not changed")
+						}
+						createdOn = value
+						return nil
+					}),
+				),
+			},
+			// test changing refresh_mode setting
+			{
+				ConfigDirectory: config.TestStepDirectory(),
+				ConfigVariables: variableSet4,
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr(resourceName, "name", name),
+					resource.TestCheckResourceAttr(resourceName, "database", acc.TestDatabaseName),
+					resource.TestCheckResourceAttr(resourceName, "schema", acc.TestSchemaName),
+					resource.TestCheckResourceAttr(resourceName, "initialize", "ON_SCHEDULE"),
+					resource.TestCheckResourceAttr(resourceName, "refresh_mode", "FULL"),
+					resource.TestCheckResourceAttr(resourceName, "target_lag.#", "1"),
+					resource.TestCheckResourceAttr(resourceName, "target_lag.0.downstream", "true"),
+					resource.TestCheckResourceAttr(resourceName, "comment", "Terraform acceptance test"),
+
+					resource.TestCheckResourceAttrWith(resourceName, "created_on", func(value string) error {
+						if value == createdOn {
+							return fmt.Errorf("expected created_on to change but was not changed")
+						}
+						return nil
+					}),
 				),
 			},
 			// test import

diff --git a/pkg/resources/testdata/TestAcc_DynamicTable_basic/1/test.tf b/pkg/resources/testdata/TestAcc_DynamicTable_basic/1/test.tf
@@ -19,9 +19,7 @@ resource "snowflake_dynamic_table" "dt" {
   target_lag {
     maximum_duration = "2 minutes"
   }
-  warehouse    = var.warehouse
-  query        = var.query
-  comment      = var.comment
-  refresh_mode = var.refresh_mode
-  initialize   = var.initialize
+  warehouse = var.warehouse
+  query     = var.query
+  comment   = var.comment
 }
diff --git a/pkg/resources/testdata/TestAcc_DynamicTable_basic/1/variables.tf b/pkg/resources/testdata/TestAcc_DynamicTable_basic/1/variables.tf
@@ -24,14 +24,6 @@ variable "comment" {
   type = string
 }
 
-variable "refresh_mode" {
-  type = string
-}
-
-variable "initialize" {
-  type = string
-}
-
 variable "table_name" {
   type = string
 }
diff --git a/pkg/resources/testdata/TestAcc_DynamicTable_basic/3/test.tf b/pkg/resources/testdata/TestAcc_DynamicTable_basic/3/test.tf
@@ -19,7 +19,8 @@ resource "snowflake_dynamic_table" "dt" {
   target_lag {
     downstream = true
   }
-  warehouse = var.warehouse
-  query     = var.query
-  comment   = var.comment
+  warehouse    = var.warehouse
+  query        = var.query
+  comment      = var.comment
+  initialize   = var.initialize
 }
diff --git a/pkg/resources/testdata/TestAcc_DynamicTable_basic/3/variables.tf b/pkg/resources/testdata/TestAcc_DynamicTable_basic/3/variables.tf
@@ -24,6 +24,10 @@ variable "comment" {
   type = string
 }
 
+variable "initialize" {
+  type = string
+}
+
 variable "table_name" {
   type = string
 }
diff --git a/pkg/resources/testdata/TestAcc_DynamicTable_basic/4/test.tf b/pkg/resources/testdata/TestAcc_DynamicTable_basic/4/test.tf
@@ -0,0 +1,27 @@
+
+
+resource "snowflake_table" "t" {
+  database        = var.database
+  schema          = var.schema
+  name            = var.table_name
+  change_tracking = true
+  column {
+    name = "id"
+    type = "NUMBER(38,0)"
+  }
+}
+
+resource "snowflake_dynamic_table" "dt" {
+  depends_on = [snowflake_table.t]
+  name       = var.name
+  database   = var.database
+  schema     = var.schema
+  target_lag {
+    downstream = true
+  }
+  warehouse    = var.warehouse
+  query        = var.query
+  comment      = var.comment
+  refresh_mode = var.refresh_mode
+  initialize   = var.initialize
+}
diff --git a/pkg/resources/testdata/TestAcc_DynamicTable_basic/4/variables.tf b/pkg/resources/testdata/TestAcc_DynamicTable_basic/4/variables.tf
@@ -0,0 +1,37 @@
+
+
+variable "name" {
+  type = string
+}
+
+variable "database" {
+  type = string
+}
+
+variable "schema" {
+  type = string
+}
+
+variable "warehouse" {
+  type = string
+}
+
+variable "query" {
+  type = string
+}
+
+variable "comment" {
+  type = string
+}
+
+variable "refresh_mode" {
+  type = string
+}
+
+variable "initialize" {
+  type = string
+}
+
+variable "table_name" {
+  type = string
+}
diff --git a/pkg/resources/testdata/TestAcc_DynamicTable_basic/5/test.tf b/pkg/resources/testdata/TestAcc_DynamicTable_basic/5/test.tf
@@ -0,0 +1,25 @@
+
+
+resource "snowflake_table" "t" {
+  database        = var.database
+  schema          = var.schema
+  name            = var.table_name
+  change_tracking = true
+  column {
+    name = "id"
+    type = "NUMBER(38,0)"
+  }
+}
+
+resource "snowflake_dynamic_table" "dt" {
+  depends_on = [snowflake_table.t]
+  name       = var.name
+  database   = var.database
+  schema     = var.schema
+  target_lag {
+    downstream = true
+  }
+  warehouse = var.warehouse
+  query     = var.query
+  comment   = var.comment
+}
diff --git a/pkg/resources/testdata/TestAcc_DynamicTable_basic/5/variables.tf b/pkg/resources/testdata/TestAcc_DynamicTable_basic/5/variables.tf
@@ -0,0 +1,29 @@
+
+
+variable "name" {
+  type = string
+}
+
+variable "database" {
+  type = string
+}
+
+variable "schema" {
+  type = string
+}
+
+variable "warehouse" {
+  type = string
+}
+
+variable "query" {
+  type = string
+}
+
+variable "comment" {
+  type = string
+}
+
+variable "table_name" {
+  type = string
+}