-
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
fix: Use ALTER for managing PUBLIC schemas that exist #2973
Conversation
pkg/resources/schema.go
Outdated
if err != nil { | ||
return diag.FromErr(err) | ||
} | ||
create := true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH I would prefer an explicit separation here, i.e.:
- if name is not PUBLIC -> run create
- otherwise if does not exist -> run create
- otherwise -> run update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simplified a bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not a separation like I asked for. Now the execution path is still not clear.
Let's leave it like this, and will follow up and do it in the following PRs. I will keep it open for the time-being.
Integration tests failure for 8b3c88dd73d35bd98911d0adeb78cea3365bbf6a |
Integration tests failure for d95252b0701ca754f28c754dd4b641cc11e92f7f |
Integration tests failure for 30547cadada7df3f617caa04f50514be529b7c76 |
quotedIdentifiersIgnoreCase, | ||
enableConsoleOutput, | ||
err := GetAllDatabaseParameters(d) | ||
// there is no PUBLIC schema, so we continue with creating |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: or the name is completely different than PUBLIC, we will handle this in the next PR
Integration tests failure for a51e435066738ee38b06af2ea22da20a93b42a55 |
🤖 I have created a release *beep* *boop* --- ## [0.94.1](v0.94.0...v0.94.1) (2024-08-02) ### 🐛 **Bug fixes:** * Use ALTER for managing PUBLIC schemas that exist ([#2973](#2973)) ([567e9be](567e9be)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). Co-authored-by: snowflake-release-please[bot] <105954990+snowflake-release-please[bot]@users.noreply.github.com>
UpdateContextSchema
(note thatname
change needs to be excluded to avoid unnecessary rename)Test Plan
References
References #2826, specifically comment #2826 (comment)