Skip to content

Commit 088ff36

Browse files
modular-magicianmodular-magician
authored
Allow in-place column drop for bigquery table (#10170) (#17777)
[upstream:e94c56025ee1215d53e4e3ca83dd4568349dac7b] Signed-off-by: Modular Magician <magic-modules@google.com>
1 parent 678449a commit 088ff36

File tree

4 files changed

+225
-22
lines changed

4 files changed

+225
-22
lines changed

‎.changelog/10170.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:enhancement
2+
bigquery: added in-place column drop support for `bigquery_table`
3+
```

‎google/services/bigquery/resource_bigquery_table.go

Lines changed: 78 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -258,19 +258,17 @@ func bigQueryTableNormalizePolicyTags(val interface{}) interface{} {
258258

259259
// Compares two existing schema implementations and decides if
260260
// it is changeable.. pairs with a force new on not changeable
261-
func resourceBigQueryTableSchemaIsChangeable(old, new interface{}) (bool, error) {
261+
func resourceBigQueryTableSchemaIsChangeable(old, new interface{}, isExternalTable bool, topLevel bool) (bool, error) {
262262
switch old.(type) {
263263
case []interface{}:
264264
arrayOld := old.([]interface{})
265265
arrayNew, ok := new.([]interface{})
266+
sameNameColumns := 0
267+
droppedColumns := 0
266268
if !ok {
267269
// if not both arrays not changeable
268270
return false, nil
269271
}
270-
if len(arrayOld) > len(arrayNew) {
271-
// if not growing not changeable
272-
return false, nil
273-
}
274272
if err := bigQueryTablecheckNameExists(arrayOld); err != nil {
275273
return false, err
276274
}
@@ -291,16 +289,28 @@ func resourceBigQueryTableSchemaIsChangeable(old, new interface{}) (bool, error)
291289
}
292290
}
293291
for key := range mapOld {
294-
// all old keys should be represented in the new config
292+
// dropping top level columns can happen in-place
293+
// but this doesn't apply to external tables
295294
if _, ok := mapNew[key]; !ok {
296-
return false, nil
295+
if !topLevel || isExternalTable {
296+
return false, nil
297+
}
298+
droppedColumns += 1
299+
continue
297300
}
298-
if isChangable, err :=
299-
resourceBigQueryTableSchemaIsChangeable(mapOld[key], mapNew[key]); err != nil || !isChangable {
301+
302+
isChangable, err := resourceBigQueryTableSchemaIsChangeable(mapOld[key], mapNew[key], isExternalTable, false)
303+
if err != nil || !isChangable {
300304
return false, err
305+
} else if isChangable && topLevel {
306+
// top level column that exists in the new schema
307+
sameNameColumns += 1
301308
}
302309
}
303-
return true, nil
310+
// in-place column dropping alongside column additions is not allowed
311+
// as of now because user intention can be ambiguous (e.g. column renaming)
312+
newColumns := len(arrayNew) - sameNameColumns
313+
return (droppedColumns == 0) || (newColumns == 0), nil
304314
case map[string]interface{}:
305315
objectOld := old.(map[string]interface{})
306316
objectNew, ok := new.(map[string]interface{})
@@ -339,7 +349,7 @@ func resourceBigQueryTableSchemaIsChangeable(old, new interface{}) (bool, error)
339349
return false, nil
340350
}
341351
case "fields":
342-
return resourceBigQueryTableSchemaIsChangeable(valOld, valNew)
352+
return resourceBigQueryTableSchemaIsChangeable(valOld, valNew, isExternalTable, false)
343353

344354
// other parameters: description, policyTags and
345355
// policyTags.names[] are changeable
@@ -378,7 +388,8 @@ func resourceBigQueryTableSchemaCustomizeDiffFunc(d tpgresource.TerraformResourc
378388
// same as above
379389
log.Printf("[DEBUG] unable to unmarshal json customized diff - %v", err)
380390
}
381-
isChangeable, err := resourceBigQueryTableSchemaIsChangeable(old, new)
391+
_, isExternalTable := d.GetOk("external_data_configuration")
392+
isChangeable, err := resourceBigQueryTableSchemaIsChangeable(old, new, isExternalTable, true)
382393
if err != nil {
383394
return err
384395
}
@@ -1712,6 +1723,12 @@ func resourceBigQueryTableRead(d *schema.ResourceData, meta interface{}) error {
17121723
return nil
17131724
}
17141725

1726+
type TableReference struct {
1727+
project string
1728+
datasetID string
1729+
tableID string
1730+
}
1731+
17151732
func resourceBigQueryTableUpdate(d *schema.ResourceData, meta interface{}) error {
17161733
config := meta.(*transport_tpg.Config)
17171734
userAgent, err := tpgresource.GenerateUserAgentString(d, config.UserAgent)
@@ -1734,13 +1751,62 @@ func resourceBigQueryTableUpdate(d *schema.ResourceData, meta interface{}) error
17341751
datasetID := d.Get("dataset_id").(string)
17351752
tableID := d.Get("table_id").(string)
17361753

1754+
tableReference := &TableReference{
1755+
project: project,
1756+
datasetID: datasetID,
1757+
tableID: tableID,
1758+
}
1759+
1760+
if err = resourceBigQueryTableColumnDrop(config, userAgent, table, tableReference); err != nil {
1761+
return err
1762+
}
1763+
17371764
if _, err = config.NewBigQueryClient(userAgent).Tables.Update(project, datasetID, tableID, table).Do(); err != nil {
17381765
return err
17391766
}
17401767

17411768
return resourceBigQueryTableRead(d, meta)
17421769
}
17431770

1771+
func resourceBigQueryTableColumnDrop(config *transport_tpg.Config, userAgent string, table *bigquery.Table, tableReference *TableReference) error {
1772+
oldTable, err := config.NewBigQueryClient(userAgent).Tables.Get(tableReference.project, tableReference.datasetID, tableReference.tableID).Do()
1773+
if err != nil {
1774+
return err
1775+
}
1776+
1777+
newTableFields := map[string]bool{}
1778+
for _, field := range table.Schema.Fields {
1779+
newTableFields[field.Name] = true
1780+
}
1781+
1782+
droppedColumns := []string{}
1783+
for _, field := range oldTable.Schema.Fields {
1784+
if !newTableFields[field.Name] {
1785+
droppedColumns = append(droppedColumns, field.Name)
1786+
}
1787+
}
1788+
1789+
if len(droppedColumns) > 0 {
1790+
droppedColumnsString := strings.Join(droppedColumns, ", DROP COLUMN ")
1791+
1792+
dropColumnsDDL := fmt.Sprintf("ALTER TABLE `%s.%s.%s` DROP COLUMN %s", tableReference.project, tableReference.datasetID, tableReference.tableID, droppedColumnsString)
1793+
log.Printf("[INFO] Dropping columns in-place: %s", dropColumnsDDL)
1794+
1795+
useLegacySQL := false
1796+
req := &bigquery.QueryRequest{
1797+
Query: dropColumnsDDL,
1798+
UseLegacySql: &useLegacySQL,
1799+
}
1800+
1801+
_, err = config.NewBigQueryClient(userAgent).Jobs.Query(tableReference.project, req).Do()
1802+
if err != nil {
1803+
return err
1804+
}
1805+
}
1806+
1807+
return nil
1808+
}
1809+
17441810
func resourceBigQueryTableDelete(d *schema.ResourceData, meta interface{}) error {
17451811
if d.Get("deletion_protection").(bool) {
17461812
return fmt.Errorf("cannot destroy instance without setting deletion_protection=false and running `terraform apply`")

‎google/services/bigquery/resource_bigquery_table_internal_test.go

Lines changed: 55 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -391,10 +391,11 @@ func TestBigQueryTableSchemaDiffSuppress(t *testing.T) {
391391
}
392392

393393
type testUnitBigQueryDataTableJSONChangeableTestCase struct {
394-
name string
395-
jsonOld string
396-
jsonNew string
397-
changeable bool
394+
name string
395+
jsonOld string
396+
jsonNew string
397+
isExternalTable bool
398+
changeable bool
398399
}
399400

400401
func (testcase *testUnitBigQueryDataTableJSONChangeableTestCase) check(t *testing.T) {
@@ -405,7 +406,7 @@ func (testcase *testUnitBigQueryDataTableJSONChangeableTestCase) check(t *testin
405406
if err := json.Unmarshal([]byte(testcase.jsonNew), &new); err != nil {
406407
t.Fatalf("unable to unmarshal json - %v", err)
407408
}
408-
changeable, err := resourceBigQueryTableSchemaIsChangeable(old, new)
409+
changeable, err := resourceBigQueryTableSchemaIsChangeable(old, new, testcase.isExternalTable, true)
409410
if err != nil {
410411
t.Errorf("%s failed unexpectedly: %s", testcase.name, err)
411412
}
@@ -421,6 +422,11 @@ func (testcase *testUnitBigQueryDataTableJSONChangeableTestCase) check(t *testin
421422
d.Before["schema"] = testcase.jsonOld
422423
d.After["schema"] = testcase.jsonNew
423424

425+
if testcase.isExternalTable {
426+
d.Before["external_data_configuration"] = ""
427+
d.After["external_data_configuration"] = ""
428+
}
429+
424430
err = resourceBigQueryTableSchemaCustomizeDiffFunc(d)
425431
if err != nil {
426432
t.Errorf("error on testcase %s - %v", testcase.name, err)
@@ -430,7 +436,7 @@ func (testcase *testUnitBigQueryDataTableJSONChangeableTestCase) check(t *testin
430436
}
431437
}
432438

433-
var testUnitBigQueryDataTableIsChangableTestCases = []testUnitBigQueryDataTableJSONChangeableTestCase{
439+
var testUnitBigQueryDataTableIsChangeableTestCases = []testUnitBigQueryDataTableJSONChangeableTestCase{
434440
{
435441
name: "defaultEquality",
436442
jsonOld: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
@@ -447,7 +453,14 @@ var testUnitBigQueryDataTableIsChangableTestCases = []testUnitBigQueryDataTableJ
447453
name: "arraySizeDecreases",
448454
jsonOld: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }, {\"name\": \"asomeValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
449455
jsonNew: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
450-
changeable: false,
456+
changeable: true,
457+
},
458+
{
459+
name: "externalArraySizeDecreases",
460+
jsonOld: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }, {\"name\": \"asomeValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
461+
jsonNew: "[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
462+
isExternalTable: true,
463+
changeable: false,
451464
},
452465
{
453466
name: "descriptionChanges",
@@ -525,6 +538,24 @@ var testUnitBigQueryDataTableIsChangableTestCases = []testUnitBigQueryDataTableJ
525538
jsonNew: "[{\"name\": \"value3\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"newVal\" }, {\"name\": \"value1\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
526539
changeable: false,
527540
},
541+
{
542+
name: "renameRequiredColumn",
543+
jsonOld: "[{\"name\": \"value1\", \"type\" : \"INTEGER\", \"mode\" : \"REQUIRED\", \"description\" : \"someVal\" }]",
544+
jsonNew: "[{\"name\": \"value3\", \"type\" : \"INTEGER\", \"mode\" : \"REQUIRED\", \"description\" : \"someVal\" }]",
545+
changeable: false,
546+
},
547+
{
548+
name: "renameNullableColumn",
549+
jsonOld: "[{\"name\": \"value1\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
550+
jsonNew: "[{\"name\": \"value3\", \"type\" : \"INTEGER\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
551+
changeable: false,
552+
},
553+
{
554+
name: "typeModeReqToNullAndColumnDropped",
555+
jsonOld: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"REQUIRED\", \"description\" : \"someVal\" }, {\"name\": \"someValue2\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"someVal\" }]",
556+
jsonNew: "[{\"name\": \"someValue\", \"type\" : \"BOOLEAN\", \"mode\" : \"NULLABLE\", \"description\" : \"some new value\" }]",
557+
changeable: true,
558+
},
528559
{
529560
name: "policyTags",
530561
jsonOld: `[
@@ -550,15 +581,29 @@ var testUnitBigQueryDataTableIsChangableTestCases = []testUnitBigQueryDataTableJ
550581
},
551582
}
552583

