Remove the default value of view.use_legacy_sql in google_bigquery_table #14575
Conversation
|
Hello! I am a robot. Tests will require approval from a repository maintainer to run. Googlers: For automatic test runs see go/terraform-auto-test-runs. @rileykarson, a repository maintainer, has been assigned to review your changes. If you have not received review feedback within 2 business days, please leave a comment on this PR asking them to take a look. You can help make sure that review is quick by doing a self-review and by running impacted tests locally. |
|
Hi there, I'm the Modular magician. I've detected the following information about your changes: Diff reportYour PR generated some diffs in downstreams - here they are.
Breaking Change(s) DetectedThe following breaking change(s) were detected within your pull request.
If you believe this detection to be incorrect please raise the concern with your reviewer. |
Tests analyticsTotal tests: 111 Click here to see the affected service packages
Found 1 affected test(s) by replaying old test recordings. Starting RECORDING based on the most recent commit. Click here to see the affected tests
|
|
🔴 Tests failed during RECORDING mode: 🔴 Errors occurred during RECORDING mode. Please fix them to complete your PR. |
|
@rileykarson This PR has been waiting for review for 3 weekdays. Please take a look! Use the label |
|
@GoogleCloudPlatform/terraform-team @rileykarson This PR has been waiting for review for 1 week. Please take a look! Use the label |
|
|
||
| ### `view.use_legacy_sql` no longer has a default value of `True` | ||
|
|
||
| If `view.use_legacy_sql` is not specified in the configuration, no value is sent to the API. |
There was a problem hiding this comment.
Just to make sure I understand this- the API will only accept null (no value) or true, and will reject false. By removing the default here we're sending null when unset (or false, I think, w/ how we serialize empty values) and users can enable the setting with an explicit true.
A user who configured a resource without setting the field today had the default set to true; they'll get a diff from true-> what appears to be false in Terraform, but is actually null, prompting an update to set the API value to null.
(You haven't done anything to make the null and false thing a specific issue, to be clear! The values are collapsed into each other by several layers by default- in Terraform schema, in json serialization, etc. I'm being very explicit about where values are false/null because that's how they're displayed in user prompts/logs at those points)
There was a problem hiding this comment.
The API accepts three values: null, true, false, and treats null and false as separate values. I tested in TF before to confirm they are different since the input of not setting vs setting to false comes back from the API response as null vs false.
For a user without it in their config, after upgrading to this version, they will see true -> (none).
This change is required for an upcoming feature where this field can't be set, so TF users of that feature need to leave it out in their config, and also there can't be a TF default value when they leave it unset.
Hope this helps. Let me know if you have suggestions on how to make the release note more clear.
Context of the breaking change: #12390 (comment)
Documentation update in
main: #14574Release Note Template for Downstream PRs (will be copied)
See Write release notes for guidance.