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.

Sign up for GitHub

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

Jump to bottom

feat: Use new data types in sql builder for functions and procedures #3247

Merged
merged 29 commits into from
Dec 6, 2024
Merged

feat: Use new data types in sql builder for functions and procedures #3247

merged 29 commits into from
Dec 6, 2024

Conversation

sfc-gh-asawicki
Copy link
Collaborator

@sfc-gh-asawicki sfc-gh-asawicki commented Dec 4, 2024 •
edited
Loading

Use new data types directly in Opts structs:

  • rename old data type fields and make them optional for functions and procedures
  • add new data type fields for functions and procedures
  • adjust the resources
  • adjust unit, integration, and acceptance tests
  • add integration tests verifying in detail the behavior of data types for functions and procedures
  • adjust and test SQL builder
  • add Canonical method to new data types (useful because of how Snowflake returns data types)

Next PRs:

  • adjust function/procedure SDK
  • handle Table return format as data type?
  • resources/datasources (these will be a few PRs)

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 4, 2024

Integration tests failure for 23322eb707e41511c35d429da47b80272f447ecb

Base automatically changed from rework-data-types to main December 5, 2024 10:13
@sfc-gh-asawicki
use data types in sql starts here
01c97c8
@sfc-gh-asawicki
Add special handling for DataTYpe interface in sql builder (PoC)
8301db3
@sfc-gh-asawicki
Add TODOs
f157cfb
@sfc-gh-asawicki
Test all data types in sql builder
d29bb72
@sfc-gh-asawicki
Alter functions def (old data type)
bb5e671
@sfc-gh-asawicki
Regenerate
9a50470
@sfc-gh-asawicki
Update function def (old data type optional, new - required)
33982f4
@sfc-gh-asawicki
Regenerate function with new data type (WIP)
d7d6ebc
@sfc-gh-asawicki
Pass all function integration tests
c215f98
@sfc-gh-asawicki
Add TODO
1605e9d
@sfc-gh-asawicki
Test all nil data types in SQL builder
ba0a3dc
@sfc-gh-asawicki
Update function creation unit tests
c7471b2
@sfc-gh-asawicki
Pass functions integration test with different data types
aa02897
@sfc-gh-asawicki
Test canonical method on all data types
1b2615e
@sfc-gh-asawicki
Regenerate procedures with renaming old data types
5dc629d
@sfc-gh-asawicki
Adjust tests
27b75b2
@sfc-gh-asawicki
Add new data type to procedure definition
0339f5e
@sfc-gh-asawicki
Regenerate procedures with new data types
8b440c6
@sfc-gh-asawicki
Add unit tests for new data types in procedures
cef1eef
@sfc-gh-asawicki
Update unit tests for procedures
da13976
@sfc-gh-asawicki
Make procedure integration tests pass
11175b2
@sfc-gh-asawicki
Add data types behavior for procedures
ba97b9c
@sfc-gh-asawicki
Add issue numbers to TODOs
30ecd5d
@sfc-gh-asawicki
Run make pre-push
03e2400
@sfc-gh-asawicki
Bump integration tests timeout
1ff1108
@sfc-gh-asawicki sfc-gh-asawicki force-pushed the use-data-types-in-sql-builder branch from 23322eb to 1ff1108 Compare December 5, 2024 10:17
@sfc-gh-asawicki sfc-gh-asawicki marked this pull request as ready for review December 5, 2024 10:23
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 5, 2024

Integration tests failure for 1ff110818283d3668cee0a5a613e507d4e4017d4

sfc-gh-jmichalak
sfc-gh-jmichalak reviewed Dec 5, 2024
View reviewed changes
pkg/sdk/datatypes/array.go Show resolved Hide resolved
pkg/sdk/procedures_def.go Outdated Show resolved Hide resolved
pkg/sdk/procedures_def.go Outdated Show resolved Hide resolved
pkg/sdk/procedures_ext.go Show resolved Hide resolved
pkg/sdk/functions_def.go Show resolved Hide resolved
pkg/sdk/sql_builder_test.go Show resolved Hide resolved
pkg/sdk/sql_builder_test.go Show resolved Hide resolved
pkg/acceptance/helpers/function_client.go Show resolved Hide resolved
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 5, 2024

Integration tests failure for 15c842aac8791a1049baad91c15dce8ce03b0d39

