[GitHub] spark pull request: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user liancheng commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-108234369 LGTM --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/6405 --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-108234694 Thanks - I've merged this. Are there any other expressions that we should add the type checking to? --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-107334960 Merged build started. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-107334830 Merged build triggered. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-107336115 [Test build #33882 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33882/consoleFull) for PR 6405 at commit [`364248f`](https://github.com/apache/spark/commit/364248f14fc1eea11ee5ecb43a3982bba2755015). --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-107342822 Merged build triggered. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-107342852 Merged build started. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-107343141 [Test build #33883 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33883/consoleFull) for PR 6405 at commit [`b5ff31b`](https://github.com/apache/spark/commit/b5ff31b0dde66ed24634dc8773dfafb11b95ee50). --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31406630
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
---
@@ -264,6 +264,10 @@ object NullPropagation extends Rule[LogicalPlan] {
case e @ Substring(_, Literal(null, _), _) = Literal.create(null,
e.dataType)
case e @ Substring(_, _, Literal(null, _)) = Literal.create(null,
e.dataType)
+ // MaxOf and MinOf can't do null propagation
--- End diff --
`MaxOf(null, null) == null` but `MaxOf(null, value) != null`, that's why we
can't do null propagation here. And `MaxOf(null, null)` will be optimized in
`ConstantFolding`, we don't need to handle it here.
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-107382964 Merged build finished. Test PASSed. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-107382949 [Test build #33882 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33882/consoleFull) for PR 6405 at commit [`364248f`](https://github.com/apache/spark/commit/364248f14fc1eea11ee5ecb43a3982bba2755015). * This patch **passes all tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `trait TypeCheckResult ` * ` case class TypeCheckFailure(message: String) extends TypeCheckResult ` * `abstract class UnaryArithmetic extends UnaryExpression ` * `case class UnaryMinus(child: Expression) extends UnaryArithmetic ` * `case class Sqrt(child: Expression) extends UnaryArithmetic ` * `case class Abs(child: Expression) extends UnaryArithmetic ` * `case class BitwiseNot(child: Expression) extends UnaryArithmetic ` * `case class MaxOf(left: Expression, right: Expression) extends BinaryArithmetic ` * `case class MinOf(left: Expression, right: Expression) extends BinaryArithmetic ` * `case class Atan2(left: Expression, right: Expression)` * `case class Hypot(left: Expression, right: Expression)` * `case class EqualTo(left: Expression, right: Expression) extends BinaryComparison ` --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-107387510 Merged build finished. Test PASSed. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-107387487 [Test build #33883 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33883/consoleFull) for PR 6405 at commit [`b5ff31b`](https://github.com/apache/spark/commit/b5ff31b0dde66ed24634dc8773dfafb11b95ee50). * This patch **passes all tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `trait TypeCheckResult ` * ` case class TypeCheckFailure(message: String) extends TypeCheckResult ` * `abstract class UnaryArithmetic extends UnaryExpression ` * `case class UnaryMinus(child: Expression) extends UnaryArithmetic ` * `case class Sqrt(child: Expression) extends UnaryArithmetic ` * `case class Abs(child: Expression) extends UnaryArithmetic ` * `case class BitwiseNot(child: Expression) extends UnaryArithmetic ` * `case class MaxOf(left: Expression, right: Expression) extends BinaryArithmetic ` * `case class MinOf(left: Expression, right: Expression) extends BinaryArithmetic ` * `case class Atan2(left: Expression, right: Expression)` * `case class Hypot(left: Expression, right: Expression)` * `case class EqualTo(left: Expression, right: Expression) extends BinaryComparison ` --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31394776
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
---
@@ -125,7 +133,13 @@ case class GroupExpression(children: Seq[Expression])
extends Expression {
* so that the proper type conversions can be performed in the analyzer.
*/
trait ExpectsInputTypes {
+ self: Expression =
def expectedChildTypes: Seq[DataType]
+ override def checkInputDataTypes: TypeCheckResult = {
+// We will always do type casting for `ExpectsInputTypes` in
`HiveTypeCoercion`,
+// so type mismatch error won't be reported here, but for underling
`Cast`s.
--- End diff --
Seems this will result in a confusing error, since it will complain about
casts that the user does not see in their query.
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31394778
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
---
@@ -86,12 +86,20 @@ abstract class Expression extends TreeNode[Expression] {
case (i1, i2) = i1 == i2
}
}
+
+ /**
+ * Check the input data types, returns `TypeCheckResult.success` if it's
valid,
+ * or return a `TypeCheckResult` with an error message if invalid.
+ * TODO: we should remove the default implementation and implement it
for all
+ * expressions with proper error message.
+ */
+ def checkInputDataTypes(): TypeCheckResult = TypeCheckResult.success
--- End diff --
We should also document that its not valid to call this until
`childrenResolved == true`.
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31394820
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala
---
@@ -395,10 +362,13 @@ case class CaseWhen(branches: Seq[Expression])
extends CaseWhenLike {
override def children: Seq[Expression] = branches
- override lazy val resolved: Boolean =
-childrenResolved
-whenList.forall(_.dataType == BooleanType)
-valueTypesEqual
+ override protected def checkTypesInternal(): TypeCheckResult = {
+if (whenList.forall(_.dataType == BooleanType)) {
+ TypeCheckResult.success
+} else {
+ TypeCheckResult.fail(sWHEN expressions in CaseWhen should all be
boolean type)
--- End diff --
Perhaps include the first one that fails and its type in the error?
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31394936
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCheckResult.scala
---
@@ -0,0 +1,33 @@
+/*
+ * 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.analysis
+
+/**
+ * Represents the result of `Expression.checkInputDataTypes`.
+ * We will throw `AnalysisException` in `CheckAnalysis` if error message
is not null.
+ * Use [[TypeCheckResult.success]] and [[TypeCheckResult.fail]] to
instantiate this.
+ *
+ */
+class TypeCheckResult private (val errorMessage: String) extends AnyVal {
+ def hasError: Boolean = errorMessage != null
+}
+
+object TypeCheckResult {
+ val success: TypeCheckResult = new TypeCheckResult(null)
+ def fail(msg: String): TypeCheckResult = new TypeCheckResult(msg)
+}
--- End diff --
I was thinking about using value class to save some allocation, but that's
probably premature optimization. I think it'd make sense to follow your
suggestion (maybe remove the Object, and just name it TypeCheckSuccess and
TypeCheckFailure)
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31394977
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
---
@@ -86,12 +86,20 @@ abstract class Expression extends TreeNode[Expression] {
case (i1, i2) = i1 == i2
}
}
+
+ /**
+ * Check the input data types, returns `TypeCheckResult.success` if it's
valid,
+ * or return a `TypeCheckResult` with an error message if invalid.
+ * TODO: we should remove the default implementation and implement it
for all
+ * expressions with proper error message.
+ */
+ def checkInputDataTypes(): TypeCheckResult = TypeCheckResult.success
--- End diff --
talked to @marmbrus -- since it is named with a verb, and might be
expensive, let's keep the parentheses here for now.
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31394968
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCheckResult.scala
---
@@ -0,0 +1,33 @@
+/*
+ * 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.analysis
+
+/**
+ * Represents the result of `Expression.checkInputDataTypes`.
+ * We will throw `AnalysisException` in `CheckAnalysis` if error message
is not null.
+ * Use [[TypeCheckResult.success]] and [[TypeCheckResult.fail]] to
instantiate this.
+ *
+ */
+class TypeCheckResult private (val errorMessage: String) extends AnyVal {
+ def hasError: Boolean = errorMessage != null
+}
+
+object TypeCheckResult {
+ val success: TypeCheckResult = new TypeCheckResult(null)
+ def fail(msg: String): TypeCheckResult = new TypeCheckResult(msg)
+}
--- End diff --
+1 to this probably being premature optimization. I'd also consider
`TypeCheck.Success` and `TypeCheck.Failure`.
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31394962
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
---
@@ -125,7 +133,13 @@ case class GroupExpression(children: Seq[Expression])
extends Expression {
* so that the proper type conversions can be performed in the analyzer.
*/
trait ExpectsInputTypes {
+ self: Expression =
def expectedChildTypes: Seq[DataType]
+ override def checkInputDataTypes: TypeCheckResult = {
+// We will always do type casting for `ExpectsInputTypes` in
`HiveTypeCoercion`,
+// so type mismatch error won't be reported here, but for underling
`Cast`s.
--- End diff --
We'd need to check in ExpectedInputConversion whether a cast is OK so we
don't blindly apply the cast. We can do that as a followup pr too.
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31394755
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
---
@@ -86,12 +86,20 @@ abstract class Expression extends TreeNode[Expression] {
case (i1, i2) = i1 == i2
}
}
+
+ /**
+ * Check the input data types, returns `TypeCheckResult.success` if it's
valid,
+ * or return a `TypeCheckResult` with an error message if invalid.
+ * TODO: we should remove the default implementation and implement it
for all
+ * expressions with proper error message.
+ */
+ def checkInputDataTypes(): TypeCheckResult = TypeCheckResult.success
--- End diff --
I think @rxin may have asked for this, but why `()`. Isn't this function
pure?
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31394810
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
---
@@ -264,6 +264,10 @@ object NullPropagation extends Rule[LogicalPlan] {
case e @ Substring(_, Literal(null, _), _) = Literal.create(null,
e.dataType)
case e @ Substring(_, _, Literal(null, _)) = Literal.create(null,
e.dataType)
+ // MaxOf and MinOf can't do null propagation
--- End diff --
Yeah, I'm actually a little bit confused here. Doesn't `MaxOf(null, null)
== 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31394796
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionTypeCheckingSuite.scala
---
@@ -0,0 +1,131 @@
+/*
+ * 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.sql.AnalysisException
+import org.apache.spark.sql.catalyst.analysis.SimpleAnalyzer
+import org.apache.spark.sql.catalyst.plans.logical.LocalRelation
+import org.apache.spark.sql.catalyst.dsl.plans._
+import org.apache.spark.sql.catalyst.dsl.expressions._
+import org.apache.spark.sql.types.{BooleanType, StringType}
+import org.scalatest.FunSuite
+
+
+class ExpressionTypeCheckingSuite extends FunSuite {
+
+ val testRelation = LocalRelation(
+'intField.int,
+'stringField.string,
+'booleanField.boolean,
+'complexField.array(StringType))
+
+ def assertError(expr: Expression, errorMessage: String): Unit = {
+val e = intercept[AnalysisException] {
+ assertSuccess(expr)
+}
+assert(e.getMessage.contains(
+ scannot resolve '${expr.prettyString}' due to data type mismatch:))
+assert(e.getMessage.contains(errorMessage))
+ }
+
+ def assertSuccess(expr: Expression): Unit = {
+val analyzed = testRelation.select(expr.as(c)).analyze
+SimpleAnalyzer.checkAnalysis(analyzed)
+ }
+
+ def assertErrorForDifferingTypes(expr: Expression): Unit = {
+assertError(expr,
+ sdiffering types in ${expr.getClass.getSimpleName} (IntegerType and
BooleanType).)
+ }
+
+ test(check types for unary arithmetic) {
+assertError(UnaryMinus('stringField), operator - accepts numeric
type)
+assertSuccess(Sqrt('stringField)) // We will cast String to Double for
sqrt
--- End diff --
We should check to see why Hive does.
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31389338
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/HiveTypeCoercion.scala
---
@@ -324,6 +324,7 @@ trait HiveTypeCoercion {
* e1 union e2 max(s1, s2) + max(p1-s1, p2-s2) max(s1, s2)
* sum(e1) p1 + 10 s1
* avg(e1) p1 + 4 s1 + 4
+ * compare max(p1, p2) max(s1, s2)
--- End diff --
This line should be moved elsewhere and reworded, since comparisons result
in boolean values rather than decimal values.
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31389366
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/HiveTypeCoercion.scala
---
@@ -610,17 +599,13 @@ trait HiveTypeCoercion {
*/
object Division extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan): LogicalPlan = plan
transformAllExpressions {
- // Skip nodes who's children have not been resolved yet.
- case e if !e.childrenResolved = e
+ // Skip Divisions who has not been resolved yet,
--- End diff --
Not only `Division`s, all types of expressions that are not resolved are
caught here.
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31389492
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
---
@@ -86,12 +86,20 @@ abstract class Expression extends TreeNode[Expression] {
case (i1, i2) = i1 == i2
}
}
+
+ /**
+ * Check the input data types, returns `TypeCheckResult.success` if it's
valid,
+ * or return a `TypeCheckResult` with an error message if invalid.
+ * TODO: we should remove the default implementation and implement it
for all
+ * expressions with proper error message.
+ */
+ def checkInputDataTypes(): TypeCheckResult = TypeCheckResult.success
}
abstract class BinaryExpression extends Expression with
trees.BinaryNode[Expression] {
self: Product =
- def symbol: String
+ def symbol: String = sys.error(sBinaryExpressions must either override
toString or symbol)
--- End diff --
Nit: please swap either and override.
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31389495
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
---
@@ -86,12 +86,20 @@ abstract class Expression extends TreeNode[Expression] {
case (i1, i2) = i1 == i2
}
}
+
+ /**
+ * Check the input data types, returns `TypeCheckResult.success` if it's
valid,
--- End diff --
Nit: Check - Checks
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31389497
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
---
@@ -86,12 +86,20 @@ abstract class Expression extends TreeNode[Expression] {
case (i1, i2) = i1 == i2
}
}
+
+ /**
+ * Check the input data types, returns `TypeCheckResult.success` if it's
valid,
+ * or return a `TypeCheckResult` with an error message if invalid.
--- End diff --
Nit: return - returns
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31389663
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCheckResult.scala
---
@@ -0,0 +1,33 @@
+/*
+ * 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.analysis
+
+/**
+ * Represents the result of `Expression.checkInputDataTypes`.
+ * We will throw `AnalysisException` in `CheckAnalysis` if error message
is not null.
+ * Use [[TypeCheckResult.success]] and [[TypeCheckResult.fail]] to
instantiate this.
+ *
+ */
+class TypeCheckResult private (val errorMessage: String) extends AnyVal {
+ def hasError: Boolean = errorMessage != null
+}
+
+object TypeCheckResult {
+ val success: TypeCheckResult = new TypeCheckResult(null)
+ def fail(msg: String): TypeCheckResult = new TypeCheckResult(msg)
+}
--- End diff --
I'm thinking something about this:
```scala
trait TypeCheckResult {
def isFailure: Boolean
def isSuccess: Boolean
}
object TypeCheckResult {
object Success extends TypeCheckResult {
override def isFailure: Boolean = false
override def isSuccess: Boolean = true
}
case class Failure(message: String) extends TypeCheckResult {
override def isFailure: Boolean = true
override def isSuccess: Boolean = false
}
}
```
One potential benefit is that now you can check `TypeCheckResult` within a
pattern match.
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31390606
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/HiveTypeCoercion.scala
---
@@ -633,25 +618,26 @@ trait HiveTypeCoercion {
import HiveTypeCoercion._
def apply(plan: LogicalPlan): LogicalPlan = plan
transformAllExpressions {
- case cw: CaseWhenLike if !cw.resolved cw.childrenResolved
!cw.valueTypesEqual =
+ case cw: CaseWhenLike if cw.childrenResolved
cw.checkInputDataTypes().hasError =
logDebug(sInput values for null casting
${cw.valueTypes.mkString(,)})
-val commonType = cw.valueTypes.reduce { (v1, v2) =
- findTightestCommonType(v1, v2).getOrElse(sys.error(
-sTypes in CASE WHEN must be the same or coercible to a common
type: $v1 != $v2))
-}
-val transformedBranches = cw.branches.sliding(2, 2).map {
- case Seq(when, value) if value.dataType != commonType =
-Seq(when, Cast(value, commonType))
- case Seq(elseVal) if elseVal.dataType != commonType =
-Seq(Cast(elseVal, commonType))
- case s = s
-}.reduce(_ ++ _)
-cw match {
- case _: CaseWhen =
-CaseWhen(transformedBranches)
- case CaseKeyWhen(key, _) =
-CaseKeyWhen(key, transformedBranches)
-}
+cw.valueTypes.foldLeft[Option[DataType]](Some(NullType))((r, c) =
r match {
+ case None = None
+ case Some(d) = findTightestCommonType(d, c)
+}).map { commonType =
+ val transformedBranches = cw.branches.sliding(2, 2).map {
+case Seq(when, value) if value.dataType != commonType =
+ Seq(when, Cast(value, commonType))
+case Seq(elseVal) if elseVal.dataType != commonType =
+ Seq(Cast(elseVal, commonType))
+case s = s
+ }.reduce(_ ++ _)
+ cw match {
+case _: CaseWhen =
+ CaseWhen(transformedBranches)
+case CaseKeyWhen(key, _) =
+ CaseKeyWhen(key, transformedBranches)
+ }
+}.getOrElse(cw)
--- End diff --
Shouldn't we throw an exception here since
`cw.checkInputDataTypes().hasError` is true?
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31390713
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/HiveTypeCoercion.scala
---
@@ -633,25 +618,26 @@ trait HiveTypeCoercion {
import HiveTypeCoercion._
def apply(plan: LogicalPlan): LogicalPlan = plan
transformAllExpressions {
- case cw: CaseWhenLike if !cw.resolved cw.childrenResolved
!cw.valueTypesEqual =
+ case cw: CaseWhenLike if cw.childrenResolved
cw.checkInputDataTypes().hasError =
logDebug(sInput values for null casting
${cw.valueTypes.mkString(,)})
-val commonType = cw.valueTypes.reduce { (v1, v2) =
- findTightestCommonType(v1, v2).getOrElse(sys.error(
-sTypes in CASE WHEN must be the same or coercible to a common
type: $v1 != $v2))
-}
-val transformedBranches = cw.branches.sliding(2, 2).map {
- case Seq(when, value) if value.dataType != commonType =
-Seq(when, Cast(value, commonType))
- case Seq(elseVal) if elseVal.dataType != commonType =
-Seq(Cast(elseVal, commonType))
- case s = s
-}.reduce(_ ++ _)
-cw match {
- case _: CaseWhen =
-CaseWhen(transformedBranches)
- case CaseKeyWhen(key, _) =
-CaseKeyWhen(key, transformedBranches)
-}
+cw.valueTypes.foldLeft[Option[DataType]](Some(NullType))((r, c) =
r match {
+ case None = None
+ case Some(d) = findTightestCommonType(d, c)
+}).map { commonType =
+ val transformedBranches = cw.branches.sliding(2, 2).map {
+case Seq(when, value) if value.dataType != commonType =
+ Seq(when, Cast(value, commonType))
+case Seq(elseVal) if elseVal.dataType != commonType =
+ Seq(Cast(elseVal, commonType))
+case s = s
+ }.reduce(_ ++ _)
+ cw match {
+case _: CaseWhen =
+ CaseWhen(transformedBranches)
+case CaseKeyWhen(key, _) =
+ CaseKeyWhen(key, transformedBranches)
+ }
+}.getOrElse(cw)
--- End diff --
Oh I see, `CheckAnalysis` catches all malformed expressions for us at the
end of the analysis phase.
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31390775
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/HiveTypeCoercion.scala
---
@@ -633,25 +618,26 @@ trait HiveTypeCoercion {
import HiveTypeCoercion._
def apply(plan: LogicalPlan): LogicalPlan = plan
transformAllExpressions {
- case cw: CaseWhenLike if !cw.resolved cw.childrenResolved
!cw.valueTypesEqual =
+ case cw: CaseWhenLike if cw.childrenResolved
cw.checkInputDataTypes().hasError =
logDebug(sInput values for null casting
${cw.valueTypes.mkString(,)})
-val commonType = cw.valueTypes.reduce { (v1, v2) =
- findTightestCommonType(v1, v2).getOrElse(sys.error(
-sTypes in CASE WHEN must be the same or coercible to a common
type: $v1 != $v2))
-}
-val transformedBranches = cw.branches.sliding(2, 2).map {
- case Seq(when, value) if value.dataType != commonType =
-Seq(when, Cast(value, commonType))
- case Seq(elseVal) if elseVal.dataType != commonType =
-Seq(Cast(elseVal, commonType))
- case s = s
-}.reduce(_ ++ _)
-cw match {
- case _: CaseWhen =
-CaseWhen(transformedBranches)
- case CaseKeyWhen(key, _) =
-CaseKeyWhen(key, transformedBranches)
-}
+cw.valueTypes.foldLeft[Option[DataType]](Some(NullType))((r, c) =
r match {
+ case None = None
+ case Some(d) = findTightestCommonType(d, c)
+}).map { commonType =
+ val transformedBranches = cw.branches.sliding(2, 2).map {
+case Seq(when, value) if value.dataType != commonType =
+ Seq(when, Cast(value, commonType))
+case Seq(elseVal) if elseVal.dataType != commonType =
+ Seq(Cast(elseVal, commonType))
+case s = s
+ }.reduce(_ ++ _)
+ cw match {
+case _: CaseWhen =
+ CaseWhen(transformedBranches)
+case CaseKeyWhen(key, _) =
+ CaseKeyWhen(key, transformedBranches)
+ }
+}.getOrElse(cw)
--- End diff --
The following version might be questionably easier to read:
```scala
val maybeCommonType = cw.valueTypes.map(Some(_)).reduce {
case (Some(dt1), Some(dt2)) = findTightestCommonType(dt1, dt2)
case _ = None
}
maybeCommonType.map { commonType =
val castedBranches = cw.branches.grouped(2).map {
case Seq(when, value) if value.dataType != commonType =
Seq(when, Cast(value, commonType))
case Seq(elseVal) if elseVal.dataType != commonType =
Seq(Cast(elseVal, commonType))
case other = other
}.reduce(_ ++ _)
cw match {
case _: CaseWhen = CaseWhen(castedBranches)
case CaseKeyWhen(key, _) = CaseKeyWhen(key, castedBranches)
}
}.getOrElse(cw)
```
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31390926
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
---
@@ -125,7 +133,13 @@ case class GroupExpression(children: Seq[Expression])
extends Expression {
* so that the proper type conversions can be performed in the analyzer.
*/
trait ExpectsInputTypes {
+ self: Expression =
def expectedChildTypes: Seq[DataType]
+ override def checkInputDataTypes: TypeCheckResult = {
--- End diff --
`()` missed
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106993379 Merged build triggered. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31378244
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
---
@@ -86,12 +86,18 @@ abstract class Expression extends TreeNode[Expression] {
case (i1, i2) = i1 == i2
}
}
+
+ /**
+ * Check the input data types, returns `TypeCheckResult.success` if it's
valid,
+ * or return a `TypeCheckResult` with an error message if invalid.
+ */
+ def checkInputDataTypes(): TypeCheckResult = TypeCheckResult.success
--- End diff --
Sure sounds good to do this in a follow up pr. Mark a TODO here.
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106993862 [Test build #33797 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33797/consoleFull) for PR 6405 at commit [`7e144e1`](https://github.com/apache/spark/commit/7e144e1d4adbd7ed023b89c77ed50003bc0952c0). * This patch **fails Spark unit tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `class TypeCheckResult(val errorMessage: String) extends AnyVal ` * `abstract class UnaryArithmetic extends UnaryExpression ` * `case class UnaryMinus(child: Expression) extends UnaryArithmetic ` * `case class Sqrt(child: Expression) extends UnaryArithmetic ` * `case class Abs(child: Expression) extends UnaryArithmetic ` * `case class BitwiseNot(child: Expression) extends UnaryArithmetic ` * `case class MaxOf(left: Expression, right: Expression) extends BinaryArithmetic ` * `case class MinOf(left: Expression, right: Expression) extends BinaryArithmetic ` * `case class Atan2(left: Expression, right: Expression)` * `case class Hypot(left: Expression, right: Expression)` * `case class EqualTo(left: Expression, right: Expression) extends BinaryComparison ` --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user cloud-fan commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106994447 The tests will still fail as we need 2 more changes to satisfy the type equal constraint for `EqualTo`. * https://github.com/apache/spark/pull/6505 to avoid something like `EqualTo(true, 2)` * https://github.com/apache/spark/pull/6516 to avoid something like `EqualTo(timestamp, Literal(0))` --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106993384 Merged build started. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106993495 [Test build #33798 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33798/consoleFull) for PR 6405 at commit [`0515cf3`](https://github.com/apache/spark/commit/0515cf37be18039f96aebc008c814027c0c7db2c). --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106993751 Merged build finished. Test FAILed. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106993873 Merged build finished. Test FAILed. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106993749 [Test build #33799 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33799/consoleFull) for PR 6405 at commit [`9daa906`](https://github.com/apache/spark/commit/9daa90688e2324d1aeac5dcd28e8d8d888f447a2). * This patch **fails Scala style tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `abstract class UnaryArithmetic extends UnaryExpression ` * `case class UnaryMinus(child: Expression) extends UnaryArithmetic ` * `case class Sqrt(child: Expression) extends UnaryArithmetic ` * `case class Abs(child: Expression) extends UnaryArithmetic ` * `case class BitwiseNot(child: Expression) extends UnaryArithmetic ` * `case class MaxOf(left: Expression, right: Expression) extends BinaryArithmetic ` * `case class MinOf(left: Expression, right: Expression) extends BinaryArithmetic ` * `case class Atan2(left: Expression, right: Expression)` * `case class Hypot(left: Expression, right: Expression)` * `case class EqualTo(left: Expression, right: Expression) extends BinaryComparison ` --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106994236 Merged build triggered. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106994241 Merged build started. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106993697 [Test build #33799 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33799/consoleFull) for PR 6405 at commit [`9daa906`](https://github.com/apache/spark/commit/9daa90688e2324d1aeac5dcd28e8d8d888f447a2). --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106993621 Merged build triggered. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106993581 Merged build finished. Test FAILed. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106993628 Merged build started. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106993580 [Test build #33798 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33798/consoleFull) for PR 6405 at commit [`0515cf3`](https://github.com/apache/spark/commit/0515cf37be18039f96aebc008c814027c0c7db2c). * This patch **fails Scala style tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `abstract class UnaryArithmetic extends UnaryExpression ` * `case class UnaryMinus(child: Expression) extends UnaryArithmetic ` * `case class Sqrt(child: Expression) extends UnaryArithmetic ` * `case class Abs(child: Expression) extends UnaryArithmetic ` * `case class BitwiseNot(child: Expression) extends UnaryArithmetic ` * `case class MaxOf(left: Expression, right: Expression) extends BinaryArithmetic ` * `case class MinOf(left: Expression, right: Expression) extends BinaryArithmetic ` * `case class Atan2(left: Expression, right: Expression)` * `case class Hypot(left: Expression, right: Expression)` * `case class EqualTo(left: Expression, right: Expression) extends BinaryComparison ` --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106994335 [Test build #33801 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33801/consoleFull) for PR 6405 at commit [`89aa1d4`](https://github.com/apache/spark/commit/89aa1d42fbf54955dcd45740fd2cba7e2a396da8). --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-107006885 [Test build #33801 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33801/consoleFull) for PR 6405 at commit [`89aa1d4`](https://github.com/apache/spark/commit/89aa1d42fbf54955dcd45740fd2cba7e2a396da8). * This patch **fails Spark unit tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `abstract class UnaryArithmetic extends UnaryExpression ` * `case class UnaryMinus(child: Expression) extends UnaryArithmetic ` * `case class Sqrt(child: Expression) extends UnaryArithmetic ` * `case class Abs(child: Expression) extends UnaryArithmetic ` * `case class BitwiseNot(child: Expression) extends UnaryArithmetic ` * `case class MaxOf(left: Expression, right: Expression) extends BinaryArithmetic ` * `case class MinOf(left: Expression, right: Expression) extends BinaryArithmetic ` * `case class Atan2(left: Expression, right: Expression)` * `case class Hypot(left: Expression, right: Expression)` * `case class EqualTo(left: Expression, right: Expression) extends BinaryComparison ` --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-107006901 Merged build finished. Test FAILed. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106782616 [Test build #33733 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33733/consoleFull) for PR 6405 at commit [`e4cb3d8`](https://github.com/apache/spark/commit/e4cb3d84f995ca23214bf3517e3ce03baf569b20). * This patch **fails Spark unit tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `class TypeCheckResult(val errorMessage: String) extends AnyVal ` * `abstract class UnaryArithmetic extends UnaryExpression ` * `case class UnaryMinus(child: Expression) extends UnaryArithmetic ` * `case class Sqrt(child: Expression) extends UnaryArithmetic ` * `case class Abs(child: Expression) extends UnaryArithmetic ` * `case class BitwiseNot(child: Expression) extends UnaryArithmetic ` * `case class MaxOf(left: Expression, right: Expression) extends BinaryArithmetic ` * `case class MinOf(left: Expression, right: Expression) extends BinaryArithmetic ` * `case class Atan2(left: Expression, right: Expression)` * `case class Hypot(left: Expression, right: Expression)` * `case class EqualTo(left: Expression, right: Expression) extends BinaryComparison ` --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106782621 Merged build finished. Test FAILed. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106775049 [Test build #33733 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33733/consoleFull) for PR 6405 at commit [`e4cb3d8`](https://github.com/apache/spark/commit/e4cb3d84f995ca23214bf3517e3ce03baf569b20). --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106753113 Merged build finished. Test FAILed. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106753106 [Test build #33727 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33727/consoleFull) for PR 6405 at commit [`e29f8dc`](https://github.com/apache/spark/commit/e29f8dc01e8a05bee761088f60044983cee04ae5). * This patch **fails Spark unit tests**. * This patch merges cleanly. * This patch adds no public classes. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106755079 [Test build #33725 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33725/consoleFull) for PR 6405 at commit [`3993ae7`](https://github.com/apache/spark/commit/3993ae7f1c97cb74e1367bd96ec949507f1fd748). * This patch **passes all tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `class TypeCheckResult(val errorMessage: String) extends AnyVal ` * `abstract class UnaryArithmetic extends UnaryExpression ` * `case class UnaryMinus(child: Expression) extends UnaryArithmetic ` * `case class Sqrt(child: Expression) extends UnaryArithmetic ` * `case class Abs(child: Expression) extends UnaryArithmetic ` * `case class BitwiseNot(child: Expression) extends UnaryArithmetic ` * `case class MaxOf(left: Expression, right: Expression) extends BinaryArithmetic ` * `case class MinOf(left: Expression, right: Expression) extends BinaryArithmetic ` * `case class Atan2(left: Expression, right: Expression)` * `case class Hypot(left: Expression, right: Expression)` * `case class EqualTo(left: Expression, right: Expression) extends BinaryComparison ` --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106755093 Merged build finished. Test PASSed. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106774288 Merged build triggered. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106774350 Merged build started. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user cloud-fan commented on a diff in the pull request: https://github.com/apache/spark/pull/6405#discussion_r31315174 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala --- @@ -441,8 +406,7 @@ case class CaseKeyWhen(key: Expression, branches: Seq[Expression]) extends CaseW override def children: Seq[Expression] = key +: branches - override lazy val resolved: Boolean = -childrenResolved valueTypesEqual + override protected def checkTypesInternal(): TypeCheckResult = TypeCheckResult.success --- End diff -- Should we ensure that `key` and `when`s must be same type in `CaseKeyWhen`? --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user cloud-fan commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106866062 Hi @rxin , I checked the failed test cases and there is only one case that we do need to check equality for different types: decimal types with different precisions. It seems not worth to remove the type equal constraint from `EqualTo` just for one case, should we cast it to `Decimal.unlimited` like what we did for `LessThan`? --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31372110
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ExpressionTypeCheckingSuite.scala
---
@@ -0,0 +1,131 @@
+/*
+ * 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.sql.AnalysisException
+import org.apache.spark.sql.catalyst.analysis.SimpleAnalyzer
+import org.apache.spark.sql.catalyst.plans.logical.LocalRelation
+import org.apache.spark.sql.catalyst.dsl.plans._
+import org.apache.spark.sql.catalyst.dsl.expressions._
+import org.apache.spark.sql.types.{BooleanType, StringType}
+import org.scalatest.FunSuite
+
+
+class ExpressionTypeCheckingSuite extends FunSuite {
+
+ val testRelation = LocalRelation(
+'intField.int,
+'stringField.string,
+'booleanField.boolean,
+'complexField.array(StringType))
+
+ def assertError(expr: Expression, errorMessage: String): Unit = {
+val e = intercept[AnalysisException] {
+ assertSuccess(expr)
+}
+assert(e.getMessage.contains(
+ scannot resolve '${expr.prettyString}' due to data type mismatch:))
+assert(e.getMessage.contains(errorMessage))
+ }
+
+ def assertSuccess(expr: Expression): Unit = {
+val analyzed = testRelation.select(expr.as(c)).analyze
+SimpleAnalyzer.checkAnalysis(analyzed)
+ }
+
+ def assertErrorForDifferingTypes(expr: Expression): Unit = {
+assertError(expr,
+ sdiffering types in ${expr.getClass.getSimpleName} (IntegerType and
BooleanType).)
+ }
+
+ test(check types for unary arithmetic) {
+assertError(UnaryMinus('stringField), operator - accepts numeric
type)
+assertSuccess(Sqrt('stringField)) // We will cast String to Double for
sqrt
+assertError(Sqrt('booleanField), function sqrt accepts numeric type)
+assertError(Abs('stringField), function abs accepts numeric type)
+assertError(BitwiseNot('stringField), operator ~ accepts integral
type)
+ }
+
+ test(check types for binary arithmetic) {
+// We will cast String to Double for binary arithmetic
+assertSuccess(Add('intField, 'stringField))
+assertSuccess(Subtract('intField, 'stringField))
+assertSuccess(Multiply('intField, 'stringField))
+assertSuccess(Divide('intField, 'stringField))
+assertSuccess(Remainder('intField, 'stringField))
+// checkAnalysis(BitwiseAnd('intField, 'stringField))
--- End diff --
Seems you want a rule in `PromoteStrings` to handle `Bitwise`?
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31371891
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/Optimizer.scala
---
@@ -264,6 +264,10 @@ object NullPropagation extends Rule[LogicalPlan] {
case e @ Substring(_, Literal(null, _), _) = Literal.create(null,
e.dataType)
case e @ Substring(_, _, Literal(null, _)) = Literal.create(null,
e.dataType)
+ // MaxOf and MinOf can't do null propagation
--- End diff --
Seems for `MaxOf` and `MinOf`, `null` does not mean `unknown`. It will be
good to add scala doc for them to explain the semantic.
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106982659 cc @mateiz since he wrote some of the decimal code. @mateiz - can you take a look at @cloud-fan's comment on whether we can always turn decimal comparison into Decimal.unlimited? My thought is that it might be too expensive (since it'd remove your decimal - long optimization). --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user rxin commented on a diff in the pull request: https://github.com/apache/spark/pull/6405#discussion_r31378052 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/predicates.scala --- @@ -441,8 +406,7 @@ case class CaseKeyWhen(key: Expression, branches: Seq[Expression]) extends CaseW override def children: Seq[Expression] = key +: branches - override lazy val resolved: Boolean = -childrenResolved valueTypesEqual + override protected def checkTypesInternal(): TypeCheckResult = TypeCheckResult.success --- End diff -- do we add the cast elsewhere? if we do, then i think it is ok to not check here. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106992479 Merged build triggered. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106992531 [Test build #33797 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/33797/consoleFull) for PR 6405 at commit [`7e144e1`](https://github.com/apache/spark/commit/7e144e1d4adbd7ed023b89c77ed50003bc0952c0). --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106992485 Merged build started. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-106991031 I chatted with @mateiz offline. It seems OK performance wise to cast. But we should look into what the standard thing is to do here. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31378221
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
---
@@ -86,12 +86,18 @@ abstract class Expression extends TreeNode[Expression] {
case (i1, i2) = i1 == i2
}
}
+
+ /**
+ * Check the input data types, returns `TypeCheckResult.success` if it's
valid,
+ * or return a `TypeCheckResult` with an error message if invalid.
+ */
+ def checkInputDataTypes(): TypeCheckResult = TypeCheckResult.success
--- End diff --
Then we need to implement it for a lot of expressions and decide error
message for them in this PR... Can we do it step by step and finally remove the
default implementation?
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31378039
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
---
@@ -86,12 +86,18 @@ abstract class Expression extends TreeNode[Expression] {
case (i1, i2) = i1 == i2
}
}
+
+ /**
+ * Check the input data types, returns `TypeCheckResult.success` if it's
valid,
+ * or return a `TypeCheckResult` with an error message if invalid.
+ */
+ def checkInputDataTypes(): TypeCheckResult = TypeCheckResult.success
--- End diff --
can we remove the default implementation?
---
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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/6405#discussion_r31378030
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCheckResult.scala
---
@@ -0,0 +1,32 @@
+/*
+ * 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.analysis
+
+/**
+ * Represents the result of `Expression.checkInputDataTypes`.
+ * We will throw `AnalysisException` in `CheckAnalysis` if error message
is not null.
+ *
+ */
+class TypeCheckResult(val errorMessage: String) extends AnyVal {
--- End diff --
mark the constructor as private to force callers to use the
object.success/fail factory methods. also update the documentation to say Use
[[TypeCheckResult.success]] and [[TypeCheckResult.fail]] to instantiate 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-105439680 Merged build started. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6405#issuecomment-105439603 Merged build triggered. --- 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: [SPARK-7562][SPARK-6444][SQL] Improve error re...
GitHub user cloud-fan opened a pull request: https://github.com/apache/spark/pull/6405 [SPARK-7562][SPARK-6444][SQL] Improve error reporting for expression data type mismatch You can merge this pull request into a Git repository by running: $ git pull https://github.com/cloud-fan/spark 6444 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/6405.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 #6405 commit bdf3be2ec0cccb89ef345c7882962e7c8029f1a7 Author: Wenchen Fan cloud0...@outlook.com Date: 2015-05-14T17:16:09Z check argument types for SQL functions commit 630c5ed5fb68683157389a5f0f14f50cebe2b036 Author: Wenchen Fan cloud0...@outlook.com Date: 2015-05-14T18:31:47Z style fix commit 67a2a5dcca80398e14e656858ef1ab7efd79c035 Author: Wenchen Fan cloud0...@outlook.com Date: 2015-05-14T19:12:50Z small refactor commit 3cab44d2cb20a71395a61c7305dc94328162d028 Author: Wenchen Fan cloud0...@outlook.com Date: 2015-05-14T19:28:00Z handle NullType correctly commit fd0280b8cfa3ccddd4fa66342fd52f0ab0665eb2 Author: Wenchen Fan cloud0...@outlook.com Date: 2015-05-15T02:09:03Z fix mistake commit 4691f50b4dfcd905e1c304384987d978d7eee35b Author: Wenchen Fan cloud0...@outlook.com Date: 2015-05-15T03:34:46Z add newline commit 6141b1b3b23397355581f8b327e9bb84dbeaae75 Author: Wenchen Fan cloud0...@outlook.com Date: 2015-05-26T07:43:52Z improve error reporting for expression data type mismatch --- 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