553-
func TestUnitBigQueryDataTable_schemaIsChangable(t *testing.T) {
584+
func TestUnitBigQueryDataTable_schemaIsChangeable(t *testing.T) {
554585
t.Parallel()
555-
for _, testcase := range testUnitBigQueryDataTableIsChangableTestCases {
586+
for _, testcase := range testUnitBigQueryDataTableIsChangeableTestCases {
556587
testcase.check(t)
588+
}
589+
}
590+
591+
func TestUnitBigQueryDataTable_schemaIsChangeableNested(t *testing.T) {
592+
t.Parallel()
593+
// Only top level column drops are changeable
594+
customNestedValues := map[string]bool{"arraySizeDecreases": false, "typeModeReqToNullAndColumnDropped": false}
595+
for _, testcase := range testUnitBigQueryDataTableIsChangeableTestCases {
596+
changeable := testcase.changeable
597+
if overrideValue, ok := customNestedValues[testcase.name]; ok {
598+
changeable = overrideValue
599+
}
600+
557601
testcaseNested := &testUnitBigQueryDataTableJSONChangeableTestCase{
558602
testcase.name + "Nested",
559603
fmt.Sprintf("[{\"name\": \"someValue\", \"type\" : \"INTEGER\", \"fields\" : %s }]", testcase.jsonOld),
560604
fmt.Sprintf("[{\"name\": \"someValue\", \"type\" : \"INT64\", \"fields\" : %s }]", testcase.jsonNew),
561-
testcase.changeable,
605+
testcase.isExternalTable,
606+
changeable,
562607
}
563608
testcaseNested.check(t)
564609
}

‎google/services/bigquery/resource_bigquery_table_test.go

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,39 @@ func TestAccBigQueryTable_Basic(t *testing.T) {
4747
})
4848
}
4949

50+
func TestAccBigQueryTable_DropColumns(t *testing.T) {
51+
t.Parallel()
52+
53+
datasetID := fmt.Sprintf("tf_test_%s", acctest.RandString(t, 10))
54+
tableID := fmt.Sprintf("tf_test_%s", acctest.RandString(t, 10))
55+
56+
acctest.VcrTest(t, resource.TestCase{
57+
PreCheck: func() { acctest.AccTestPreCheck(t) },
58+
ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories(t),
59+
CheckDestroy: testAccCheckBigQueryTableDestroyProducer(t),
60+
Steps: []resource.TestStep{
61+
{
62+
Config: testAccBigQueryTableTimePartitioningDropColumns(datasetID, tableID),
63+
},
64+
{
65+
ResourceName: "google_bigquery_table.test",
66+
ImportState: true,
67+
ImportStateVerify: true,
68+
ImportStateVerifyIgnore: []string{"deletion_protection"},
69+
},
70+
{
71+
Config: testAccBigQueryTableTimePartitioningDropColumnsUpdate(datasetID, tableID),
72+
},
73+
{
74+
ResourceName: "google_bigquery_table.test",
75+
ImportState: true,
76+
ImportStateVerify: true,
77+
ImportStateVerifyIgnore: []string{"deletion_protection"},
78+
},
79+
},
80+
})
81+
}
82+
5083
func TestAccBigQueryTable_Kms(t *testing.T) {
5184
t.Parallel()
5285
resourceName := "google_bigquery_table.test"
@@ -1761,6 +1794,62 @@ EOH
17611794
`, datasetID, tableID, partitioningType)
17621795
}
17631796

1797+
func testAccBigQueryTableTimePartitioningDropColumns(datasetID, tableID string) string {
1798+
return fmt.Sprintf(`
1799+
resource "google_bigquery_dataset" "test" {
1800+
dataset_id = "%s"
1801+
}
1802+
1803+
resource "google_bigquery_table" "test" {
1804+
deletion_protection = false
1805+
table_id = "%s"
1806+
dataset_id = google_bigquery_dataset.test.dataset_id
1807+
1808+
schema = <<EOH
1809+
[
1810+
{
1811+
"name": "ts",
1812+
"type": "TIMESTAMP"
1813+
},
1814+
{
1815+
"name": "some_string",
1816+
"type": "STRING"
1817+
},
1818+
{
1819+
"name": "some_int",
1820+
"type": "INTEGER"
1821+
}
1822+
]
1823+
EOH
1824+
1825+
}
1826+
`, datasetID, tableID)
1827+
}
1828+
1829+
func testAccBigQueryTableTimePartitioningDropColumnsUpdate(datasetID, tableID string) string {
1830+
return fmt.Sprintf(`
1831+
resource "google_bigquery_dataset" "test" {
1832+
dataset_id = "%s"
1833+
}
1834+
1835+
resource "google_bigquery_table" "test" {
1836+
deletion_protection = false
1837+
table_id = "%s"
1838+
dataset_id = google_bigquery_dataset.test.dataset_id
1839+
1840+
schema = <<EOH
1841+
[
1842+
{
1843+
"name": "ts",
1844+
"type": "TIMESTAMP"
1845+
}
1846+
]
1847+
EOH
1848+
1849+
}
1850+
`, datasetID, tableID)
1851+
}
1852+
17641853
func testAccBigQueryTableKms(cryptoKeyName, datasetID, tableID string) string {
17651854
return fmt.Sprintf(`
17661855
resource "google_bigquery_dataset" "test" {

0 commit comments

Comments
 (0)