You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2022/07/04 03:01:44 UTC
[GitHub] [spark] HyukjinKwon opened a new pull request, #37067: [WIP][SPARK-39666][SQL] Use UnsafeProjection.create to respect `spark.sql.codegen.factoryMode` in RowEncoder
HyukjinKwon opened a new pull request, #37067:
URL: https://github.com/apache/spark/pull/37067
### What changes were proposed in this pull request?
This PR is sort of a followup of https://github.com/apache/spark/commit/88d8de9260edf6e9d5449ff7ef6e35d16051fc9f to respect interpreted projections in `RowEncoder`. Similar fix was made before in deserialization side at https://github.com/apache/spark/commit/26128484228089c74517cd15cef0bb4166a4186f, and this PR completes it in serialization side too.
### Why are the changes needed?
This is a symmetry with https://github.com/apache/spark/commit/26128484228089c74517cd15cef0bb4166a4186f. We should make the configurations respected consistently.
### Does this PR introduce _any_ user-facing change?
No, by default.
Yes, when users set `spark.sql.codegen.factoryMode` to other values. The operations such as DSv1, DSv2, or object conversions such as `MapObjects` will respect the value.
### How was this patch tested?
Existing test cases at `RowEncoderSuite extends CodegenInterpretedPlanTest` should cover this.
--
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
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #37067: [WIP][SPARK-39666][SQL] Use UnsafeProjection.create to respect `spark.sql.codegen.factoryMode` in RowEncoder
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #37067:
URL: https://github.com/apache/spark/pull/37067#discussion_r912660637
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala:
##########
@@ -1916,12 +1935,11 @@ case class ValidateExternalType(child: Expression, expected: DataType, lenient:
}
}
- override def eval(input: InternalRow): Any = {
- val result = child.eval(input)
- if (checkType(result)) {
- result
+ override def nullSafeEval(input: Any): Any = {
Review Comment:
This is also matched with the codegen side if I am not mistaken.
--
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
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #37067: [WIP][SPARK-39666][SQL] Use UnsafeProjection.create to respect `spark.sql.codegen.factoryMode` in RowEncoder
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #37067:
URL: https://github.com/apache/spark/pull/37067#discussion_r912660130
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala:
##########
@@ -555,8 +555,27 @@ case class NewInstance(
}
override def eval(input: InternalRow): Any = {
Review Comment:
I borrowed some codes from `InvokeLike`. It's a bit complicated to share the same codebase.
--
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
[GitHub] [spark] HyukjinKwon commented on pull request #37067: [SPARK-39666][SQL] Use UnsafeProjection.create to respect `spark.sql.codegen.factoryMode` in ExpressionEncoder
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #37067:
URL: https://github.com/apache/spark/pull/37067#issuecomment-1173645455
cc @cloud-fan
--
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
[GitHub] [spark] HyukjinKwon commented on pull request #37067: [SPARK-39666][SQL] Use UnsafeProjection.create to respect `spark.sql.codegen.factoryMode` in ExpressionEncoder
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #37067:
URL: https://github.com/apache/spark/pull/37067#issuecomment-1173665171
Merged to master.
--
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
[GitHub] [spark] HyukjinKwon closed pull request #37067: [SPARK-39666][SQL] Use UnsafeProjection.create to respect `spark.sql.codegen.factoryMode` in ExpressionEncoder
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #37067: [SPARK-39666][SQL] Use UnsafeProjection.create to respect `spark.sql.codegen.factoryMode` in ExpressionEncoder
URL: https://github.com/apache/spark/pull/37067
--
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
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #37067: [WIP][SPARK-39666][SQL] Use UnsafeProjection.create to respect `spark.sql.codegen.factoryMode` in RowEncoder
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #37067:
URL: https://github.com/apache/spark/pull/37067#discussion_r912659945
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala:
##########
@@ -555,8 +555,27 @@ case class NewInstance(
}
override def eval(input: InternalRow): Any = {
Review Comment:
I believe this is now matched with codegen part.
--
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