SNOW-1000284: Add schema support for structure types #1323

sfc-gh-jrose · 2024-03-18T22:16:00Z

Please answer these questions before submitting your pull requests. Thanks!

What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.

Fixes #SNOW-1000284
Fill out the following pre-review checklist:
- I am adding a new automated test(s) to verify correctness of my new code
- I am adding new logging messages
- I am adding a new telemetry message
- I am adding new credentials
- I am adding a new dependency
Please describe how your code solves the related issue.

This PR adds relevant changes needed to support Structured Types. The goal of this commit is to enable reading structured data from iceberg tables. Writing new tables and support for non-iceberg tables is a future goal of the project.

The change does the following:

metadata that contains fields for relevant types (MAP, OBJECT, ARRAY) signals that structured types are enabled and being returned.
When receiving that metadata the relevant types are marked as structured types using a new new parameter. Users that would like to make new dataframes with structured types will similarly need to set the structured_types parameter on the schema objects. In the future we will enable this behavior by default.
When marked as structured the relevant types will generate slightly different queries for sprocs and udfs which allows them to specify data is structured for both input and output.

I've added test cases that test the following:

Returned data for iceberg and non-iceberg data can be handled with the new metadata. We don't expect non-iceberg structured data yet, but it couldn't hurt to be prepared for it.
Sprocs can take structured data as input and provide it as output. The same goes for udfs.
Subfields in structured objects/maps and indexes in structured arrays can be accessed with regular dataframe semantics.
We're able to return structured data from requests for pandas dataframes from iceberg tables.

Merge Notes:
This change cannot merge until the next python-connector version is released. I will need to add it as a dependency to support the new pandas changes.

This change also will not pass merge gates until I figure out how to add permissions to the test runner in order to access the external volume that we're using for iceberg tests. Maybe @sfc-gh-yixie can help me figure that out.

src/snowflake/snowpark/_internal/type_utils.py

src/snowflake/snowpark/_internal/udf_utils.py

sfc-gh-jdu · 2024-04-26T18:47:05Z

tests/integ/scala/test_datatype_suite.py

@@ -229,6 +294,170 @@ def test_dtypes(session):
    ]


