[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/12340#discussion_r59840175
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala
---
@@ -487,6 +487,50 @@ case class PrintToStderr(child: Expression) extends
UnaryExpression {
}
/**
+ * A function throws an exception if 'condition' is not true.
+ */
+@ExpressionDescription(
+ usage = "_FUNC_(condition) - Throw an exception if 'condition' is not
true.")
+case class AssertTrue(child: Expression) extends UnaryExpression {
+
+ override def nullable: Boolean = true
+
+ def dataType: DataType = NullType
+
+ override def prettyName: String = "assert_true"
+
+ override def checkInputDataTypes(): TypeCheckResult = {
+if (child.dataType != BooleanType) {
+ TypeCheckResult.TypeCheckFailure(
+s"type of condition expression in assert_true should be boolean,
not ${child.dataType}")
+} else {
+ TypeCheckResult.TypeCheckSuccess
+}
+ }
+
+ override def eval(input: InternalRow) : Any = {
+val v = child.eval(input)
+if (java.lang.Boolean.TRUE.equals(v)) {
+ null
+} else {
+ throw new RuntimeException(s"'${child.simpleString}' is not true!")
+}
+ }
+
+ override def genCode(ctx: CodegenContext, ev: ExprCode): String = {
+val eval = child.gen(ctx)
+ev.isNull = "true"
+ev.value = "null"
+s"""if (${eval.isNull} ||
!java.lang.Boolean.TRUE.equals(${eval.value})) {
+ | throw new RuntimeException("'${child.simpleString}' is not
true.");
+ |}
+ """.stripMargin
+ }
+
+ override def sql: String = s"assert_true(${child.sql})"
--- End diff --
I thought like that, but It's used in SQL representation like the following.
== Physical Plan ==
WholeStageCodegen
: +- Project [null AS **assert_true(true)**#4]
: +- INPUT
+- Scan OneRowRelation[]
---
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-14580][SQL] Hive IfCoercion should pres...
Github user dongjoon-hyun commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-210346484 Thank you for fast review. I'll update and create the JIRA. --- 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-14580][SQL] Hive IfCoercion should pres...
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-210342856 can you create a new jira ticket for assert_true, and put both in the title? --- 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-14580][SQL] Hive IfCoercion should pres...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12340#discussion_r59838077
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala
---
@@ -487,6 +487,50 @@ case class PrintToStderr(child: Expression) extends
UnaryExpression {
}
/**
+ * A function throws an exception if 'condition' is not true.
+ */
+@ExpressionDescription(
+ usage = "_FUNC_(condition) - Throw an exception if 'condition' is not
true.")
+case class AssertTrue(child: Expression) extends UnaryExpression {
--- End diff --
maybe this should do implicit type cast
---
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-14580][SQL] Hive IfCoercion should pres...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12340#discussion_r59838031
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala
---
@@ -487,6 +487,50 @@ case class PrintToStderr(child: Expression) extends
UnaryExpression {
}
/**
+ * A function throws an exception if 'condition' is not true.
+ */
+@ExpressionDescription(
+ usage = "_FUNC_(condition) - Throw an exception if 'condition' is not
true.")
+case class AssertTrue(child: Expression) extends UnaryExpression {
+
+ override def nullable: Boolean = true
+
+ def dataType: DataType = NullType
+
+ override def prettyName: String = "assert_true"
+
+ override def checkInputDataTypes(): TypeCheckResult = {
+if (child.dataType != BooleanType) {
+ TypeCheckResult.TypeCheckFailure(
+s"type of condition expression in assert_true should be boolean,
not ${child.dataType}")
+} else {
+ TypeCheckResult.TypeCheckSuccess
+}
+ }
+
+ override def eval(input: InternalRow) : Any = {
+val v = child.eval(input)
+if (java.lang.Boolean.TRUE.equals(v)) {
+ null
+} else {
+ throw new RuntimeException(s"'${child.simpleString}' is not true!")
+}
+ }
+
+ override def genCode(ctx: CodegenContext, ev: ExprCode): String = {
+val eval = child.gen(ctx)
+ev.isNull = "true"
+ev.value = "null"
+s"""if (${eval.isNull} ||
!java.lang.Boolean.TRUE.equals(${eval.value})) {
--- End diff --
eval.value is a primitive, so we should just do `|| !${eval.value}`
---
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-14580][SQL] Hive IfCoercion should pres...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12340#discussion_r59837905
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala
---
@@ -487,6 +487,50 @@ case class PrintToStderr(child: Expression) extends
UnaryExpression {
}
/**
+ * A function throws an exception if 'condition' is not true.
+ */
+@ExpressionDescription(
+ usage = "_FUNC_(condition) - Throw an exception if 'condition' is not
true.")
+case class AssertTrue(child: Expression) extends UnaryExpression {
+
+ override def nullable: Boolean = true
+
+ def dataType: DataType = NullType
+
+ override def prettyName: String = "assert_true"
+
+ override def checkInputDataTypes(): TypeCheckResult = {
+if (child.dataType != BooleanType) {
+ TypeCheckResult.TypeCheckFailure(
+s"type of condition expression in assert_true should be boolean,
not ${child.dataType}")
+} else {
+ TypeCheckResult.TypeCheckSuccess
+}
+ }
+
+ override def eval(input: InternalRow) : Any = {
+val v = child.eval(input)
+if (java.lang.Boolean.TRUE.equals(v)) {
+ null
+} else {
+ throw new RuntimeException(s"'${child.simpleString}' is not true!")
+}
+ }
+
+ override def genCode(ctx: CodegenContext, ev: ExprCode): String = {
+val eval = child.gen(ctx)
+ev.isNull = "true"
+ev.value = "null"
+s"""if (${eval.isNull} ||
!java.lang.Boolean.TRUE.equals(${eval.value})) {
+ | throw new RuntimeException("'${child.simpleString}' is not
true.");
+ |}
+ """.stripMargin
+ }
+
+ override def sql: String = s"assert_true(${child.sql})"
--- End diff --
this is not necessary is it?
---
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-14580][SQL] Hive IfCoercion should pres...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-210337909 **[Test build #55907 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55907/consoleFull)** for PR 12340 at commit [`fa3e95a`](https://github.com/apache/spark/commit/fa3e95a11d0ba0d05b468cc26060de5e6688f91e). --- 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-14580][SQL] Hive IfCoercion should pres...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/12340#discussion_r59828732
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/HiveTypeCoercionSuite.scala
---
@@ -348,6 +350,20 @@ class HiveTypeCoercionSuite extends PlanTest {
test("type coercion for If") {
val rule = HiveTypeCoercion.IfCoercion
+
+// SPARK-14580 Hive IfCoercion should preserve predicate
+case class AssertTrue(condition: Boolean) extends LeafExpression {
+ def nullable: Boolean = true
+ def dataType: DataType = NullType
+ def eval(input: InternalRow = null) : Any = {
+if (!condition) throw new Exception("SPARK-14580 TEST")
+Literal.create(null, NullType)
+ }
+ override def genCode(ctx: CodegenContext, ev: ExprCode): String = {
+s"""if (!$condition) throw new Exception("SPARK-14580 TEST");"""
--- End diff --
I'll fix soon.
---
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-14580][SQL] Hive IfCoercion should pres...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/12340#discussion_r59828693
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/HiveTypeCoercionSuite.scala
---
@@ -348,6 +350,20 @@ class HiveTypeCoercionSuite extends PlanTest {
test("type coercion for If") {
val rule = HiveTypeCoercion.IfCoercion
+
+// SPARK-14580 Hive IfCoercion should preserve predicate
+case class AssertTrue(condition: Boolean) extends LeafExpression {
--- End diff --
Right. That'll be much better.
---
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-14580][SQL] Hive IfCoercion should pres...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-210293749 **[Test build #55900 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55900/consoleFull)** for PR 12340 at commit [`f1c2dc6`](https://github.com/apache/spark/commit/f1c2dc6e5fde2392a841eb046342897093c94383). * This patch **fails MiMa tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `case class AssertTrue(condition: Boolean) extends LeafExpression ` --- 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-14580][SQL] Hive IfCoercion should pres...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-210293795 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55900/ 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-14580][SQL] Hive IfCoercion should pres...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-210293788 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-14580][SQL] Hive IfCoercion should pres...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12340#discussion_r59828445
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/HiveTypeCoercionSuite.scala
---
@@ -348,6 +350,20 @@ class HiveTypeCoercionSuite extends PlanTest {
test("type coercion for If") {
val rule = HiveTypeCoercion.IfCoercion
+
+// SPARK-14580 Hive IfCoercion should preserve predicate
+case class AssertTrue(condition: Boolean) extends LeafExpression {
+ def nullable: Boolean = true
+ def dataType: DataType = NullType
+ def eval(input: InternalRow = null) : Any = {
+if (!condition) throw new Exception("SPARK-14580 TEST")
+Literal.create(null, NullType)
+ }
+ override def genCode(ctx: CodegenContext, ev: ExprCode): String = {
+s"""if (!$condition) throw new Exception("SPARK-14580 TEST");"""
--- End diff --
RuntimeException
---
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-14580][SQL] Hive IfCoercion should pres...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12340#discussion_r59828455
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/HiveTypeCoercionSuite.scala
---
@@ -348,6 +350,20 @@ class HiveTypeCoercionSuite extends PlanTest {
test("type coercion for If") {
val rule = HiveTypeCoercion.IfCoercion
+
+// SPARK-14580 Hive IfCoercion should preserve predicate
+case class AssertTrue(condition: Boolean) extends LeafExpression {
+ def nullable: Boolean = true
+ def dataType: DataType = NullType
+ def eval(input: InternalRow = null) : Any = {
+if (!condition) throw new Exception("SPARK-14580 TEST")
+Literal.create(null, NullType)
+ }
+ override def genCode(ctx: CodegenContext, ev: ExprCode): String = {
+s"""if (!$condition) throw new Exception("SPARK-14580 TEST");"""
--- End diff --
and should say the condition in the exception
---
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-14580][SQL] Hive IfCoercion should pres...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12340#discussion_r59828440
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/HiveTypeCoercionSuite.scala
---
@@ -348,6 +350,20 @@ class HiveTypeCoercionSuite extends PlanTest {
test("type coercion for If") {
val rule = HiveTypeCoercion.IfCoercion
+
+// SPARK-14580 Hive IfCoercion should preserve predicate
+case class AssertTrue(condition: Boolean) extends LeafExpression {
--- End diff --
maybe we should have this as an expression in spark sql, and register it so
we don't need to delegate to hive.
---
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-14580][SQL] Hive IfCoercion should pres...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-210292283 **[Test build #55900 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55900/consoleFull)** for PR 12340 at commit [`f1c2dc6`](https://github.com/apache/spark/commit/f1c2dc6e5fde2392a841eb046342897093c94383). --- 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-14580][SQL] Hive IfCoercion should pres...
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210291837
I added the test cases into `HiveTypeCoercionSuite.test("type coercion for
If")`.
Thank you, @rxin. It should be there.
---
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-14580][SQL] Hive IfCoercion should pres...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/12340#discussion_r59826908
--- Diff:
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HivePlanTest.scala
---
@@ -49,4 +49,12 @@ class HivePlanTest extends QueryTest with
TestHiveSingleton {
assert(plan.collect{ case w: logical.Window => w }.size === 1,
"Should have only 1 Window operator.")
}
+
+ test("SPARK-14580 Hive IfCoercion should preserve predicate") {
+val plan = sql("SELECT IF(assert_true(false), 1,
2)").queryExecution.analyzed
--- End diff --
Thanks for tip!
---
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-14580][SQL] Hive IfCoercion should pres...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12340#discussion_r59826549
--- Diff:
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HivePlanTest.scala
---
@@ -49,4 +49,12 @@ class HivePlanTest extends QueryTest with
TestHiveSingleton {
assert(plan.collect{ case w: logical.Window => w }.size === 1,
"Should have only 1 Window operator.")
}
+
+ test("SPARK-14580 Hive IfCoercion should preserve predicate") {
+val plan = sql("SELECT IF(assert_true(false), 1,
2)").queryExecution.analyzed
--- End diff --
yea maybe you should just create an assert_true expression
---
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-14580][SQL] Hive IfCoercion should pres...
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/12340#discussion_r59826449
--- Diff:
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HivePlanTest.scala
---
@@ -49,4 +49,12 @@ class HivePlanTest extends QueryTest with
TestHiveSingleton {
assert(plan.collect{ case w: logical.Window => w }.size === 1,
"Should have only 1 Window operator.")
}
+
+ test("SPARK-14580 Hive IfCoercion should preserve predicate") {
+val plan = sql("SELECT IF(assert_true(false), 1,
2)").queryExecution.analyzed
--- End diff --
Sure, I'll try in `sql/catalyst` first, then `sql/core` module.
---
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-14580][SQL] Hive IfCoercion should pres...
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12340#discussion_r59825935
--- Diff:
sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HivePlanTest.scala
---
@@ -49,4 +49,12 @@ class HivePlanTest extends QueryTest with
TestHiveSingleton {
assert(plan.collect{ case w: logical.Window => w }.size === 1,
"Should have only 1 Window operator.")
}
+
+ test("SPARK-14580 Hive IfCoercion should preserve predicate") {
+val plan = sql("SELECT IF(assert_true(false), 1,
2)").queryExecution.analyzed
--- End diff --
can you create a different test case? in general we are going to remove
hive specific things so i don't think this is the right place to test 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-14580][SQL] Hive IfCoercion should pres...
Github user dongjoon-hyun commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-210282267 Hmm. The following is [GenericUDFAssertTrue](https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFAssertTrue.java) in Hive master branch. It's registered in HIVE-2532 and described in some books, too. However, Wiki page seems not to describe that. I couldn't find so far. --- 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-14580][SQL] Hive IfCoercion should pres...
Github user dongjoon-hyun commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-21028 It's Hive built-in function. The following is function description. ``` hive> desc function assert_true; OK assert_true(condition) - Throw an exception if 'condition' is not true. Time taken: 0.022 seconds, Fetched: 1 row(s) ``` Let me find some documentation more. By the way, when I make a UDF return NULL, it shows the same behavior. --- 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-14580][SQL] Hive IfCoercion should pres...
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-210276359 What is assert_true? I didnt' find its documentation anywhere. --- 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-14580][SQL] Hive IfCoercion should pres...
Github user dongjoon-hyun commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-210087678 Hi, @rxin . This is a PR to fix `HiveTypeCoercion.IfCoercion` behavior. I think you're the best person to review this since it was originally written by you. So, I ask again. Could you take a look at this 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-14580][SQL] Hive IfCoercion should pres...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-209193831 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-14580][SQL] Hive IfCoercion should pres...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-209193832 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55671/ 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-14580][SQL] Hive IfCoercion should pres...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-209193710 **[Test build #55671 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55671/consoleFull)** for PR 12340 at commit [`a5f1daf`](https://github.com/apache/spark/commit/a5f1daf6ba8d2a51db92e083091dc5b30fa1a646). * This patch passes all 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-14580][SQL] Hive IfCoercion should pres...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-209164856 **[Test build #55671 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55671/consoleFull)** for PR 12340 at commit [`a5f1daf`](https://github.com/apache/spark/commit/a5f1daf6ba8d2a51db92e083091dc5b30fa1a646). --- 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-14580][SQL] Hive IfCoercion should pres...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-209157468 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-14580][SQL] Hive IfCoercion should pres...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-209157469 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55657/ 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-14580][SQL] Hive IfCoercion should pres...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-209157343 **[Test build #55657 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55657/consoleFull)** for PR 12340 at commit [`6271515`](https://github.com/apache/spark/commit/627151515ae772fbf7be5107ab9d7f352abe1436). * 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-14580][SQL] Hive IfCoercion should pres...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-209133718 **[Test build #55657 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55657/consoleFull)** for PR 12340 at commit [`6271515`](https://github.com/apache/spark/commit/627151515ae772fbf7be5107ab9d7f352abe1436). --- 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-14580][SQL] Hive IfCoercion should pres...
Github user dongjoon-hyun commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-209133359 Rebased to trigger Jenkins again. --- 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-14580][SQL] Hive IfCoercion should pres...
Github user dongjoon-hyun commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-209132312 Amazing. There must be some problem in Jenkins server. It's JVM error. ``` Java HotSpot(TM) 64-Bit Server VM warning: ignoring option MaxPermSize=512m; support was removed in 8.0 [info] Loading project definition from /home/jenkins/workspace/SparkPullRequestBuilder/project/project [CodeBlob (0x7f1704216390)] Framesize: 2 Runtime Stub (0x7f1704216390): handle_exception_from_callee Runtime1 stub Could not load hsdis-amd64.so; library not loadable; PrintAssembly is disabled ``` --- 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-14580][SQL] Hive IfCoercion should pres...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-209130627 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55655/ 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-14580][SQL] Hive IfCoercion should pres...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-209130615 **[Test build #55655 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55655/consoleFull)** for PR 12340 at commit [`938334a`](https://github.com/apache/spark/commit/938334ac236f1854da95a756ae0c97c15cb70b3c). * This patch **fails to build**. * 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-14580][SQL] Hive IfCoercion should pres...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-209130625 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-14580][SQL] Hive IfCoercion should pres...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-209129813 **[Test build #55655 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55655/consoleFull)** for PR 12340 at commit [`938334a`](https://github.com/apache/spark/commit/938334ac236f1854da95a756ae0c97c15cb70b3c). --- 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-14580][SQL] Hive IfCoercion should pres...
Github user dongjoon-hyun commented on the pull request: https://github.com/apache/spark/pull/12340#issuecomment-209128788 Hi, @rxin . Could you review this PR? --- 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-14580][SQL] Hive IfCoercion should pres...
GitHub user dongjoon-hyun opened a pull request:
https://github.com/apache/spark/pull/12340
[SPARK-14580][SQL] Hive IfCoercion should preserve predicate.
## What changes were proposed in this pull request?
Currently, `HiveTypeCoercion.IfCoercion` removes all predicates whose
return-type are null. However, some UDFs need evaluations because they are
designed to throw exceptions.
**Hive**
```
hive> select if(assert_true(false),2,3);
OK
Failed with exception
java.io.IOException:org.apache.hadoop.hive.ql.metadata.HiveException:
ASSERT_TRUE(): assertion failed.
```
This PR fixes that to preserve the predicates.
**Before**
```
scala> sql("select if(assert_true(false),2,3)").head
res2: org.apache.spark.sql.Row = [3]
```
**After**
```
scala> sql("select if(assert_true(false),2,3)").head
... ASSERT_TRUE ...
```
## How was this patch tested?
Pass the Jenkins tests (including a new testcase in `HivePlanTest`)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dongjoon-hyun/spark SPARK-14580
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/12340.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 #12340
commit 938334ac236f1854da95a756ae0c97c15cb70b3c
Author: Dongjoon Hyun
Date: 2016-04-12T17:23:09Z
[SPARK-14580][SQL] Hive IfCoercion should preserve predicate.
```
hive> select if(assert_true(false),2,3);
OK
Failed with exception
java.io.IOException:org.apache.hadoop.hive.ql.metadata.HiveException:
ASSERT_TRUE(): assertion failed.
```
**Before**
```
scala> sql("select if(assert_true(false),2,3)").head
res2: org.apache.spark.sql.Row = [3]
```
**After**
```
scala> sql("select if(assert_true(false),2,3)").head
... ASSERT_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
