terraform-provider-google icon indicating copy to clipboard operation
terraform-provider-google copied to clipboard

resource bigquery_table "forces replacement" additional conditions

Open dlesco opened this issue 3 years ago • 3 comments

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request.
  • Please do not leave +1 or me too comments, they generate extra noise for issue followers and do not help prioritize the request.
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment.
  • If an issue is assigned to the modular-magician user, it is either in the process of being autogenerated, or is planned to be autogenerated soon. If an issue is assigned to a user, that user is claiming responsibility for the issue. If an issue is assigned to hashibot, a community member has claimed the issue already.

Terraform Version

Current code in main branch: https://github.com/hashicorp/terraform-provider-google/blob/0f56a4f569d7d94006df282511eb9930ff37ee15/google/resource_bigquery_table.go

Affected Resource(s)

  • google_bigquery_table

Terraform Configuration Files

N/A

Debug Output

N/A

Panic Output

N/A

Expected Behavior

I see in the code that it's not handling certain cases where it should Force Replacement of the resource, instead of doing an update-in-place.

  1. When view attribute changes between nil and non-nil
  2. When materialized_view attribute changes between nil and non-nil.
  3. When external_data_configuration attribute changes between nil and non-nil.
  4. When doing schema change checking to see if the schema change forces replacement, it needs to add a special check for when the resource is a view (when the view attribute is non-nil). For the view case, if you're adding a new field to the schema, that should force replacement of the view. Because the BigQuery API forces you to recreate a view if you are adding new fields to the schema. FYI, we are starting to supply schemas for views for UI and documentation purposes, especially to add field descriptions, so that in the BQ UI and in Data Catalog, we have descriptions for the view fields.

Actual Behavior

The base table for a view had a new field added named new_field. The view query was

SELECT * FROM `i-dss-dw-poc.foo.bar`

When I edited the view definition to include "new_field" field, terraform treated it as an update-in-place, but then we got this BQ API error:

│ Error: googleapi: Error 400: Provided Schema does not match Table i-dss-dw-poc:foo_vw.bar. Cannot add fields (field: new_field), invalid

In order to fix the problem, I removed the code that generated this table resource, ran apply to drop the view, then added the code again that defines the resource, to force the drop-and-create. But it would be better if the provider forced replacement for this case.

Steps to Reproduce

  1. Define a physical table resource, apply it
  2. Define a view resource that does select star on the fields on the physical table, and supply a schema attribute that documents the fields. Apply it.
  3. Add a new field to the physical table, apply it.
  4. Edit the view resource to add the new field to the view schema, apply it

Important Factoids

References

  • #0000

dlesco avatar Jul 26 '22 15:07 dlesco

@dlesco if I understand correctly, you want to recreate the table when a new filed is added. In this case, do you need to keep the existing data in the table? In most use cases, force-replacement is the last resort. In your case, can't you add step to delete the resource first?

edwardmedia avatar Jul 26 '22 16:07 edwardmedia

@dlesco if I understand correctly, you want to recreate the table when a new filed is added. In this case, do you need to keep the existing data in the table? In most use cases, force-replacement is the last resort. In your case, can't you add step to delete the resource first?

Not a physical table, a view. When the type of the "table" is a View, rather than a Physical Table, the API does not allow you to add fields to the schema. You need to recreate the view. Views are also handled by this resource, because Views are a subtype of Table.

Views do not have any physical data that will be dropped when the view is recreated.

Deleting the resource from the TF code, then re-adding is possible (as a work-around), but it is a bad workflow for our developers. We have a framework around the TF code where we have a yaml file per view in the dataset. To delete first, then recreate, they'd first have to do one PR to rename the yaml file to a .bak file, get the PR approved and merged, have CICD deploy it, then do a second PR to rename the yaml file back so the view is redefined, get it approved, merged and deployed.

Much better workflow is if the provider detects that this is a view where the schema has an added field, and chooses Forces Replacement strategy. Then the developer can just do one PR with the schema change, and the one PR approval, merge, and deploy will take care of changing the schema.

dlesco avatar Jul 27 '22 14:07 dlesco

@dlesco I see what you meant now. I do not believe there is a mechanism available in terraform that can detect a change in another resource and handle something accordingly. To your case, besides deleting the view, you may also take a look at terrafrom taint to see if you can add it in your pipeline. What do you think?

edwardmedia avatar Jul 28 '22 13:07 edwardmedia

@dlesco is this still an issue?

edwardmedia avatar Aug 11 '22 17:08 edwardmedia

@dlesco closing this assuming it is no longer an issue

edwardmedia avatar Aug 15 '22 14:08 edwardmedia

Hi @edwardmedia, thank you for the suggestion, I think taint/replace could help in combination with some CI/CD logic. However, I would like to say that what this issue is suggesting would be a nice to have feature and I think it could be possible simulating a DDL instruction CREATE OR REPLACE. Whenever the apply is happening, the provider uses the tables.get method to get the view, I think it compares the returned object with the one you have defined in your tf code and then call tables.update. If that is true, then with an additional argument for google_bigquery_table and the proper validation during the apply process, the replacement it could be introduced; something like replacement_force let's say. If you look at the bq cli reference and search for --replace={true|false} you will see this approach is used for different use case but I think is still valid for the use case we are describing here.

msgongora avatar Aug 15 '22 20:08 msgongora

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 have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

github-actions[bot] avatar Sep 15 '22 02:09 github-actions[bot]