+@pytest.mark.parametrize(


should we also skip these tests in stored proc? Does stored proc connector have the corresponding change for struct type?

By the time I merge this change it should have the corresponding change.

sfc-gh-jdu · 2024-04-26T18:49:16Z

tests/utils.py

@@ -76,6 +76,12 @@
 IS_NOT_ON_GITHUB = os.getenv("GITHUB_ACTIONS") != "true"
 # this env variable is set in regression test
 IS_IN_STORED_PROC_LOCALFS = IS_IN_STORED_PROC and os.getenv("IS_LOCAL_FS")
+STRUCTURED_TYPE_ENVIRONMENTS = {"dev", "aws"}
+IS_STRUCTURED_TYPES_SUPPORTED = (
+    os.getenv("cloud_provider", "dev") in STRUCTURED_TYPE_ENVIRONMENTS


cloud_provider is set on github, right? We also have some tests running on jenkins, so they will just be skipped on jenkins?

I'm not sure how that gets set. I know that in tox.ini it's passed in regardless of environment, but that doesn't mean it is set.

so we will skip these tests on jenkins right now? e.g., https://ci-dev-142.int.snowflakecomputing.com/job/SnowparkPythonClientRegressRunner/

Yes. Once the parameter for enabling structured types is on by default we will need to make sure these tests work in all environments, but for now sfctest0 is the only environment that has them enabled. It is also the only environment that we have iceberg test infrastructure set up.

got it, thanks. Then maybe create a jira and add a TODO here to make sure these conditions will be finally removed?

sfc-gh-jdu · 2024-04-26T18:49:51Z

tests/integ/scala/test_datatype_suite.py

+        ),
+        id="structured-types-enabled",
+    ),
+    False: pytest.param(


we finally will remove this example once all deployments supporting struct types?

Yes, at some point in the future we will make a BCR that turns structured types on by default. That will probably be the best time to remove this.

Co-authored-by: Jianzhun Du <[email protected]>

sfc-gh-yixie · 2024-04-30T20:16:54Z

tests/integ/scala/test_datatype_suite.py

+            [
+                StructField(
+                    "MAP",
+                    MapType(StringType(16777216), LongType(), structured=True),


Do we have to explicit use the length 16777216? This number may change soon.

Without a BCR we have to use the length for now. I expect that whenever I make the lob change I'll need to update this test and others that depend on this constant.

sfc-gh-jrose force-pushed the jrose_snow_1000284_structured_type_support branch 2 times, most recently from ff69f3f to d6f83cd Compare March 26, 2024 20:03

sfc-gh-jrose force-pushed the jrose_snow_1000284_structured_type_support branch from d1033e3 to 7b56a11 Compare April 25, 2024 22:08

SNOW-1000284: Add schema support for structure types

e2e444e

sfc-gh-jrose force-pushed the jrose_snow_1000284_structured_type_support branch from 7b56a11 to e2e444e Compare April 25, 2024 22:08

sfc-gh-jrose requested review from sfc-gh-jdu, sfc-gh-yixie and sfc-gh-aling April 26, 2024 00:05

sfc-gh-jrose marked this pull request as ready for review April 26, 2024 00:15

sfc-gh-jrose requested a review from a team as a code owner April 26, 2024 00:15

sfc-gh-jdu reviewed Apr 26, 2024

View reviewed changes

sfc-gh-jrose and others added 2 commits April 26, 2024 12:06

Update src/snowflake/snowpark/_internal/udf_utils.py

b3e78a6

Co-authored-by: Jianzhun Du <[email protected]>

Review Feedback

c8ee62f

sfc-gh-jdu approved these changes Apr 26, 2024

View reviewed changes

sfc-gh-jrose added 2 commits April 26, 2024 16:45

Add todo comment.

7c9c388

Update connctor dependency

4f9eb04

sfc-gh-jrose requested a review from a team as a code owner April 29, 2024 23:14

sfc-gh-jrose requested review from sfc-gh-nkumar and sfc-gh-vbudati April 29, 2024 23:14

sfc-gh-jrose added 2 commits April 30, 2024 11:23

update tests

e26e7a5

another test update

0e8a71e

sfc-gh-yixie approved these changes Apr 30, 2024

View reviewed changes

sfc-gh-yixie reviewed Apr 30, 2024

View reviewed changes

sfc-gh-jrose merged commit 9ccac9e into main Apr 30, 2024
26 checks passed

sfc-gh-jrose deleted the jrose_snow_1000284_structured_type_support branch April 30, 2024 21:52

github-actions bot locked and limited conversation to collaborators Apr 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SNOW-1000284: Add schema support for structure types #1323

SNOW-1000284: Add schema support for structure types #1323

sfc-gh-jrose commented Mar 18, 2024 •

edited

Loading

sfc-gh-jdu Apr 26, 2024

sfc-gh-jrose Apr 26, 2024

sfc-gh-jdu Apr 26, 2024

sfc-gh-jrose Apr 26, 2024

sfc-gh-jdu Apr 26, 2024

sfc-gh-jrose Apr 26, 2024

sfc-gh-jdu Apr 26, 2024

sfc-gh-jdu Apr 26, 2024

sfc-gh-jrose Apr 26, 2024

sfc-gh-yixie Apr 30, 2024

sfc-gh-jrose Apr 30, 2024

		@@ -229,6 +294,170 @@ def test_dtypes(session):
		]


		@pytest.mark.parametrize(

SNOW-1000284: Add schema support for structure types #1323

SNOW-1000284: Add schema support for structure types #1323

Conversation

sfc-gh-jrose commented Mar 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sfc-gh-jrose commented Mar 18, 2024 •

edited

Loading