Skip to content
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

feat: Upgrade view sdk #2969

Merged
merged 10 commits into from
Aug 6, 2024
Merged

feat: Upgrade view sdk #2969

merged 10 commits into from
Aug 6, 2024

Conversation

sfc-gh-jmichalak
Copy link
Collaborator

@sfc-gh-jmichalak sfc-gh-jmichalak commented Jul 30, 2024

  • add acceptance clients for aggregation and projection policies (with jiras to use proper SDK clients)
  • adjust SDK to match snowflake
  • change handling of PolicyReference to be more generic
  • add new sturct ExtendedIn to handle SHOWs that can specify applications and applications packages (will be useful in other resources)
  • add more validations
  • add quotes for fields with column names
  • update resource and datasource to be compatible with sdk
  • update statuses of remaining objects
  • NOTE: view behavior is not consistent with docs, that is:
    • adding columns is not working
    • columns with options in CREATE are without () in docs - this is not correct, they need to be wrapped in (), this will be reported on doc-discuss

Test Plan

  • integration tests
  • unit tests

References

https://docs.snowflake.com/en/sql-reference/sql/create-view#examples

Copy link

Integration tests failure for 3ea1e0d2ee0cabd0dcd261b348ab368ced3faa25

Copy link

Integration tests failure for fc9211fe0e9296c3c43f45f73deb8a1b14ea4970

pkg/acceptance/helpers/common.go Show resolved Hide resolved
pkg/acceptance/helpers/test_client.go Outdated Show resolved Hide resolved
pkg/sdk/testint/views_gen_integration_test.go Outdated Show resolved Hide resolved
pkg/sdk/views_def.go Show resolved Hide resolved
pkg/acceptance/helpers/row_access_policy_client.go Outdated Show resolved Hide resolved
pkg/acceptance/helpers/common.go Show resolved Hide resolved
pkg/sdk/common_types.go Show resolved Hide resolved
pkg/sdk/views_gen.go Show resolved Hide resolved
pkg/sdk/views_gen.go Show resolved Hide resolved
pkg/sdk/testint/views_gen_integration_test.go Show resolved Hide resolved
pkg/sdk/testint/views_gen_integration_test.go Outdated Show resolved Hide resolved
pkg/sdk/testint/views_gen_integration_test.go Show resolved Hide resolved
pkg/sdk/views_def.go Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Aug 1, 2024

Integration tests failure for 393368326234bc5faee826b37b426e5b3076fe19

