fix: Fix various small problems (#2552)

- Fixed clustering for table_resource (clustering with nested functions
are parsed correctly now)
- Adjusted view resource (`copy_grants` require `or_replace`) and
corrected the documentation

References: #2110 #2495 #2519

docs/resources/view.md

@@ -41,7 +41,7 @@ SQL
 ### Optional
 
 - `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.
+- `copy_grants` (Boolean) Retains the access permissions from the original view when a new view is created using the OR REPLACE clause. OR REPLACE must be set when COPY GRANTS is set.
 - `is_secure` (Boolean) Specifies that the view is secure.
 - `or_replace` (Boolean) Overwrites the View if it exists.
 - `tag` (Block List, Deprecated) Definitions of a tag to associate with the resource. (see [below for nested schema](#nestedblock--tag))

pkg/resources/table_acceptance_test.go

@@ -1537,3 +1537,50 @@ func testAccCheckTableDestroy(s *terraform.State) error {
 	}
 	return nil
 }
+
+// proves issues https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2110 and https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2495
+func TestAcc_Table_ClusterBy(t *testing.T) {
+	accName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: testAccCheckTableDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: tableConfigWithComplexClusterBy(accName, acc.TestDatabaseName, acc.TestSchemaName),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("snowflake_table.test_table", "name", accName),
+					resource.TestCheckResourceAttr("snowflake_table.test_table", "database", acc.TestDatabaseName),
+					resource.TestCheckResourceAttr("snowflake_table.test_table", "schema", acc.TestSchemaName),
+					resource.TestCheckResourceAttr("snowflake_table.test_table", "cluster_by.#", "2"),
+					resource.TestCheckResourceAttr("snowflake_table.test_table", "cluster_by.0", "date_trunc('month', LAST_LOAD_TIME)"),
+					resource.TestCheckResourceAttr("snowflake_table.test_table", "cluster_by.1", "COL1"),
+				),
+			},
+		},
+	})
+}
+
+func tableConfigWithComplexClusterBy(name string, databaseName string, schemaName string) string {
+	return fmt.Sprintf(`
+resource "snowflake_table" "test_table" {
+	name                = "%[1]s"
+	database            = "%[2]s"
+	schema              = "%[3]s"
+	cluster_by = ["date_trunc('month', LAST_LOAD_TIME)", "COL1"]
+	column {
+		name = "COL1"
+		type = "VARCHAR(16777216)"
+	}
+    column {
+        name     = "LAST_LOAD_TIME"
+        type     = "TIMESTAMP_LTZ(6)"
+        nullable = true
+    }
+}
+`, name, databaseName, schemaName)
+}

pkg/resources/view.go

@@ -44,10 +44,11 @@ var viewSchema = map[string]*schema.Schema{
 		Type:        schema.TypeBool,
 		Optional:    true,
 		Default:     false,
-		Description: "Retains the access permissions from the original view when a new view is created using the OR REPLACE clause.",
+		Description: "Retains the access permissions from the original view when a new view is created using the OR REPLACE clause. OR REPLACE must be set when COPY GRANTS is set.",
 		DiffSuppressFunc: func(k, oldValue, newValue string, d *schema.ResourceData) bool {
 			return oldValue != "" && oldValue != newValue
 		},
+		RequiredWith: []string{"or_replace"},
 	},
 	"is_secure": {
 		Type:        schema.TypeBool,

pkg/resources/view_acceptance_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"database/sql"
 	"fmt"
+	"regexp"
 	"strings"
 	"testing"
 
@@ -378,6 +379,38 @@ func TestAcc_ViewStatementUpdate(t *testing.T) {
 	})
 }
 
+func TestAcc_View_copyGrants(t *testing.T) {
+	accName := strings.ToUpper(acctest.RandStringFromCharSet(10, acctest.CharSetAlpha))
+	query := "SELECT ROLE_NAME, ROLE_OWNER FROM INFORMATION_SCHEMA.APPLICABLE_ROLES"
+
+	resource.Test(t, resource.TestCase{
+		ProtoV6ProviderFactories: acc.TestAccProtoV6ProviderFactories,
+		PreCheck:                 func() { acc.TestAccPreCheck(t) },
+		TerraformVersionChecks: []tfversion.TerraformVersionCheck{
+			tfversion.RequireAbove(tfversion.Version1_5_0),
+		},
+		CheckDestroy: testAccCheckViewDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config:      viewConfigWithCopyGrants(acc.TestDatabaseName, acc.TestSchemaName, accName, query, true),
+				ExpectError: regexp.MustCompile("all of `copy_grants,or_replace` must be specified"),
+			},
+			{
+				Config: viewConfigWithCopyGrantsAndOrReplace(acc.TestDatabaseName, acc.TestSchemaName, accName, query, true, true),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("snowflake_view.test", "name", accName),
+				),
+			},
+			{
+				Config: viewConfigWithOrReplace(acc.TestDatabaseName, acc.TestSchemaName, accName, query, true),
+				Check: resource.ComposeTestCheckFunc(
+					resource.TestCheckResourceAttr("snowflake_view.test", "name", accName),
+				),
+			},
+		},
+	})
+}
+
 func viewConfigWithGrants(databaseName string, schemaName string, selectStatement string) string {
 	return fmt.Sprintf(`
 resource "snowflake_table" "table" {
@@ -430,6 +463,43 @@ data "snowflake_grants" "grants" {
 		databaseName, schemaName)
 }
 
+func viewConfigWithCopyGrants(databaseName string, schemaName string, name string, selectStatement string, copyGrants bool) string {
+	return fmt.Sprintf(`
+resource "snowflake_view" "test" {
+  name = "%[3]s"
+  database = "%[1]s"
+  schema = "%[2]s"
+  statement = "%[4]s"
+  copy_grants = %[5]t
+}
+	`, databaseName, schemaName, name, selectStatement, copyGrants)
+}
+
+func viewConfigWithCopyGrantsAndOrReplace(databaseName string, schemaName string, name string, selectStatement string, copyGrants bool, orReplace bool) string {
+	return fmt.Sprintf(`
+resource "snowflake_view" "test" {
+  name = "%[3]s"
+  database = "%[1]s"
+  schema = "%[2]s"
+  statement = "%[4]s"
+  copy_grants = %[5]t
+  or_replace = %[6]t
+}
+	`, databaseName, schemaName, name, selectStatement, copyGrants, orReplace)
+}
+
+func viewConfigWithOrReplace(databaseName string, schemaName string, name string, selectStatement string, orReplace bool) string {
+	return fmt.Sprintf(`
+resource "snowflake_view" "test" {
+  name = "%[3]s"
+  database = "%[1]s"
+  schema = "%[2]s"
+  statement = "%[4]s"
+  or_replace = %[5]t
+}
+	`, databaseName, schemaName, name, selectStatement, orReplace)
+}
+
 func testAccCheckViewDestroy(s *terraform.State) error {
 	db := acc.TestAccProvider.Meta().(*sql.DB)
 	client := sdk.NewClientFromDB(db)

pkg/sdk/tables.go

@@ -564,11 +564,31 @@ func (v *Table) GetClusterByKeys() []string {
 	}
 
 	statementWithoutLinear := strings.TrimSuffix(strings.Replace(v.ClusterBy, "LINEAR(", "", 1), ")")
-	keysRaw := strings.Split(statementWithoutLinear, ",")
-	keysClean := make([]string, 0, len(keysRaw))
-	for _, key := range keysRaw {
-		keysClean = append(keysClean, strings.TrimSpace(key))
+	return splitClusterBy(statementWithoutLinear)
+}
+
+func splitClusterBy(statementWithoutLinear string) []string {
+	keysClean := make([]string, 0)
+
+	var current string
+	var open int
+	for _, character := range statementWithoutLinear {
+		strChar := string(character)
+		switch strChar {
+		case "(":
+			open++
+		case ")":
+			open--
+		case ",":
+			if open == 0 {
+				keysClean = append(keysClean, strings.TrimSpace(current))
+				current = ""
+				continue
+			}
+		}
+		current += strChar
 	}
+	keysClean = append(keysClean, strings.TrimSpace(current))
 
 	return keysClean
 }

pkg/sdk/tables_test.go

@@ -1629,4 +1629,22 @@ func TestTable_GetClusterByKeys(t *testing.T) {
 
 		assert.Equal(t, []string{"abc", "def"}, table.GetClusterByKeys())
 	})
+
+	t.Run("with function with one param", func(t *testing.T) {
+		table := Table{ClusterBy: "LINEAR(some_func(some_param),other_param)"}
+
+		assert.Equal(t, []string{"some_func(some_param)", "other_param"}, table.GetClusterByKeys())
+	})
+
+	t.Run("with function with more than one param", func(t *testing.T) {
+		table := Table{ClusterBy: "LINEAR(date_trunc('HOUR',TIMESTAMP_HR),CLIENT_ID)"}
+
+		assert.Equal(t, []string{"date_trunc('HOUR',TIMESTAMP_HR)", "CLIENT_ID"}, table.GetClusterByKeys())
+	})
+
+	t.Run("with nested functions", func(t *testing.T) {
+		table := Table{ClusterBy: "LINEAR(some_func(some_param, some_other_param, other_func(some_param, some_other_param)),other_param)"}
+
+		assert.Equal(t, []string{"some_func(some_param, some_other_param, other_func(some_param, some_other_param))", "other_param"}, table.GetClusterByKeys())
+	})
 }