-
Notifications
You must be signed in to change notification settings - Fork 427
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Bug]: column block-list is changing in every plan (upgrade to v0.95.0 not possible #3073
Comments
Hey @JESCHO99. Thanks for reaching out to us. View resource is a newly reworked resource, so we will analyze this with a high priority. cc: @sfc-gh-jmichalak |
Hi @JESCHO99 👋, As a workaround, please specify columns in the config, like:
|
Hey @sfc-gh-jmichalak, Best regards, |
I think we can release the fix ~after Wednesday next week. |
<!-- Feel free to delete comments as you fill this in --> - fix permadiff when columns are not specified - adjust row access policy signature in describe_output and move parsing to sdk - adjust the identifier doc - add procedure id <!-- summary of changes --> ## Test Plan <!-- detail ways in which this PR has been tested or needs to be tested --> * [x] acceptance tests <!-- add more below if you think they are relevant --> * [ ] … ## References <!-- issues documentation links, etc --> #3073 #2994
Hitting the same issue, also appreciate the fast fix. Trying to stay current as the provider nears 1.0 🤞 Thanks! |
Also encountering this issue - any update on the release date for the fix? @sfc-gh-jmichalak |
This will be released today or tomorrow. |
<!-- Feel free to delete comments as you fill this in --> - fix permadiff when columns are not specified - adjust row access policy signature in describe_output and move parsing to sdk - adjust the identifier doc - add procedure id <!-- summary of changes --> ## Test Plan <!-- detail ways in which this PR has been tested or needs to be tested --> * [x] acceptance tests <!-- add more below if you think they are relevant --> * [ ] … ## References <!-- issues documentation links, etc --> #3073 #2994
Hi all, we've released v0.96.0, which includes the fix and acceptance test for the column list in views. Please upgrade with the migration guide. |
Thanks for all your work on this, much appreciated. v0.96.0 fixes the issue for me when no resource "snowflake_view" "example" {
name = "EXAMPLE"
database = snowflake_schema.this.database
schema = snowflake_schema.this.name
comment = snowflake_table.event_v0.comment
## The column identifiers in the SELECT list are unquoted
statement = <<-SQL
SELECT ${join(", ", [for c in local.view_columns_v0 : c.name])}
FROM ${snowflake_table.source.name}
SQL
dynamic "column" {
for_each = local.view_columns_v0
content {
column_name = column.value.name
comment = column.value.comment
}
}
} ... but on subsequent runs, I'm seeing same plan loop as before, with the perma-diff being on the This seems to only be an issue when explicitly specifying the columns in the SELECT in This feels like the same issue with the v0.96.0 release containing a partial fix for the plan loop under some circumstances, but can file a new issue if not (or maybe I'm missing something obvious?) |
Hi @gws 👋 I tried to reproduce this, but I have no permadiff. Could you also provide the referenced table definition, the output of |
@sfc-gh-jmichalak Thanks, this is a weirder one than I thought initially. I think I can do you one better and get you something that reproduces the issue consistently for me on v0.96.0, I think the issue is related to Not as minimal as it could be but should be self-contained enough to repro: locals {
db_name = "TEST"
schema_name = "GH3073"
columns = [
{
comment = "COL1 comment"
name = "COL1"
nullable = false
type = "VARCHAR"
},
{
## This appears to be the source of the issue
comment = "COL2 comment has issues when the following (parentheses) appear."
name = "COL2"
nullable = false
type = "VARCHAR"
},
]
view_columns = [for c in local.columns : c if c.name != "COL1"]
}
resource "snowflake_schema" "this" {
database = local.db_name
name = local.schema_name
}
resource "snowflake_table" "test" {
name = "T_TEST"
database = snowflake_schema.this.database
schema = snowflake_schema.this.name
dynamic "column" {
for_each = local.columns
content {
comment = column.value.comment
name = column.value.name
nullable = column.value.nullable
type = column.value.type
}
}
}
resource "snowflake_view" "test" {
name = "V_TEST"
database = snowflake_schema.this.database
schema = snowflake_schema.this.name
statement = <<-SQL
SELECT ${join(", ", [for c in local.view_columns : c.name])}
FROM ${snowflake_table.test.name}
SQL
dynamic "column" {
for_each = local.view_columns
content {
column_name = column.value.name
comment = column.value.comment
}
}
} Terraform plan output:
If you still can't reproduce with the above I'll get the logs over to you as well but I suspect we won't need them and they're pretty verbose. |
Thanks! We need to parse manually the |
Please make sure you have a matching column list with the columns returned from the statement. I encountered many errors like this during testing, and usually, the problem was bad configuration. If it doesn't work, please provide the whole resource configuration. |
Hi @sfc-gh-jmichalak, we aren't setting the We have a hundreds of views that use a module that makes it so people w/ little to no terraform experience can easily manage views by just managing one .SQL file. The module takes sql files in a directory like
|
Hi, I just tested your configuration with a simple
When the column definition is empty in the config (as in your case), the column list in |
@sfc-gh-jmichalak Thanks, will do if it comes up again! |
…IdentifierWithArguments (#3102) <!-- Feel free to delete comments as you fill this in --> <!-- summary of changes --> ## Test Plan <!-- detail ways in which this PR has been tested or needs to be tested --> * [x] unit tests <!-- add more below if you think they are relevant --> ## References <!-- issues documentation links, etc --> #3073 (comment) ## Summary fix: add a check for `(` in ParseSchemaObjectIdentifierWithArguments; add a unit test fix: improve view parser; add unit tests docs: add a note to docs that we discourage using special characters in views docs: add granting PUBLIC role to common issues docs: add a recommendation about upgrading versions one by one docs: adjust essential objects table
Hey @sfc-gh-jmichalak this just happened again. Terraform changed the actual definition but not the column_list, which is a very confusing experience for the end users. Here's the TF Plan:
Here's the Query generated by Terraform:
Heres' the current definition after the TF apply.
|
Thanks for the detailed logs. This is indeed an error on our side (both of the cases). It happens because the old column list is still populated during CREATE OR REPLACE, which gets called when the statement is changed. This causes incorrect column definition in this SQL statement. This column list should be empty because it's not specified in the config. The workaround here is to manually define the Currently, I'm working on fixing this behavior. |
@sfc-gh-jmichalak Awesome, thanks for the confirmation! Do you think the fix will be part of 0.97? |
<!-- Feel free to delete comments as you fill this in --> - add snowflake_stream_on_external_table resource - adjust copy grants documentation - adjust external table helper client - move copy grants handling to `resource_helpers_create.go` - move baseModel to `ext` - move common stream code to `stream_common.go` - fix using empty columns in views recreation ## Test Plan <!-- detail ways in which this PR has been tested or needs to be tested --> * [x] acceptance tests <!-- add more below if you think they are relevant --> ## References <!-- issues documentation links, etc --> https://docs.snowflake.com/en/sql-reference/sql/create-stream #3073 ## TODO - add remaining resources (stage, view) - rework data source
Hey, @liamjamesfoley we have just released v0.97.0 version of the provider (release notes, migration guide) containing the fix for this behavior. |
Confirmed this fixed it for us - thank you! |
Fixed for us as well - we appreciate the fast turn-around! |
Thanks @sfc-gh-jmichalak ! |
Thanks for the quick feedback! As the fix was confirmed by a few users, I'm closing this issue. If this happens again, please create a new one. |
Terraform CLI Version
1.9.1
Terraform Provider Version
v0.95.0
Terraform Configuration
Category
category:resource
Object type(s)
data_source:views
Expected Behavior
The following terraform configurations should be able to execute a terraform apply without any error and after a successful apply of this code there should not be any open changes left when a new terraform plan is executed after the successful apply
Actual Behavior
After a successful apply when a terraform plan is executed there is always a change for the view object for the column list. Even is the column list is not touched in the code this behaviour leads to an update/replacement of all views in every terraform plan executed.
This is the change/update which shows up after every successful apply in the next terraform plan when using this terraform version and view resources.
Steps to Reproduce
How much impact is this issue causing?
High
Logs
No response
Additional Information
We are not able to upgrade to version 0.95.0 because of this issue right now and for us it would be important because we need the hotfix for table data types which is implemented in the 0.95.0 release
Would you like to implement a fix?
The text was updated successfully, but these errors were encountered: