Terraform plan permits renaming BigQuery column

[ronan-mch]

I tried to rename a Google BigQuery column by altering the Terraform schema. terraform plan reported that the table would be changed (see below), but terraform apply led to an error from Google BigQuery. As it turns out, Google BigQuery does not permit renaming columns but perhaps this should be flagged by terraform plan?

Plan output

An execution plan has been generated and is shown below.
Resource actions are indicated with the following symbols:
  ~ update in-place

Terraform will perform the following actions:

  ~ google_bigquery_table.<table_name>
      schema: "[{\"mode\":\"NULLABLE\",\"name\":\"time\",\"type\":\"TIMESTAMP\"},{\"mode\":\"NULLABLE\",\"name\":\"event_id\",\"type\":\"STRING\"}... {\"mode\":\"NULLABLE\",\"name\":\"<new_field>\",\"type\":\"STRING\"}]"


Plan: 0 to add, 1 to change, 0 to destroy.

Apply Output

google_bigquery_table.<table_name>: googleapi: Error 400: Provided Schema does not match Table <table_name>. Field <old_field> is missing in new schema, invalid

Terraform Version

Terraform v0.11.3
+ provider.google v1.6.0

Affected Resource(s)

Please list the resources as a list, for example:

• google_bigquery

[nat-henderson]

The easiest solution would be to make schema changes ForceNew, since almost all of them are. But that seems like overkill... let's see if there's something simpler that can be done.

[nat-henderson]

I'm not sure there is something simpler, and there's no API to check whether an update is possible. Let's see what people think - is it better to get this error during apply, or is it better to have all schema changes be ForceNew?

[ronan-mch]

As an end user I would prefer to get an error from plan or perhaps even a dialog saying "Hey, BigQuery doesn't permit this - do you want to remake the table (you will lose all your data)". The thing is that forcing recreation of the table is a big step and I don't think any user would want to do this without carefully planning and preparing for it first (you could for example create a new table with the updated schema and copy your existing data to that before deleting the old one.

Anyway that's just my 2 cents, thanks for looking into this!

[dasch]

I think the error only happens when removing a column, right? If so, the plan stage could check the diff and see if that’s the case, returning an error? I don’t think ForceNew is required, as some schema changes should work.

…

On Sat, 3 Mar 2018 at 21.17, Ronan McHugh ***@***.***> wrote: As an end user I would prefer to get an error from plan or perhaps even a dialog saying "Hey, BigQuery doesn't permit this - do you want to remake the table (*you will lose all your data*)". The thing is that forcing recreation of the table is a big step and I don't think any user would want to do this without carefully planning and preparing for it first (you could for example create a new table with the updated schema and copy your existing data to that before deleting the old one. Anyway that's just my 2 cents, thanks for looking into this! — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1144 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAYz0jh7qfWtjNy8fjdggkZlYOaa9VKks5tavpwgaJpZM4SaGfS> .

[nat-henderson]

I think you folks are probably right that ForceNew is a drastic step. @dasch it's unfortunately not as simple as "removing a column" - there seem to be lots of kinds of changes that aren't supported, according to the BigQuery docs, and they aren't enumerated. That is the one that people are most likely to hit, in my opinion ... it could be that solving that case is the right way to go. I'll discuss this this week.

[nat-henderson]

I've changed my mind! @dasch has had a better idea by starting #1113, and the right move is probably to make this field ForceNew but to support a detailed system of diffs on the new fields introduced there.

[hcliff]

any follow up on this? while #1113 seems nice it appears to be stale

[nat-henderson]

Let me unassign this so that it gets picked up by our triage process next week. :)

[nat-henderson]

This is tracked by b/170333727.

[rileykarson]

Note that there's some exploration of the implementation in the internal bug above that's worth considering.

[rileykarson]

Revert incoming for this, reopening.

[maroshmka]

Hello, here's some feedback on this feature. I've checked #1113 solution written up in the thread, I didn't understood it tho as there is little explained for end user.

BQ doesn't support breaking changes, which means it supports only Addition and relaxation from required to nullable. All other changes are not supported (rename, change type, remove etc.) due to nature of database.

So, when breaking change occurs, only way to do it is to recreate table. Recreation of table deletes data. Either, you'll do data migration (which can't be done automatically for all cases) or you'll loose all your data. Also, note that reading data on BQ costs money. So, you usually need to figure out some other way of doing breaking changes.

Terraform should honor this restrictions and do not allow any breaking change to happen until explicitly set by user.

Can you guys provide more info or redirect me somewhere on what are the plans with this ? It caused us some troubles already. Not blaming here, it was also our fault, just wanted to express that we would be glad to have more info on this, mainly to the re-creation of the tables resource.

[rileykarson]

@maroshmka: Thanks for raising your concerns! The initial change definitely didn't work as expected- we'd introduced a regression in 3.53.0 that caused Terraform to unintentionally see diffs on tables that would have been correct in 3.52.0. Combined with this change- allowing it to attempt to recreate tables where schema changes were made- it was dangerous. #8337 covers some context and outlines what'll happen over the next couple of releases.

---

I want to speak to this feature outside of that bug, though. I think this is ultimately a desirable feature for Terraform. In version 3.53.0 and earlier if a user specifies an incompatible schema change and runs terraform apply Terraform fails to make the change and returns an error.

As intended, this change would instead cause Terraform to indicate that it wants to recreate the resource. From my perspective, that satisfies the Terraform should honor this restrictions and do not allow any breaking change to happen until explicitly set by user. point you raised. By accepting a terraform apply whose plan makes a breaking change in their table and recreates it, the user has indicated that Terraform should take action.

Additionally, from a provider versioning perspective I'll admit that this is walking a murky line between a breaking and non-breaking change. My take is ultimately that it is not a breaking change- previously Terraform would have indicated that it would make an update and then failed to make that update returning an error, and this increases the fidelity of the plan. Users who would not want their resource deleted under any circumstance can apply lifecycle.prevent_destroy to it.

That's not to say that I don't think we can improve how we roll this out, though, and I think we missed the mark when balancing against the safety of the user's resources here.

---

I'd like to propose improving the safety of the feature by making it more difficult to delete google_bigquery_table resources. We established a provider precedent for adding a deletion_protection field to resources that contain data, and that would be extremely undesirable to destroy for most users. I have a breakdown of the feature here and a past team member outlined their thoughts in a core issue. I believe we should do the same here.

That allows us to release this feature and let Terraform handle incompatible schema changes properly while simultaneously decreasing the likelihood that users will inadvertently destroy their resources (even by other causes).

@maroshmka and @ScottSuarez I'm curious about both of your thoughts (as well as anybody else who sees this comment & has any input!)

[maroshmka]

sorry for late reply.

ad tf in general - thanks, I didn't see it this way, but it make complete sense. From PoV of tf it does what it should. Plan says it will "delete" and then the apply will delete, so user actually "approves" it by running apply.

ad breaking change - again, is probably not a breaking change from PoV of tf. On the other hand, deleting data is generally a huge problem. So, considering this breaking change, even tho its not "technically" a breaking change, but more "logically, make sense here. Of course, this is up to you and internal discussion between committers how strictly do you consider something breaking. My suggestion would be to revisit it and try to define use-cases when it is not that clear and discuss them as I'd expect this won't be the only one.

ad deletion_protection - if I understood correctly, deletion_protection will be True by default, so it will be protected always unless user says it doesn't need to be. This is very welcomed and imho much more handy than lifecyle for the exact reasons you've mentioned in the bq issue.

thanks for great discussion 🚀

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!