feat: Rework data types #3244

sfc-gh-asawicki · 2024-12-03T12:40:15Z

Rework data types handling inside our SDK:

Extract in a separate package with two main entry points:
- func ParseDataType(raw string) (DataType, error) - responsible for proper datatype parsing and storing the attributes If any (falling to the hardcoded default for now - this is a continuation from fix: Fix issues 2972 and 3007 #3020 where diff suppressions for number and text types were set exactly this way; for V1 we should stay with hardcoded defaults and we can think of improving them later on without the direct influence on our users)
- func AreTheSame(a DataType, b DataType) bool - responsible for comparing any two data types; used mostly for diff suppressions and changes discovery
Replace all invocations of the old ToDataType in a compatible way (temporary function LegacyDataTypeFrom that underneath calls ToLegacyDataTypeSql() which returns the same output as old data types used two)
Test on the integration level the consistency of data types' behavior with Snowflake docs
Use new parser and comparator in validation and diff suppression functions (replacing the old ones in all resources)

Next steps/future improvements:

Use new DataType in SDK SQL builder
Replace the rest of the old DataType usages (especially in the SDK Requests/Opts)
Tackle SNOW-1596962 TODOs regarding VECTOR type
Improve parsing functions for lists of arguments (and defaults) - will be done on a case-by-case basis with all the resources
Clean up the code in new data types (TODOs with SNOW-1843440)

github-actions · 2024-12-03T14:31:31Z

Integration tests failure for ac1c372c83e16200debaca221640af3981c275a8

github-actions · 2024-12-03T16:17:43Z

Integration tests cancelled for 9c3087c458cf2397860da9b7a5d8163ab467a754

github-actions · 2024-12-03T18:46:12Z

Integration tests failure for e66a9367264a3c4087d174e15736e9b24d47362d

github-actions · 2024-12-04T00:46:46Z

Integration tests failure for 189a6f09b3a4b8c6851d40bc250b60c30e129087

pkg/sdk/testint/data_types_integration_test.go

pkg/sdk/datatypes/text.go

pkg/sdk/datatypes/vector.go

pkg/sdk/testint/data_types_integration_test.go

pkg/resources/function.go

pkg/resources/helpers.go

pkg/resources/procedure.go

pkg/sdk/data_types_deprecated.go

pkg/sdk/datatypes/data_types.go

sfc-gh-jmichalak · 2024-12-04T10:02:25Z

pkg/sdk/datatypes/binary.go

+	}
+	if !strings.HasPrefix(r, "(") || !strings.HasSuffix(r, ")") {
+		logging.DebugLogger.Printf(`binary %s could not be parsed, use "%s(size)" format`, raw.raw, raw.matchedByType)
+		return nil, fmt.Errorf(`binary %s could not be parsed, use "%s(size)" format`, raw.raw, raw.matchedByType)


We shouldn't log and return err at the same time. Same in other types.

Log is information to allow easier debugging, here it's hidden (you need env to allow it).
Error is returned and handled above.

pkg/sdk/datatypes/timestamp_tz.go

pkg/sdk/testint/data_types_integration_test.go

github-actions · 2024-12-04T15:23:08Z

Integration tests failure for 6a5ba55439a3c0790e1c23a4ac9f6f7327dfb3f6

github-actions · 2024-12-05T10:11:02Z

Integration tests cancelled for 9f76d27c8a5180985a5ee850ba3dea3917035c4f

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

sfc-gh-asawicki added 30 commits December 3, 2024 13:13

Rename files with old data type implementation

32a8918

Start with number data type (WIP)

e7fd119

Extract new data types to a separate package

1526fe3

Extract number data type to file

a6a559a

Prepare number parsing (WIP)

aee468b

Test new number parsing using old test cases

1e79a5d

Add more number test cases

8ba5559

Add float data type and improve number

a6fecbd

Add text data type

2cffacf

Add binary data type

b7589d2

Add boolean data type

aa7316d

Add date data type

7b2b672

Add time data type

d53abcc

Add timestamp ltz data type

75453a8

Add timestamp ntz data type

d94dd5c

Add timestamp tz data type

c4429f6

Add precision to all timestamp types

3fcc093

Add variant data type

e4bf46e

Add object data type

afd6e37

Add array data type

1d5f0cd

Add geography and geometry data types

b0fc671

Add vector data type

b1128b8

Add to sql and legacy to sql methods

9f3a968

Add TODOs

939d9b3

Add short description

add5256

Add data type integration tests (WIP)

fdd32ff

Add data type integration tests (WIP 2)

4fd0e7d

Add final data type integration tests

0139d48

Use new data type in a few places

63d5214

Use new data type in a few places more

da51c2a

Add migration guide

ac1c372

sfc-gh-asawicki requested review from sfc-gh-jcieslak and sfc-gh-jmichalak December 3, 2024 13:08

sfc-gh-asawicki marked this pull request as ready for review December 3, 2024 13:08

Revert legacy DataType in external table due to #2334

9c3087c

Don't use slices.Concat (we still use go 1.21)

e66a936

Fix integration tests

189a6f0

sfc-gh-jcieslak reviewed Dec 4, 2024

View reviewed changes

pkg/sdk/testint/data_types_integration_test.go Outdated Show resolved Hide resolved

pkg/sdk/datatypes/text.go Show resolved Hide resolved

pkg/sdk/datatypes/vector.go Show resolved Hide resolved

pkg/sdk/testint/data_types_integration_test.go Outdated Show resolved Hide resolved

Fix after review

e0a90e7

sfc-gh-jmichalak requested changes Dec 4, 2024

View reviewed changes

Fix after review

6a5ba55

sfc-gh-asawicki requested review from sfc-gh-jmichalak and sfc-gh-jcieslak December 4, 2024 12:59

sfc-gh-jcieslak previously approved these changes Dec 5, 2024

View reviewed changes

sfc-gh-jmichalak previously approved these changes Dec 5, 2024

View reviewed changes

Merge branch 'main' into rework-data-types

9f76d27

sfc-gh-asawicki dismissed stale reviews from sfc-gh-jmichalak and sfc-gh-jcieslak via 9f76d27 December 5, 2024 10:08

sfc-gh-jcieslak approved these changes Dec 5, 2024

View reviewed changes

sfc-gh-jmichalak approved these changes Dec 5, 2024

View reviewed changes

sfc-gh-asawicki merged commit 05ada91 into main Dec 5, 2024
8 of 9 checks passed

sfc-gh-asawicki deleted the rework-data-types branch December 5, 2024 10:13

snowflake-release-please bot mentioned this pull request Dec 5, 2024

chore(main): release 0.100.0 #3237

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Rework data types #3244

feat: Rework data types #3244

sfc-gh-asawicki commented Dec 3, 2024 •

edited

Loading

github-actions bot commented Dec 3, 2024

github-actions bot commented Dec 3, 2024

github-actions bot commented Dec 3, 2024

github-actions bot commented Dec 4, 2024

sfc-gh-jmichalak Dec 4, 2024

sfc-gh-asawicki Dec 4, 2024

sfc-gh-asawicki Dec 4, 2024

github-actions bot commented Dec 4, 2024

github-actions bot commented Dec 5, 2024

feat: Rework data types #3244

feat: Rework data types #3244

Conversation

sfc-gh-asawicki commented Dec 3, 2024 • edited Loading

github-actions bot commented Dec 3, 2024

github-actions bot commented Dec 3, 2024

github-actions bot commented Dec 3, 2024

github-actions bot commented Dec 4, 2024

sfc-gh-jmichalak Dec 4, 2024

Choose a reason for hiding this comment

sfc-gh-asawicki Dec 4, 2024

Choose a reason for hiding this comment

sfc-gh-asawicki Dec 4, 2024

Choose a reason for hiding this comment

github-actions bot commented Dec 4, 2024

github-actions bot commented Dec 5, 2024

sfc-gh-asawicki commented Dec 3, 2024 •

edited

Loading