You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "hvanhovell (via GitHub)" <gi...@apache.org> on 2025/12/10 19:40:37 UTC
[PR] [SPARK-54670][CONNECT] Rework Connect Literal handling [spark]
hvanhovell opened a new pull request, #53430:
URL: https://github.com/apache/spark/pull/53430
### Why are the changes needed?
This PR fixes a number of problems with literal handling in Spark Connect:
- This removes the dataType field in literal, this is an unshipped change. The problem with that particular change is that it is prohibitively easy to create an inconsitent state (e.g. an int can be a binary now...), and it causes duplication.
- ... TBD...
- ...TBD...
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Modified existing tests.
### Was this patch authored or co-authored using generative AI tooling?
No.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54670][CONNECT] Rework Connect Literal handling [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #53430:
URL: https://github.com/apache/spark/pull/53430#discussion_r2611862708
##########
sql/connect/server/src/main/scala/org/apache/spark/sql/connect/planner/LiteralExpressionProtoConverter.scala:
##########
@@ -31,9 +39,95 @@ object LiteralExpressionProtoConverter {
* Expression
*/
def toCatalystExpression(lit: proto.Expression.Literal): expressions.Literal = {
- val dataType = LiteralValueProtoConverter.getDataType(lit)
- val scalaValue = LiteralValueProtoConverter.toScalaValue(lit)
- val convert = CatalystTypeConverters.createToCatalystConverter(dataType)
- expressions.Literal(convert(scalaValue), dataType)
+ val (dataType, value) = FromProtoToCatalystConverter.convert(lit)
+ expressions.Literal(value, dataType)
+ }
+
+ private object FromProtoToCatalystConverter extends FromProtoConvertor {
Review Comment:
This directly converts the literal to its catalyst representation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54670][CONNECT] Rework Connect Literal handling [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #53430: [SPARK-54670][CONNECT] Rework Connect Literal handling
URL: https://github.com/apache/spark/pull/53430
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54670][CONNECT] Rework Connect Literal handling [spark]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #53430:
URL: https://github.com/apache/spark/pull/53430#issuecomment-4107347341
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54670][CONNECT] Rework Connect Literal handling [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #53430:
URL: https://github.com/apache/spark/pull/53430#discussion_r2608008075
##########
sql/connect/common/src/test/resources/query-tests/explain-results/function_typedLit.explain:
##########
Review Comment:
Difference in `Binary` handling:
```
0x0806 AS X'0806'#0,
[8,6] AS ARRAY(8Y, 6Y)#0
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-54670][CONNECT] Rework Connect Literal handling [spark]
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #53430:
URL: https://github.com/apache/spark/pull/53430#discussion_r2607998774
##########
sql/connect/common/src/test/resources/query-tests/explain-results/function_lit_array.explain:
##########
Review Comment:
Difference in Decimal handling:
```
[89.976200000000000000,89.976210000000000000] AS ARRAY(89.976200000000000000BD, 89.976210000000000000BD)#0, [89889.766723100000000000,89889.766723100000000000] AS ARRAY(89889.766723100000000000BD, 89889.766723100000000000BD)#0
[89.97620,89.97621] AS ARRAY(89.97620BD, 89.97621BD)#0, [89889.7667231,89889.7667231] AS ARRAY(89889.7667231BD, 89889.7667231BD)#0
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org