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/08/20 11:41:53 UTC
[GitHub] [spark] wangyum opened a new pull request, #37589: [SPARK-40152][SQL] Fix split_part codegen compilation issue
wangyum opened a new pull request, #37589:
URL: https://github.com/apache/spark/pull/37589
### What changes were proposed in this pull request?
Fix `split_part` codegen compilation issue:
```sql
SELECT split_part(str, delimiter, partNum) FROM VALUES ('11.12.13', '.', 3) AS v1(str, delimiter, partNum);
```
```
org.codehaus.commons.compiler.CompileException: File 'generated.java', Line 42, Column 1: failed to compile: org.codehaus.commons.compiler.CompileException: File 'generated.java', Line 42, Column 1: Expression "project_isNull_0 = false" is not a type
```
### Why are the changes needed?
Fix bug.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Unit test.
--
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] cloud-fan commented on a diff in pull request #37589: [SPARK-40152][SQL] Fix split_part codegen compilation issue
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on code in PR #37589:
URL: https://github.com/apache/spark/pull/37589#discussion_r952168934
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -2270,9 +2270,9 @@ case class ElementAt(
case Some(value) =>
val defaultValueEval = value.genCode(ctx)
s"""
- ${defaultValueEval.code}
- ${ev.isNull} = ${defaultValueEval.isNull}
- ${ev.value} = ${defaultValueEval.value}
+ ${defaultValueEval.code};
Review Comment:
do we need `;` here? The `.code` should always be a valid java statement.
--
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] srowen closed pull request #37589: [SPARK-40152][SQL] Fix split_part codegen compilation issue
Posted by GitBox <gi...@apache.org>.
srowen closed pull request #37589: [SPARK-40152][SQL] Fix split_part codegen compilation issue
URL: https://github.com/apache/spark/pull/37589
--
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] wangyum commented on a diff in pull request #37589: [SPARK-40152][SQL] Fix split_part codegen compilation issue
Posted by GitBox <gi...@apache.org>.
wangyum commented on code in PR #37589:
URL: https://github.com/apache/spark/pull/37589#discussion_r952429755
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:
##########
@@ -2270,9 +2270,9 @@ case class ElementAt(
case Some(value) =>
val defaultValueEval = value.genCode(ctx)
s"""
- ${defaultValueEval.code}
- ${ev.isNull} = ${defaultValueEval.isNull}
- ${ev.value} = ${defaultValueEval.value}
+ ${defaultValueEval.code};
Review Comment:
fixed: https://github.com/apache/spark/pull/37626/files
--
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] srowen commented on pull request #37589: [SPARK-40152][SQL] Fix split_part codegen compilation issue
Posted by GitBox <gi...@apache.org>.
srowen commented on PR #37589:
URL: https://github.com/apache/spark/pull/37589#issuecomment-1221607056
Merged to master/3.3
--
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] cloud-fan commented on pull request #37589: [SPARK-40152][SQL] Fix split_part codegen compilation issue
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on PR #37589:
URL: https://github.com/apache/spark/pull/37589#issuecomment-1223565877
https://github.com/databricks/runtime/pull/36671 forgot to add new unit tests for `ElementAt` when adding a new parameter, and that's why this bug was not found... @wangyum can you help to add new unit tests in `CollectionExpressionsSuite`?
--
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] wangyum commented on pull request #37589: [SPARK-40152][SQL] Fix split_part codegen compilation issue
Posted by GitBox <gi...@apache.org>.
wangyum commented on PR #37589:
URL: https://github.com/apache/spark/pull/37589#issuecomment-1223567871
OK.
--
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