@sfc-gh-asawicki sfc-gh-asawicki self-requested a review August 5, 2024 12:11
pkg/sdk/views_def.go Outdated Show resolved Hide resolved
@@ -84,10 +88,47 @@ type AlterViewOptions struct {
SetTagsOnColumn *ViewSetColumnTags `ddl:"keyword"`
UnsetTagsOnColumn *ViewUnsetColumnTags `ddl:"keyword"`
}
type DoubleQuotedString struct {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm, I don't like this abstraction, we shouldn't have content information inside the type

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you recommend, then? This field is case sensitive.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not a normal string?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we use unquoted strings, Snowflake makes them uppercase (similar to IDs), so we wouldn't be able to support all names here.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But what does the name have to do with the logic?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed this and the struct will be renamed to Column (or similarly) in the next PR. I will keep this thread open till then.

v1-preparations/ESSENTIAL_GA_OBJECTS.MD Outdated Show resolved Hide resolved
Copy link

github-actions bot commented Aug 5, 2024

Integration tests failure for 31e27b6905565aa598839459185d88ca9c154309

Copy link

github-actions bot commented Aug 5, 2024

Integration tests failure for 3d0d31eb6d62eca9347e8fc3064377c318eca007

Copy link

github-actions bot commented Aug 5, 2024

Integration tests failure for 0ba2e9281f9a0ad951005ec51d3ad7c860ec83ea

Copy link

github-actions bot commented Aug 5, 2024

Integration tests failure for 592ceac838143696dd3266fab0b618a67cde7d4b

sfc-gh-jcieslak
sfc-gh-jcieslak previously approved these changes Aug 6, 2024
SetTags()

var viewUnsetColumnTags = g.NewQueryStruct("ViewUnsetColumnTags").
// In the docs there is a MODIFY alternative, but for simplicity only one is supported here.
SQL("ALTER").
SQL("COLUMN").
Text("Name", g.KeywordOptions().Required()).
Text("Name", g.KeywordOptions().Required().DoubleQuotes()).
Copy link
Collaborator

@sfc-gh-jcieslak sfc-gh-jcieslak Aug 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering why that's a Text and not an Identifier.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be. If further changes to sdk are required, I'll change it.

@sfc-gh-asawicki sfc-gh-asawicki self-requested a review August 6, 2024 09:48
sfc-gh-asawicki
sfc-gh-asawicki previously approved these changes Aug 6, 2024
Copy link

github-actions bot commented Aug 6, 2024

Integration tests failure for e6f6c2f6f4b0ca6ea7e3ede9a6238c7eb0c2b4d1

@sfc-gh-asawicki sfc-gh-asawicki self-requested a review August 6, 2024 11:20
@sfc-gh-jmichalak sfc-gh-jmichalak merged commit ef2d50a into main Aug 6, 2024
8 of 9 checks passed
@sfc-gh-jmichalak sfc-gh-jmichalak deleted the views-v1 branch August 6, 2024 11:27
sfc-gh-jcieslak pushed a commit that referenced this pull request Sep 4, 2024
🤖 I have created a release *beep* *boop*
---


##
[0.95.0](v0.94.1...v0.95.0)
(2024-09-04)


### 🎉 **What's new:**

* Add change_tracking, row access policy and aggregation policy to views
([#2988](#2988))
([1f88bb1](1f88bb1))
* Add fully_qualified_name to all resources
([#2990](#2990))
([1b0462f](1b0462f))
* Add identifier parsers
([#2957](#2957))
([824ec52](824ec52))
* Add identifier with arguments
([#2979](#2979))
([00ae1c5](00ae1c5))
* Add timeouts block to cortex
([#3004](#3004))
([34d764b](34d764b))
* Add user parameters to resource
([#2968](#2968))
([f4ae380](f4ae380))
* Conclude user rework
([#3036](#3036))
([23e4625](23e4625))
* database role v1 readiness
([#3014](#3014))
([c4db255](c4db255))
* Identifier with arguments for procedure and external function
([#2987](#2987))
([f13cc5c](f13cc5c))
* Rework user resource
([#3026](#3026))
([bde2638](bde2638)),
closes
[#1572](#1572)
* Rework users datasource
([#3030](#3030))
([751239b](751239b)),
closes
[#2902](#2902)
* Upgrade view sdk
([#2969](#2969))
([ef2d50a](ef2d50a))
* View rework part 2
([#3021](#3021))
([e05377d](e05377d))
* View rework part 3
([#3023](#3023))
([195b41c](195b41c))


### 🔧 **Misc**

* Add annotation about fully_qualified_name and fix handling granteeName
([#3009](#3009))
([94e6345](94e6345))
* Apply identifier conventions
([#2996](#2996))
([5cbea84](5cbea84))
* apply identifier conventions to grants
([#3008](#3008))
([d7780ae](d7780ae))
* Clean collection utils
([#3028](#3028))
([426ddb1](426ddb1))
* Clean old assertions
([#3029](#3029))
([ad657eb](ad657eb))
* Conclude identifiers rework
([#3011](#3011))
([c1b53f3](c1b53f3))
* Improve user test and add manual test for user default database and
role
([#3035](#3035))
([6cb0b4e](6cb0b4e))
* Use new identifier with arguments in function, external function and
procedure grants
([#3002](#3002))
([5053f8b](5053f8b))
* User improvements
([#3034](#3034))
([65b64d7](65b64d7))


### 🐛 **Bug fixes:**

* database tests and introduce a new parameter
([#2981](#2981))
([3bae7f6](3bae7f6))
* Fix custom diffs for fields with diff supression
([#3032](#3032))
([2499602](2499602))
* Fix default secondary roles after BCR 2024_07
([#3040](#3040))
([2ca465a](2ca465a)),
closes
[#3038](#3038)
* Fix issues 2972 and 3007
([#3020](#3020))
([1772387](1772387))
* Fix known user resource issues
([#3013](#3013))
([a5dfeac](a5dfeac))
* identifier issues
([#2998](#2998))
([6fb76b7](6fb76b7))
* minor issues
([#3027](#3027))
([467b06e](467b06e)),
closes
[#3015](#3015)
[#2807](#2807)
[#3025](#3025)
* Nuke users
([#2971](#2971))
([0d90cc9](0d90cc9))

---
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants