[SPARK-41412][CONNECT] Implement `Column.cast` #38970

amaliujia · 2022-12-07T20:58:44Z

What changes were proposed in this pull request?

Add types.py and move to/from type proto to this file.
Improve Cast proto to support both DataType and DataType as a string.
support cast to the following types in Python:
ByteType,
ShortType,
IntegerType,
FloatType,
DayTimeIntervalType,
MapType,
StringType,
DoubleType,
LongType,
DecimalType,
BinaryType,
BooleanType

Why are the changes needed?

API coverage

Does this PR introduce any user-facing change?

NO

How was this patch tested?

UT

amaliujia · 2022-12-07T20:59:41Z

cc @zhengruifeng

amaliujia · 2022-12-07T21:09:40Z

connector/connect/common/src/main/protobuf/spark/connect/expressions.proto

+    oneof cast_to_type {
+      DataType type = 2;
+      // If this is set, Server will use Catalyst parser to parse this string to DataType.
+      string type_str = 3;


I add this to follow the design principle so far that we should move repeated clients implementation to the server side to reduce client side redundant work.

Otherwise client side will need to implement string to DataType conversion and each client will need to the same thing.

This comes back to the discussion that I had in my draft PR if the second argument to the cast() function should be modeled as an expression or not. Right now it takes a value but could be modeled as a string as well.

grundprinzip · 2022-12-07T23:45:49Z

connector/connect/common/src/main/protobuf/spark/connect/expressions.proto


    // (Required) the data type that the expr to be casted to.
-    DataType cast_to_type = 2;
+    oneof cast_to_type {


FYI: this is a breaking change in the proto message. Ideally, we would use

reserved 2; oneof cast_to_type { DataType type = 3; string type_str = 4; }

Ah thanks. I didn't know this way to evolve the proto.

Given Spark Connect is still alpha component though for now we don't need to be enforced to maintain the backwards compatibility.

But when we are ready to leave from the alpha component then we should follow this way.

I think it's fine since this message has not been actually used?

I think we are ok now. But later there is indeed a need to build a process/good practice etc. for how to evolve proto without breaking older versions (if possible)

We're ok now. We can use Buf to check for breaking changes.

grundprinzip · 2022-12-07T23:46:36Z

connector/connect/common/src/main/protobuf/spark/connect/expressions.proto

+    oneof cast_to_type {
+      DataType type = 2;
+      // If this is set, Server will use Catalyst parser to parse this string to DataType.
+      string type_str = 3;


This comes back to the discussion that I had in my draft PR if the second argument to the cast() function should be modeled as an expression or not. Right now it takes a value but could be modeled as a string as well.

grundprinzip · 2022-12-07T23:50:29Z

...connect/server/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala

+        Cast(
+          transformExpression(cast.getExpr),
+          session.sessionState.sqlParser.parseDataType(cast.getTypeStr))


Wouldn't it be nice if we could just add a second string argument to the Cast expression so it could resolved the expression automatically? Would this make the design easier? Because then you wouldn't even need a custom expression for cast.

I am not following this? Can you give some example proto/sample code?

Are you saying we do not use oneof but add the third string field?

When I looked at cast as a unicorn in the context of expressions, it's one of the few where not all argument types resolve to expressions. I was wondering if we could simplify the approach to make the type argument of cast an expression that can resolve as a string then we can do the matching of the expression in the analyzer.

This is very similar to why you added the oneof to the proto message.

grundprinzip · 2022-12-08T08:50:17Z

My discussion was not meant to block the PR, this was more a more general observation.

LGTM, thanks!

cc @HyukjinKwon @zhengruifeng

zhengruifeng · 2022-12-08T08:57:27Z

merged into master

…e on ### What changes were proposed in this pull request? This PR is a followup of #38970 which makes the test pass with ANSI mode on. ### Why are the changes needed? To recover the build with ANSI mode on. Currently it's broke as follows: ``` ====================================================================== ERROR [2.651s]: test_cast (pyspark.sql.tests.connect.test_connect_column.SparkConnectTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/__w/spark/spark/python/pyspark/sql/tests/connect/test_connect_column.py", line 119, in test_cast df.select(df.id.cast(x)).toPandas(), df2.select(df2.id.cast(x)).toPandas() File "/__w/spark/spark/python/pyspark/sql/connect/dataframe.py", line 1466, in toPandas return self._session.client._to_pandas(query) File "/__w/spark/spark/python/pyspark/sql/connect/client.py", line 333, in _to_pandas return self._execute_and_fetch(req) File "/__w/spark/spark/python/pyspark/sql/connect/client.py", line 418, in _execute_and_fetch for b in self._stub.ExecutePlan(req, metadata=self._builder.metadata()): File "/usr/local/lib/python3.9/dist-packages/grpc/_channel.py", line 426, in __next__ return self._next() File "/usr/local/lib/python3.9/dist-packages/grpc/_channel.py", line 826, in _next raise self grpc._channel._MultiThreadedRendezvous: <_MultiThreadedRendezvous of RPC that terminated with: status = StatusCode.UNKNOWN details = "[DATATYPE_MISMATCH.CAST_WITH_CONF_SUGGESTION] Cannot resolve "id" due to data type mismatch: cannot cast "BIGINT" to "BINARY" with ANSI mode on. If you have to cast "BIGINT" to "BINARY", you can set "spark.sql.ansi.enabled" as 'false'.; 'Project [unresolvedalias(cast(id#31L as binary), None)] +- SubqueryAlias spark_catalog.default.test_connect_basic_table_1 +- Relation spark_catalog.default.test_connect_basic_table_1[id#31L,name#32] parquet " debug_error_string = "UNKNOWN:Error received from peer ipv4:127.0.0.1:15002 {created_time:"2022-12-09T01:54:45.378316841+00:00", grpc_status:2, grpc_message:"[DATATYPE_MISMATCH.CAST_WITH_CONF_SUGGESTION] Cannot resolve \"id\" due to data type mismatch: cannot cast \"BIGINT\" to \"BINARY\" with ANSI mode on.\nIf you have to cast \"BIGINT\" to \"BINARY\", you can set \"spark.sql.ansi.enabled\" as \'false\'.;\n\'Project [unresolvedalias(cast(id#31L as binary), None)]\n+- SubqueryAlias spark_catalog.default.test_connect_basic_table_1\n +- Relation spark_catalog.default.test_connect_basic_table_1[id#31L,name#32] parquet\n"}" > ``` https://github.com/apache/spark/actions/runs/3671813752 ### Does this PR introduce _any_ user-facing change? No, test-only. ### How was this patch tested? This PR fixes the unittest to make passed. I manually tested. Closes #39034 from HyukjinKwon/SPARK-41412-followup. Authored-by: Hyukjin Kwon <[email protected]> Signed-off-by: Hyukjin Kwon <[email protected]>

### What changes were proposed in this pull request? 1. Add `types.py` and move to/from type proto to this file. 2. Improve Cast proto to support both DataType and DataType as a string. 3. support cast to the following types in Python: ByteType, ShortType, IntegerType, FloatType, DayTimeIntervalType, MapType, StringType, DoubleType, LongType, DecimalType, BinaryType, BooleanType ### Why are the changes needed? API coverage ### Does this PR introduce _any_ user-facing change? NO ### How was this patch tested? UT Closes apache#38970 from amaliujia/add_cast. Authored-by: Rui Wang <[email protected]> Signed-off-by: Ruifeng Zheng <[email protected]>

…e on ### What changes were proposed in this pull request? This PR is a followup of apache#38970 which makes the test pass with ANSI mode on. ### Why are the changes needed? To recover the build with ANSI mode on. Currently it's broke as follows: ``` ====================================================================== ERROR [2.651s]: test_cast (pyspark.sql.tests.connect.test_connect_column.SparkConnectTests) ---------------------------------------------------------------------- Traceback (most recent call last): File "/__w/spark/spark/python/pyspark/sql/tests/connect/test_connect_column.py", line 119, in test_cast df.select(df.id.cast(x)).toPandas(), df2.select(df2.id.cast(x)).toPandas() File "/__w/spark/spark/python/pyspark/sql/connect/dataframe.py", line 1466, in toPandas return self._session.client._to_pandas(query) File "/__w/spark/spark/python/pyspark/sql/connect/client.py", line 333, in _to_pandas return self._execute_and_fetch(req) File "/__w/spark/spark/python/pyspark/sql/connect/client.py", line 418, in _execute_and_fetch for b in self._stub.ExecutePlan(req, metadata=self._builder.metadata()): File "/usr/local/lib/python3.9/dist-packages/grpc/_channel.py", line 426, in __next__ return self._next() File "/usr/local/lib/python3.9/dist-packages/grpc/_channel.py", line 826, in _next raise self grpc._channel._MultiThreadedRendezvous: <_MultiThreadedRendezvous of RPC that terminated with: status = StatusCode.UNKNOWN details = "[DATATYPE_MISMATCH.CAST_WITH_CONF_SUGGESTION] Cannot resolve "id" due to data type mismatch: cannot cast "BIGINT" to "BINARY" with ANSI mode on. If you have to cast "BIGINT" to "BINARY", you can set "spark.sql.ansi.enabled" as 'false'.; 'Project [unresolvedalias(cast(id#31L as binary), None)] +- SubqueryAlias spark_catalog.default.test_connect_basic_table_1 +- Relation spark_catalog.default.test_connect_basic_table_1[id#31L,name#32] parquet " debug_error_string = "UNKNOWN:Error received from peer ipv4:127.0.0.1:15002 {created_time:"2022-12-09T01:54:45.378316841+00:00", grpc_status:2, grpc_message:"[DATATYPE_MISMATCH.CAST_WITH_CONF_SUGGESTION] Cannot resolve \"id\" due to data type mismatch: cannot cast \"BIGINT\" to \"BINARY\" with ANSI mode on.\nIf you have to cast \"BIGINT\" to \"BINARY\", you can set \"spark.sql.ansi.enabled\" as \'false\'.;\n\'Project [unresolvedalias(cast(id#31L as binary), None)]\n+- SubqueryAlias spark_catalog.default.test_connect_basic_table_1\n +- Relation spark_catalog.default.test_connect_basic_table_1[id#31L,name#32] parquet\n"}" > ``` https://github.com/apache/spark/actions/runs/3671813752 ### Does this PR introduce _any_ user-facing change? No, test-only. ### How was this patch tested? This PR fixes the unittest to make passed. I manually tested. Closes apache#39034 from HyukjinKwon/SPARK-41412-followup. Authored-by: Hyukjin Kwon <[email protected]> Signed-off-by: Hyukjin Kwon <[email protected]>

github-actions bot added CONNECT CORE PYTHON SQL labels Dec 7, 2022

amaliujia added 3 commits December 7, 2022 12:59

[SPARK-41412][CONNECT] Implement Cast

e271484

update

7b6ef37

update

0fda215

amaliujia force-pushed the add_cast branch from 5d98968 to 0fda215 Compare December 7, 2022 20:59

amaliujia commented Dec 7, 2022

View reviewed changes

grundprinzip reviewed Dec 7, 2022

View reviewed changes

grundprinzip approved these changes Dec 8, 2022

View reviewed changes

zhengruifeng closed this in 5c8949c Dec 8, 2022

HyukjinKwon mentioned this pull request Dec 12, 2022

[SPARK-41412][CONNECT][FOLLOW-UP] Fix test_cast to pass with ANSI mode on #39034

Closed

[SPARK-41412][CONNECT] Implement Column.cast #38970

[SPARK-41412][CONNECT] Implement Column.cast #38970

Uh oh!

Conversation

amaliujia commented Dec 7, 2022

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

amaliujia commented Dec 7, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amaliujia Dec 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

amaliujia Dec 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

grundprinzip commented Dec 8, 2022

Uh oh!

zhengruifeng commented Dec 8, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[SPARK-41412][CONNECT] Implement `Column.cast` #38970

[SPARK-41412][CONNECT] Implement `Column.cast` #38970

amaliujia Dec 8, 2022 •

edited

Loading

amaliujia Dec 8, 2022 •

edited

Loading