Skip to content

Commit 9be583d

Browse files
nkvuongalexott
nkvuong
and
alexott
authored
[Fix] Populate partitions when reading databricks_sql_table (#4674)
## Changes - Redo #4486, marking `partitions` column as optional computed to avoid unexpected diff. - UC returns `partition_index` in the column struct, so we can calculate partitions value based on that - Resolves #3980 ## Tests <!-- How is this tested? Please see the checklist below and also describe any other relevant tests --> - [x] `make test` run locally - [x] relevant change in `docs/` folder - [x] covered with integration tests in `internal/acceptance` - [x] using Go SDK - [x] using TF Plugin Framework --------- Co-authored-by: Alex Ott <[email protected]>
1 parent 2382f26 commit 9be583d

File tree

4 files changed

+183
-2
lines changed

4 files changed

+183
-2
lines changed

‎NEXT_CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
### Bug Fixes
1010

11+
* Populate `partitions` when reading `databricks_sql_table` ([#4674](https://github.com/databricks/terraform-provider-databricks/pull/4674)).
1112
* Fail when creating `databricks_query` and `databricks_alert` with already existing names [#4697](https://github.com/databricks/terraform-provider-databricks/pull/4697).
1213

1314
### Documentation

‎catalog/resource_sql_table.go‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"encoding/json"
66
"fmt"
77
"log"
8+
"maps"
89
"reflect"
910
"slices"
1011
"strings"
@@ -49,7 +50,7 @@ type SqlTableInfo struct {
4950
TableType string `json:"table_type" tf:"force_new"`
5051
DataSourceFormat string `json:"data_source_format,omitempty" tf:"force_new"`
5152
ColumnInfos []SqlColumnInfo `json:"columns,omitempty" tf:"alias:column,computed"`
52-
Partitions []string `json:"partitions,omitempty" tf:"force_new"`
53+
Partitions []string `json:"partitions,omitempty" tf:"force_new,computed"`
5354
ClusterKeys []string `json:"cluster_keys,omitempty"`
5455
StorageLocation string `json:"storage_location,omitempty" tf:"suppress_diff"`
5556
StorageCredentialName string `json:"storage_credential_name,omitempty" tf:"force_new"`
@@ -675,13 +676,37 @@ func ResourceSqlTable() common.Resource {
675676
if err != nil {
676677
return err
677678
}
679+
w, err := c.WorkspaceClient()
680+
if err != nil {
681+
return err
682+
}
683+
partitionInfo, err := w.Tables.GetByFullName(ctx, d.Id())
684+
if err != nil {
685+
return err
686+
}
687+
partitionIndexes := map[int]string{}
678688
for i := range ti.ColumnInfos {
679689
c := &ti.ColumnInfos[i]
680690
c.Identity, err = reconstructIdentity(c)
681691
if err != nil {
682692
return err
683693
}
684694
}
695+
696+
for i := range partitionInfo.Columns {
697+
c := &partitionInfo.Columns[i]
698+
if slices.Contains(c.ForceSendFields, "PartitionIndex") {
699+
partitionIndexes[c.PartitionIndex] = c.Name
700+
}
701+
}
702+
indexes := slices.Sorted(maps.Keys(partitionIndexes))
703+
704+
partitions := []string{}
705+
for _, p := range indexes {
706+
partitions = append(partitions, partitionIndexes[p])
707+
}
708+
709+
d.Set("partitions", partitions)
685710
return common.StructToData(ti, tableSchema, d)
686711
},
687712
Update: func(ctx context.Context, d *schema.ResourceData, c *common.DatabricksClient) error {

‎catalog/resource_sql_table_test.go‎

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,11 @@ func TestResourceSqlTableCreateTable(t *testing.T) {
249249
},
250250
},
251251
},
252+
{
253+
Method: "GET",
254+
Resource: "/api/2.1/unity-catalog/tables/main.foo.bar?",
255+
Response: catalog.TableInfo{},
256+
},
252257
}, useExistingClusterForSql...),
253258
Create: true,
254259
Resource: ResourceSqlTable(),
@@ -324,6 +329,11 @@ func TestResourceSqlTableCreateTableWithOwner(t *testing.T) {
324329
},
325330
},
326331
},
332+
{
333+
Method: "GET",
334+
Resource: "/api/2.1/unity-catalog/tables/main.foo.bar?",
335+
Response: catalog.TableInfo{},
336+
},
327337
}, useExistingClusterForSql...),
328338
Create: true,
329339
Resource: ResourceSqlTable(),
@@ -446,6 +456,11 @@ func TestResourceSqlTableUpdateTable(t *testing.T) {
446456
},
447457
},
448458
},
459+
{
460+
Method: "GET",
461+
Resource: "/api/2.1/unity-catalog/tables/main.foo.bar?",
462+
Response: catalog.TableInfo{},
463+
},
449464
{
450465
Method: "POST",
451466
Resource: "/api/2.0/clusters/start",
@@ -547,6 +562,11 @@ func TestResourceSqlTableUpdateTableAndOwner(t *testing.T) {
547562
Owner: "old group",
548563
},
549564
},
565+
{
566+
Method: "GET",
567+
Resource: "/api/2.1/unity-catalog/tables/main.foo.bar?",
568+
Response: catalog.TableInfo{},
569+
},
550570
{
551571
Method: "POST",
552572
Resource: "/api/2.0/clusters/start",
@@ -643,6 +663,11 @@ func TestResourceSqlTableUpdateTableClusterKeys(t *testing.T) {
643663
},
644664
},
645665
},
666+
{
667+
Method: "GET",
668+
Resource: "/api/2.1/unity-catalog/tables/main.foo.bar?",
669+
Response: catalog.TableInfo{},
670+
},
646671
{
647672
Method: "POST",
648673
Resource: "/api/2.0/clusters/start",
@@ -733,6 +758,11 @@ func TestResourceSqlTableUpdateView(t *testing.T) {
733758
},
734759
},
735760
},
761+
{
762+
Method: "GET",
763+
Resource: "/api/2.1/unity-catalog/tables/main.foo.bar?",
764+
Response: catalog.TableInfo{},
765+
},
736766
{
737767
Method: "POST",
738768
Resource: "/api/2.0/clusters/start",
@@ -842,6 +872,11 @@ func TestResourceSqlTableUpdateView_Definition(t *testing.T) {
842872
ViewDefinition: "SELECT * FROM main.foo.bar2",
843873
},
844874
},
875+
{
876+
Method: "GET",
877+
Resource: "/api/2.1/unity-catalog/tables/main.foo.barview?",
878+
Response: catalog.TableInfo{},
879+
},
845880
{
846881
Method: "POST",
847882
Resource: "/api/2.0/clusters/start",
@@ -897,6 +932,11 @@ func TestResourceSqlTableUpdateView_Comments(t *testing.T) {
897932
Comment: "to be changed (requires new)",
898933
},
899934
},
935+
{
936+
Method: "GET",
937+
Resource: "/api/2.1/unity-catalog/tables/main.foo.barview?",
938+
Response: catalog.TableInfo{},
939+
},
900940
{
901941
Method: "POST",
902942
Resource: "/api/2.0/clusters/start",
@@ -989,6 +1029,11 @@ func resourceSqlTableUpdateColumnHelper(t *testing.T, testMetaData resourceSqlTa
9891029
ColumnInfos: testMetaData.oldColumns,
9901030
},
9911031
},
1032+
{
1033+
Method: "GET",
1034+
Resource: "/api/2.1/unity-catalog/tables/main.foo.bar?",
1035+
Response: catalog.TableInfo{},
1036+
},
9921037
{
9931038
Method: "POST",
9941039
Resource: "/api/2.0/clusters/start",
@@ -1377,6 +1422,11 @@ func TestResourceSqlTableCreateTable_ExistingSQLWarehouse(t *testing.T) {
13771422
},
13781423
},
13791424
},
1425+
{
1426+
Method: "GET",
1427+
Resource: "/api/2.1/unity-catalog/tables/main.foo.bar?",
1428+
Response: catalog.TableInfo{},
1429+
},
13801430
},
13811431
Create: true,
13821432
Resource: ResourceSqlTable(),
@@ -1470,6 +1520,11 @@ func TestResourceSqlTableCreateTableWithIdentityColumn_ExistingSQLWarehouse(t *t
14701520
},
14711521
},
14721522
},
1523+
{
1524+
Method: "GET",
1525+
Resource: "/api/2.1/unity-catalog/tables/main.foo.bar?",
1526+
Response: catalog.TableInfo{},
1527+
},
14731528
},
14741529
Create: true,
14751530
Resource: ResourceSqlTable(),
@@ -1536,6 +1591,99 @@ func TestResourceSqlTableReadTableWithIdentityColumn_ExistingSQLWarehouse(t *tes
15361591
},
15371592
},
15381593
},
1594+
{
1595+
Method: "GET",
1596+
Resource: "/api/2.1/unity-catalog/tables/main.foo.bar?",
1597+
Response: catalog.TableInfo{},
1598+
},
1599+
},
1600+
ID: "main.foo.bar",
1601+
Read: true,
1602+
Resource: ResourceSqlTable(),
1603+
}.ApplyAndExpectData(t, map[string]any{
1604+
"column.0.identity": "always",
1605+
"column.1.identity": "",
1606+
"column.2.identity": "default",
1607+
})
1608+
}
1609+
1610+
func TestResourceSqlTableReadTableWithPartitionColumn_ExistingSQLWarehouse(t *testing.T) {
1611+
qa.ResourceFixture{
1612+
CommandMock: func(commandStr string) common.CommandResults {
1613+
return common.CommandResults{
1614+
ResultType: "",
1615+
Data: nil,
1616+
}
1617+
},
1618+
HCL: `
1619+
name = "bar"
1620+
catalog_name = "main"
1621+
schema_name = "foo"
1622+
table_type = "MANAGED"
1623+
data_source_format = "DELTA"
1624+
storage_location = "abfss://container@account/somepath"
1625+
warehouse_id = "existingwarehouse"
1626+
1627+
1628+
comment = "this table is managed by terraform"
1629+
`,
1630+
Fixtures: []qa.HTTPFixture{
1631+
{
1632+
Method: "GET",
1633+
Resource: "/api/2.1/unity-catalog/tables/main.foo.bar",
1634+
Response: SqlTableInfo{
1635+
Name: "bar",
1636+
CatalogName: "main",
1637+
SchemaName: "foo",
1638+
TableType: "EXTERNAL",
1639+
DataSourceFormat: "DELTA",
1640+
StorageLocation: "s3://ext-main/foo/bar1",
1641+
StorageCredentialName: "somecred",
1642+
Comment: "terraform managed",
1643+
Properties: map[string]string{
1644+
"one": "two",
1645+
"three": "four",
1646+
},
1647+
ColumnInfos: []SqlColumnInfo{
1648+
{
1649+
Name: "id",
1650+
Type: "bigint",
1651+
TypeJson: "{\"type\":\"bigint\",\"nullable\":true, \"metadata\":{\"delta.identity.start\":1,\"delta.identity.allowExplicitInsert\":false}}",
1652+
},
1653+
{
1654+
Name: "name",
1655+
Type: "string",
1656+
Comment: "name of thing",
1657+
},
1658+
{
1659+
Name: "number",
1660+
Type: "bigint",
1661+
TypeJson: "{\"type\":\"bigint\",\"nullable\":true, \"metadata\":{\"delta.identity.start\":1,\"delta.identity.allowExplicitInsert\":true}}",
1662+
},
1663+
},
1664+
},
1665+
},
1666+
{
1667+
Method: "GET",
1668+
Resource: "/api/2.1/unity-catalog/tables/main.foo.bar?",
1669+
Response: catalog.TableInfo{
1670+
Columns: []catalog.ColumnInfo{
1671+
{
1672+
Name: "id",
1673+
PartitionIndex: 1,
1674+
ForceSendFields: []string{"PartitionIndex"},
1675+
},
1676+
{
1677+
Name: "name",
1678+
PartitionIndex: 0,
1679+
ForceSendFields: []string{"PartitionIndex"},
1680+
},
1681+
{
1682+
Name: "number",
1683+
},
1684+
},
1685+
},
1686+
},
15391687
},
15401688
ID: "main.foo.bar",
15411689
Read: true,
@@ -1544,6 +1692,7 @@ func TestResourceSqlTableReadTableWithIdentityColumn_ExistingSQLWarehouse(t *tes
15441692
"column.0.identity": "always",
15451693
"column.1.identity": "",
15461694
"column.2.identity": "default",
1695+
"partitions": []any{"name", "id"},
15471696
})
15481697
}
15491698

@@ -1607,6 +1756,11 @@ func TestResourceSqlTableCreateTable_OnlyManagedProperties(t *testing.T) {
16071756
},
16081757
},
16091758
},
1759+
{
1760+
Method: "GET",
1761+
Resource: "/api/2.1/unity-catalog/tables/main.foo.bar?",
1762+
Response: catalog.TableInfo{},
1763+
},
16101764
},
16111765
Create: true,
16121766
Resource: ResourceSqlTable(),
@@ -1718,6 +1872,7 @@ func TestResourceSqlTable_Diff_ExistingResource(t *testing.T) {
17181872
instanceState["cluster_id"] = "test-1234"
17191873
instanceState["column.#"] = "0"
17201874
instanceState["owner"] = "testuser"
1875+
instanceState["partitions.#"] = "0"
17211876

17221877
expectedDiff := testCase.expectedDiff
17231878
if expectedDiff == nil {

‎docs/resources/sql_table.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ The following arguments are supported:
190190

191191
### `column` configuration block
192192

193-
For table columns
193+
For table columns. These can be optional for external tables.
194194
Currently, changing the column definitions for a table will require dropping and re-creating the table
195195

196196
* `name` - User-visible name of column

0 commit comments

Comments
 (0)