[GitHub] spark pull request #14259: [SPARK-16622][SQL] Fix NullPointerException when ...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/14259 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14259: [SPARK-16622][SQL] Fix NullPointerException when ...
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/14259#discussion_r71885571
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ObjectExpressionsSuite.scala
---
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.expressions
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.catalyst.InternalRow
+import org.apache.spark.sql.catalyst.expressions.objects.Invoke
+import org.apache.spark.sql.types.{IntegerType, ObjectType}
+
+
+class ObjectExpressionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
+
+ test("SPARK-16622: The returned value of the called method in Invoke can
be null") {
+val inputRow = InternalRow.fromSeq(Seq((false, null)))
+val cls = classOf[Tuple2[Boolean, Int]]
--- End diff --
Fixed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14259: [SPARK-16622][SQL] Fix NullPointerException when ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/14259#discussion_r71880582
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ObjectExpressionsSuite.scala
---
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.expressions
+
+import org.apache.spark.SparkFunSuite
+import org.apache.spark.sql.catalyst.InternalRow
+import org.apache.spark.sql.catalyst.expressions.objects.Invoke
+import org.apache.spark.sql.types.{IntegerType, ObjectType}
+
+
+class ObjectExpressionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
+
+ test("SPARK-16622: The returned value of the called method in Invoke can
be null") {
+val inputRow = InternalRow.fromSeq(Seq((false, null)))
+val cls = classOf[Tuple2[Boolean, Int]]
--- End diff --
`java.lang.Integer`? `Int` can not be null
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14259: [SPARK-16622][SQL] Fix NullPointerException when ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/14259#discussion_r71825821
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/DatasetAggregatorSuite.scala ---
@@ -314,4 +343,14 @@ class DatasetAggregatorSuite extends QueryTest with
SharedSQLContext {
val ds3 = sql("SELECT 'Some String' AS b, 1279869254 AS a").as[AggData]
assert(ds3.select(NameAgg.toColumn).schema.head.nullable === true)
}
+
+ test("Aggregator on empty dataset") {
--- End diff --
we can create a `ObjectExpressionsSuite` under
org.apache.spark.sql.catalyst.expressions
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14259: [SPARK-16622][SQL] Fix NullPointerException when ...
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/14259#discussion_r71825205
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/DatasetAggregatorSuite.scala ---
@@ -314,4 +343,14 @@ class DatasetAggregatorSuite extends QueryTest with
SharedSQLContext {
val ds3 = sql("SELECT 'Some String' AS b, 1279869254 AS a").as[AggData]
assert(ds3.select(NameAgg.toColumn).schema.head.nullable === true)
}
+
+ test("Aggregator on empty dataset") {
--- End diff --
Another problem is this test should not be in `DatasetAggregatorSuite`. I
don't find a proper file for this test. Which one you would suggest?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14259: [SPARK-16622][SQL] Fix NullPointerException when ...
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/14259#discussion_r71676959
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/DatasetAggregatorSuite.scala ---
@@ -314,4 +343,14 @@ class DatasetAggregatorSuite extends QueryTest with
SharedSQLContext {
val ds3 = sql("SELECT 'Some String' AS b, 1279869254 AS a").as[AggData]
assert(ds3.select(NameAgg.toColumn).schema.head.nullable === true)
}
+
+ test("Aggregator on empty dataset") {
--- End diff --
Agreed. I will update this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14259: [SPARK-16622][SQL] Fix NullPointerException when ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/14259#discussion_r71669886
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/DatasetAggregatorSuite.scala ---
@@ -314,4 +343,14 @@ class DatasetAggregatorSuite extends QueryTest with
SharedSQLContext {
val ds3 = sql("SELECT 'Some String' AS b, 1279869254 AS a").as[AggData]
assert(ds3.select(NameAgg.toColumn).schema.head.nullable === true)
}
+
+ test("Aggregator on empty dataset") {
--- End diff --
I think we should have a unit test for this bug instead of this complex
end-to-end test, e.g. we can create `Invoke` directly and call a method
returning null.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14259: [SPARK-16622][SQL] Fix NullPointerException when ...
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/14259#discussion_r71638351
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -166,11 +185,11 @@ case class Invoke(
} else {
""
}
-
val code = s"""
${obj.code}
${argGen.map(_.code).mkString("\n")}
$setIsNull
+ $callFunc
--- End diff --
uh. Looks good. Let me update this. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14259: [SPARK-16622][SQL] Fix NullPointerException when ...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/14259#discussion_r71496316
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -166,11 +185,11 @@ case class Invoke(
} else {
""
}
-
val code = s"""
${obj.code}
${argGen.map(_.code).mkString("\n")}
$setIsNull
+ $callFunc
--- End diff --
I'm thinking of something like
```
val returnPrimitive = method.isDefined &&
method.get.getReturnType.isPrimitive
val needTryCatch = method.isDefined && method.get.getExceptionTypes.nonEmpty
def getFuncResult(resultVal: String, funcCall: String): String = if
(needTryCatch) {
"""
try {
$resultVal = $funcCall;
} catch (Exception e) {
org.apache.spark.unsafe.Platform.throwException(e);
}
"""
} else {
s"$resultVal = $funcCall;"
}
val evaluate = if (returnPrimitive) {
getFuncResult(ev.value, s"${obj.value}.$functionName($argString)")
} else {
val funcResult = ctx.freshName("funcResult")
s"""
Object $funcResult = null;
$getFuncResult(funcResult, s"${obj.value}.$functionName($argString)")
if ($funcResult == null) {
${ev.isNull} = true;
} else {
${ev.value} = (${ctx.boxedType(javaType)}) $funcResult;
}
"""
}
val code = s"""
...
$javaType ${ev.value} = ${ctx.defaultValue(dataType)};
$evaluete
...
"""
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14259: [SPARK-16622][SQL] Fix NullPointerException when ...
Github user kiszk commented on a diff in the pull request: https://github.com/apache/spark/pull/14259#discussion_r71371459 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala --- @@ -147,12 +158,12 @@ case class Invoke( } --- End diff -- Oh, yes. You are right. Sorry for my misunderstanding. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14259: [SPARK-16622][SQL] Fix NullPointerException when ...
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/14259#discussion_r71331553
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -147,12 +158,12 @@ case class Invoke(
}
--- End diff --
Here `${ev.value}` will be `null` only when `ctx.defaultValue(javaType)` is
`null`. There is a `postNullCheck` to do the extra check for this case.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14259: [SPARK-16622][SQL] Fix NullPointerException when ...
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/14259#discussion_r71330808
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -134,10 +134,21 @@ case class Invoke(
val argGen = arguments.map(_.genCode(ctx))
val argString = argGen.map(_.value).mkString(", ")
+val funcVal = ctx.freshName("funcVal")
+val funcValIsNull = ctx.freshName("funcValIsNull")
val callFunc = if (method.isDefined &&
method.get.getReturnType.isPrimitive) {
- s"${obj.value}.$functionName($argString)"
+ s"""
+$javaType $funcVal = ${obj.value}.$functionName($argString);
--- End diff --
yea, I miss the checking on this. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14259: [SPARK-16622][SQL] Fix NullPointerException when ...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/14259#discussion_r71313787
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -134,10 +134,21 @@ case class Invoke(
val argGen = arguments.map(_.genCode(ctx))
val argString = argGen.map(_.value).mkString(", ")
+val funcVal = ctx.freshName("funcVal")
+val funcValIsNull = ctx.freshName("funcValIsNull")
val callFunc = if (method.isDefined &&
method.get.getReturnType.isPrimitive) {
- s"${obj.value}.$functionName($argString)"
+ s"""
+$javaType $funcVal = ${obj.value}.$functionName($argString);
--- End diff --
Can we guarantee `obj` is always non-null?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14259: [SPARK-16622][SQL] Fix NullPointerException when ...
Github user kiszk commented on a diff in the pull request:
https://github.com/apache/spark/pull/14259#discussion_r71313676
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects/objects.scala
---
@@ -147,12 +158,12 @@ case class Invoke(
}
--- End diff --
Is is better to pass `$funcValIsNull` to `${ev.isNull}`? This is because
`$funcValIsNull` determines whether `${ev.value}` is `null` or not instead of
`$ev.isNull`.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #14259: [SPARK-16622][SQL] Fix NullPointerException when ...
GitHub user viirya opened a pull request: https://github.com/apache/spark/pull/14259 [SPARK-16622][SQL] Fix NullPointerException when the returned value of the called method in Invoke is null ## What changes were proposed in this pull request? Currently we don't check the value returned by called method in `Invoke`. When the returned value is null, `NullPointerException` will be thrown. ## How was this patch tested? Jenkins tests. (If this patch involves UI changes, please attach a screenshot; otherwise, remove this) â¦d method is null. You can merge this pull request into a Git repository by running: $ git pull https://github.com/viirya/spark-1 agg-empty-ds Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/14259.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #14259 commit 1ba01e4b87c47d55e229325db9282180bd20ea74 Author: Liang-Chi Hsieh Date: 2016-07-19T08:31:45Z Fix a bug in Invoke that would be failed when returned value of called method is null. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
