From 95935c4178c12481a967b0c92fb17ea376b39b6c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jan=20Cie=C5=9Blak?= <jan.cieslak@snowflake.com>
Date: Fri, 8 Mar 2024 10:37:07 +0100
Subject: [PATCH] wip

---
 pkg/resources/table.go                 |  6 ++-
 pkg/resources/table_acceptance_test.go | 51 ++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/pkg/resources/table.go b/pkg/resources/table.go
index 981073a80c..daec0b1902 100644
--- a/pkg/resources/table.go
+++ b/pkg/resources/table.go
@@ -818,7 +818,11 @@ func UpdateTable(d *schema.ResourceData, meta interface{}) error {
 		}
 		for _, cA := range changed {
 			if cA.changedDataType || cA.changedCollate {
-				err := client.Tables.Alter(ctx, sdk.NewAlterTableRequest(id).WithColumnAction(sdk.NewTableColumnActionRequest().WithAlter([]sdk.TableColumnAlterActionRequest{*sdk.NewTableColumnAlterActionRequest(fmt.Sprintf("\"%s\"", cA.newColumn.name)).WithType(sdk.Pointer(sdk.DataType(cA.newColumn.dataType))).WithCollate(sdk.String(cA.newColumn.collate))})))
+				var newCollation *string
+				if sdk.IsStringType(cA.newColumn.dataType) && cA.newColumn.collate != "" {
+					newCollation = sdk.String(cA.newColumn.collate)
+				}
+				err := client.Tables.Alter(ctx, sdk.NewAlterTableRequest(id).WithColumnAction(sdk.NewTableColumnActionRequest().WithAlter([]sdk.TableColumnAlterActionRequest{*sdk.NewTableColumnAlterActionRequest(fmt.Sprintf("\"%s\"", cA.newColumn.name)).WithType(sdk.Pointer(sdk.DataType(cA.newColumn.dataType))).WithCollate(newCollation)})))
 				if err != nil {
 					return fmt.Errorf("error changing property on %v: err %w", d.Id(), err)
 				}
diff --git a/pkg/resources/table_acceptance_test.go b/pkg/resources/table_acceptance_test.go
index ced83e9108..eb786115af 100644
--- a/pkg/resources/table_acceptance_test.go
+++ b/pkg/resources/table_acceptance_test.go
@@ -1844,6 +1844,57 @@ resource "snowflake_table" "test_table" {
 `, name, databaseName, schemaName)
 }
 
+// proves https://github.com/Snowflake-Labs/terraform-provider-snowflake/issues/2588 is fixed
+func TestAcc_ColumnTypeChangeWithNonTextType(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: tableConfigWithNumberColumnType(accName, acc.TestDatabaseName, acc.TestSchemaName, "NUMBER(38,0)"),
+				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", "column.0.name", "id"),
+					resource.TestCheckResourceAttr("snowflake_table.test_table", "column.0.type", "NUMBER(38,0)"),
+				),
+			},
+			{
+				Config: tableConfigWithNumberColumnType(accName, acc.TestDatabaseName, acc.TestSchemaName, "NUMBER(36,0)"),
+				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", "column.0.name", "id"),
+					resource.TestCheckResourceAttr("snowflake_table.test_table", "column.0.type", "NUMBER(36,0)"),
+				),
+			},
+		},
+	})
+}
+
+func tableConfigWithNumberColumnType(name string, databaseName string, schemaName string, columnType string) string {
+	return fmt.Sprintf(`
+resource "snowflake_table" "test_table" {
+	name                = "%[1]s"
+	database            = "%[2]s"
+	schema              = "%[3]s"
+
+	column {
+		name = "id"
+		type = "%[4]s"
+	}
+}
+`, name, databaseName, schemaName, columnType)
+}
+
 func checkDatabaseSchemaAndTableDataRetentionTime(id sdk.SchemaObjectIdentifier, expectedDatabaseRetentionDays int, expectedSchemaRetentionDays int, expectedTableRetentionsDays int) func(state *terraform.State) error {
 	return func(state *terraform.State) error {
 		client := acc.TestAccProvider.Meta().(*provider.Context).Client