From 9c8c2a3d0f57a442984d9745fe85ea8b3a9e7216 Mon Sep 17 00:00:00 2001 From: Bobby Iliev Date: Wed, 15 Nov 2023 13:15:46 +0200 Subject: [PATCH 1/2] Add ForceNew for table column name changes --- pkg/provider/acceptance_table_test.go | 63 +++++++++++++++++++++++++++ pkg/resources/resource_table.go | 1 + 2 files changed, 64 insertions(+) diff --git a/pkg/provider/acceptance_table_test.go b/pkg/provider/acceptance_table_test.go index 471da557..2db1f5c2 100644 --- a/pkg/provider/acceptance_table_test.go +++ b/pkg/provider/acceptance_table_test.go @@ -76,6 +76,30 @@ func TestAccTable_update(t *testing.T) { resource.TestCheckResourceAttr("materialize_table.test_role", "ownership_role", roleName), ), }, + { + Config: testAccTableResourceWithUpdatedColumnName(roleName, tableName, tableRoleName, "mz_system", "new_column_1"), + Check: resource.ComposeTestCheckFunc( + testAccCheckTableExists("materialize_table.test"), + resource.TestCheckResourceAttr("materialize_table.test", "name", tableName), + resource.TestCheckResourceAttr("materialize_table.test", "column.0.name", "new_column_1"), + resource.TestCheckResourceAttr("materialize_table.test", "column.0.type", "text"), + resource.TestCheckResourceAttr("materialize_table.test", "column.1.name", "column_2"), + resource.TestCheckResourceAttr("materialize_table.test", "column.1.type", "int"), + resource.TestCheckResourceAttr("materialize_table.test", "column.2.name", "column_3"), + resource.TestCheckResourceAttr("materialize_table.test", "column.2.type", "text"), + resource.TestCheckResourceAttr("materialize_table.test", "schema_name", "public"), + resource.TestCheckResourceAttr("materialize_table.test", "database_name", "materialize"), + resource.TestCheckResourceAttr("materialize_table.test", "ownership_role", "mz_system"), + resource.TestCheckResourceAttr("materialize_table.test", "qualified_sql_name", fmt.Sprintf(`"materialize"."public"."%s"`, tableName)), + resource.TestCheckResourceAttr("materialize_table.test", "column.#", "3"), + testAccCheckTableExists("materialize_table.test_role"), + resource.TestCheckResourceAttr("materialize_table.test_role", "name", tableRoleName), + resource.TestCheckResourceAttr("materialize_table.test_role", "ownership_role", roleName), + ), + ResourceName: "materialize_table.test", + ImportState: true, + ImportStateVerify: true, + }, }, }) } @@ -180,3 +204,42 @@ func testAccCheckAllTablesDestroyed(s *terraform.State) error { return nil } + +func testAccTableResourceWithUpdatedColumnName(roleName, tableName, tableRoleName, tableOwnership, newColumnName string) string { + return fmt.Sprintf(` +resource "materialize_role" "test" { + name = "%s" +} + +resource "materialize_table" "test" { + name = "%s" + comment = "comment" + column { + name = "%s" + type = "text" + } + column { + name = "column_2" + type = "int" + comment = "comment" + } + column { + name = "column_3" + type = "text" + nullable = true + } +} + +resource "materialize_table" "test_role" { + name = "%s" + ownership_role = "%s" + + column { + name = "column_1" + type = "text" + } + + depends_on = [materialize_role.test] +} +`, roleName, tableName, newColumnName, tableRoleName, tableOwnership) +} diff --git a/pkg/resources/resource_table.go b/pkg/resources/resource_table.go index 10289b6d..93419d42 100644 --- a/pkg/resources/resource_table.go +++ b/pkg/resources/resource_table.go @@ -27,6 +27,7 @@ var tableSchema = map[string]*schema.Schema{ Description: "The name of the column to be created in the table.", Type: schema.TypeString, Required: true, + ForceNew: true, }, "type": { Description: "The data type of the column indicated by name.", From aa575678bea9547a6d45898372f72e40fe2e4e95 Mon Sep 17 00:00:00 2001 From: Bobby Iliev Date: Wed, 15 Nov 2023 15:02:21 +0200 Subject: [PATCH 2/2] Table column comments updates --- pkg/provider/acceptance_table_test.go | 75 +++++++++++++++++---------- pkg/resources/resource_table.go | 27 ++++++---- 2 files changed, 66 insertions(+), 36 deletions(-) diff --git a/pkg/provider/acceptance_table_test.go b/pkg/provider/acceptance_table_test.go index 2db1f5c2..8dc88f29 100644 --- a/pkg/provider/acceptance_table_test.go +++ b/pkg/provider/acceptance_table_test.go @@ -77,7 +77,7 @@ func TestAccTable_update(t *testing.T) { ), }, { - Config: testAccTableResourceWithUpdatedColumnName(roleName, tableName, tableRoleName, "mz_system", "new_column_1"), + Config: testAccTableResourceWithUpdates(roleName, tableName, tableRoleName, "mz_system", "new_column_1", ""), Check: resource.ComposeTestCheckFunc( testAccCheckTableExists("materialize_table.test"), resource.TestCheckResourceAttr("materialize_table.test", "name", tableName), @@ -100,6 +100,16 @@ func TestAccTable_update(t *testing.T) { ImportState: true, ImportStateVerify: true, }, + { + Config: testAccTableResourceWithUpdates(roleName, tableName, tableRoleName, "mz_system", "", "Updated comment"), + Check: resource.ComposeTestCheckFunc( + testAccCheckTableExists("materialize_table.test"), + resource.TestCheckResourceAttr("materialize_table.test", "column.1.comment", "Updated comment"), + resource.TestCheckResourceAttr("materialize_table.test", "name", tableName), + resource.TestCheckResourceAttr("materialize_table.test", "column.0.name", "column_1"), + resource.TestCheckResourceAttr("materialize_table.test", "column.0.type", "text"), + ), + }, }, }) } @@ -205,41 +215,52 @@ func testAccCheckAllTablesDestroyed(s *terraform.State) error { return nil } -func testAccTableResourceWithUpdatedColumnName(roleName, tableName, tableRoleName, tableOwnership, newColumnName string) string { +func testAccTableResourceWithUpdates(roleName, tableName, tableRoleName, tableOwnership, newColumnName, updatedComment string) string { + columnName1 := "column_1" + if newColumnName != "" { + columnName1 = newColumnName + } + + commentColumn2 := "comment" + if updatedComment != "" { + commentColumn2 = updatedComment + } + return fmt.Sprintf(` resource "materialize_role" "test" { - name = "%s" + name = "%s" } resource "materialize_table" "test" { - name = "%s" - comment = "comment" - column { - name = "%s" - type = "text" - } - column { - name = "column_2" - type = "int" - comment = "comment" - } - column { - name = "column_3" - type = "text" - nullable = true - } + name = "%s" + comment = "Initial table comment" + column { + name = "%s" + type = "text" + } + column { + name = "column_2" + type = "int" + comment = "%s" + } + column { + name = "column_3" + type = "text" + nullable = true + } + ownership_role = "%s" } resource "materialize_table" "test_role" { - name = "%s" - ownership_role = "%s" + name = "%s" + ownership_role = "%s" - column { - name = "column_1" - type = "text" - } + column { + name = "%s" + type = "text" + } - depends_on = [materialize_role.test] + depends_on = [materialize_role.test] } -`, roleName, tableName, newColumnName, tableRoleName, tableOwnership) +`, roleName, tableName, columnName1, commentColumn2, tableOwnership, tableRoleName, tableOwnership, columnName1) } diff --git a/pkg/resources/resource_table.go b/pkg/resources/resource_table.go index 93419d42..065af79c 100644 --- a/pkg/resources/resource_table.go +++ b/pkg/resources/resource_table.go @@ -33,6 +33,7 @@ var tableSchema = map[string]*schema.Schema{ Description: "The data type of the column indicated by name.", Type: schema.TypeString, Required: true, + ForceNew: true, StateFunc: func(val any) string { alias, ok := aliases[val.(string)] if ok { @@ -44,6 +45,7 @@ var tableSchema = map[string]*schema.Schema{ "nullable": { Description: "Do not allow the column to contain NULL values. Columns without this constraint can contain NULL values.", Type: schema.TypeBool, + ForceNew: true, Optional: true, Default: false, }, @@ -233,15 +235,22 @@ func tableUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) } } - if d.HasChange("columns") { - _, newColumns := d.GetChange("columns") - columns := materialize.GetTableColumnStruct(newColumns.([]interface{})) - comment := materialize.NewCommentBuilder(meta.(*sqlx.DB), o) - - // Reset all comments if change present - for _, c := range columns { - if c.Comment != "" { - if err := comment.Column(c.ColName, c.Comment); err != nil { + if d.HasChange("column") { + oldColumns, newColumns := d.GetChange("column") + oldColumnsList := oldColumns.([]interface{}) + newColumnsList := newColumns.([]interface{}) + + for index, newColMap := range newColumnsList { + newCol := newColMap.(map[string]interface{}) + oldCol := oldColumnsList[index].(map[string]interface{}) + + // Check specifically if the column comment has changed. + if newCol["comment"] != oldCol["comment"] { + // Apply the comment change + comment := materialize.NewCommentBuilder(meta.(*sqlx.DB), o) + colName := newCol["name"].(string) + colComment := newCol["comment"].(string) + if err := comment.Column(colName, colComment); err != nil { return diag.FromErr(err) } }