@sfc-gh-jmichalak sfc-gh-jmichalak mentioned this pull request Dec 5, 2024
Merged
2 tasks
sfc-gh-jcieslak
sfc-gh-jcieslak approved these changes Dec 6, 2024
View reviewed changes
pkg/sdk/random_test.go
@@ -14,6 +15,12 @@ var (
emptyDatabaseObjectIdentifier = NewDatabaseObjectIdentifier("", "")
emptySchemaObjectIdentifier = NewSchemaObjectIdentifier("", "", "")
emptySchemaObjectIdentifierWithArguments = NewSchemaObjectIdentifierWithArguments("", "", "")

// TODO [SNOW-1843440]: create using constructors (when we add them)?
dataTypeNumber, _ = datatypes.ParseDataType("NUMBER(36, 2)")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe they should be created in func init() to ensure they don't throw the error?

Copy link
Collaborator Author

@sfc-gh-asawicki sfc-gh-asawicki Dec 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I will add this as an alternative to constructors

@sfc-gh-asawicki sfc-gh-asawicki merged commit 69f677a into main Dec 6, 2024
8 of 9 checks passed
@sfc-gh-asawicki sfc-gh-asawicki deleted the use-data-types-in-sql-builder branch December 6, 2024 10:03
@snowflake-release-please snowflake-release-please bot mentioned this pull request Dec 6, 2024
Merged
sfc-gh-jmichalak added a commit that referenced this pull request Dec 10, 2024
@sfc-gh-jmichalak
fix: Minor fixes 2 (#3230)
73b7e74 
<!-- Feel free to delete comments as you fill this in -->
- prove that using network policy with lowercase characters in oauth
integration fails in Snowflake
- update the migration guide regarding migrating provider configuration
- rename `datasource` to `data source`
- add missing examples and fix some examples in the docs
- add notes about missing fields (they will be done in SNOW-1844996)
- adjust documentation for provider configuration
- improve quoting with import and resource configuration examples
- some examples of replication and grants were not changed because they
show a nicer setup, IMO - can discuss about this
- add info about external changes not being detected for certain fields
or resource types
- change infobox format for some notes (`[!WARNING]` -> `!> Note`)
because it isn't rendered properly in the registry
- fix empty `using` in view masking policies
- add links to documentation of the referenced resources
- adjust enum validations to use `sdkValidation` with SDK converter
function instead of `StringInSlice`
- address
#3247
by improving the documentation
- update lists in `v1-preparations` files
<!-- summary of changes -->

## Test Plan
<!-- detail ways in which this PR has been tested or needs to be tested
-->
* [ ] acceptance tests
<!-- add more below if you think they are relevant -->
* [x] integration tests

## References

#3198

#3251
## TODO (next PRs):
- check the migration guide once again
sfc-gh-jcieslak pushed a commit that referenced this pull request Dec 12, 2024
@snowflake-release-please
chore(main): release 0.100.0 (#3237)
5139b1a 
🤖 I have created a release *beep* *boop*
---


##
[0.100.0](v0.99.0...v0.100.0)
(2024-12-12)


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

* Account v1 readiness
([#3236](#3236))
([5df33a8](5df33a8))
* Account v1 readiness generated files
([#3242](#3242))
([3df59dd](3df59dd))
* Account v1 readiness resource
([#3252](#3252))
([8f5698d](8f5698d))
* Add a new account roles data source
([#3257](#3257))
([b3d6b9e](b3d6b9e))
* Add account data source
([#3261](#3261))
([6087fc9](6087fc9))
* Add all other functions and procedures implementations
([#3275](#3275))
([7a6f68d](7a6f68d))
* Basic functions implementation
([#3269](#3269))
([6d4a103](6d4a103))
* Basic procedures implementation
([#3271](#3271))
([933335f](933335f))
* Docs, test, and missing parameter
([#3280](#3280))
([10517f3](10517f3))
* Functions and procedures schemas and generated sources
([#3262](#3262))
([9b70f87](9b70f87))
* Functions sdk update
([#3254](#3254))
([fc1eace](fc1eace))
* Handle missing fields in function and procedure
([#3273](#3273))
([53e7a0a](53e7a0a))
* Procedures schemas and generated sources
([#3263](#3263))
([211ad46](211ad46))
* Procedures sdk update
([#3255](#3255))
([682606a](682606a))
* Rework account parameter resource
([#3264](#3264))
([15aa9c2](15aa9c2))
* Rework data types
([#3244](#3244))
([05ada91](05ada91))
* support table data type
([#3274](#3274))
([13401d5](13401d5))
* Tag association v1 readiness
([#3210](#3210))
([04f6d54](04f6d54))
* Test imports and small fixes
([#3276](#3276))
([a712195](a712195))
* Unsafe execute v1 readiness
([#3266](#3266))
([c4f1e8f](c4f1e8f))
* Use new data types in sql builder for functions and procedures
([#3247](#3247))
([69f677a](69f677a))


### 🔧 **Misc**

* Add ShowByID filtering generation
([#3227](#3227))
([548ec42](548ec42))
* Adress small task-related todos
([#3243](#3243))
([40de9ae](40de9ae))
* Apply masking
([#3234](#3234))
([c209a8a](c209a8a))
* fix missing references in toOpts and changes with newlines
([#3240](#3240))
([246547f](246547f))
* function tests
([#3279](#3279))
([5af6efb](5af6efb))
* Improve config builders
([#3207](#3207))
([425787c](425787c))
* Revert to proper env
([#3238](#3238))
([5d4ed3b](5d4ed3b))
* Use service user for ci
([#3228](#3228))
([2fb50d7](2fb50d7))


### 🐛 **Bug fixes:**

* Make blocked_roles_field optional in OAuth security integrations
([#3267](#3267))
([7197b57](7197b57))
* Minor fixes
([#3231](#3231))
([1863bf6](1863bf6))
* Minor fixes 2
([#3230](#3230))
([73b7e74](73b7e74))

---
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
Reviewers

@sfc-gh-jmichalak sfc-gh-jmichalak sfc-gh-jmichalak approved these changes

@sfc-gh-jcieslak sfc-gh-jcieslak sfc-gh-jcieslak approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sfc-gh-asawicki @sfc-gh-jcieslak @sfc-gh-jmichalak