You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by dongjoon-hyun <gi...@git.apache.org> on 2016/04/13 00:28:31 UTC
[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 <do...@apache.org>
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-211538395
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/56084/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-211548789
Let me fix that.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210373329
**[Test build #55907 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55907/consoleFull)** for PR 12340 at commit [`fa3e95a`](https://github.com/apache/spark/commit/fa3e95a11d0ba0d05b468cc26060de5e6688f91e).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class AssertTrue(child: Expression) extends UnaryExpression `
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210414400
**[Test build #55915 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55915/consoleFull)** for PR 12340 at commit [`bdcef78`](https://github.com/apache/spark/commit/bdcef787efcb0faff56d14f89d260773056cafec).
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-211538077
**[Test build #56084 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56084/consoleFull)** for PR 12340 at commit [`f73b18e`](https://github.com/apache/spark/commit/f73b18ebb623c96b4666c207af7eab2212323d95).
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by SparkQA <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by AmplabJenkins <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by dongjoon-hyun <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by AmplabJenkins <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210414562
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by SparkQA <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by AmplabJenkins <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210877367
Hi, @rxin
Does the latest code fit your intention?
If I missed something again, please let me know.
Thank you for review so far. I learned a little bit more about how Spark embrace Hive things.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-211487788
**[Test build #56084 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56084/consoleFull)** for PR 12340 at commit [`f73b18e`](https://github.com/apache/spark/commit/f73b18ebb623c96b4666c207af7eab2212323d95).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by AmplabJenkins <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12340#discussion_r59961811
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala ---
@@ -487,6 +487,43 @@ 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 (false, 0, null, '').")
+case class AssertTrue(child: Expression) extends UnaryExpression {
+
+ override def nullable: Boolean = true
+
+ def dataType: DataType = NullType
--- End diff --
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by rxin <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by rxin <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by SparkQA <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/12340#discussion_r60096468
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala ---
@@ -487,6 +487,44 @@ 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 with ImplicitCastInputTypes {
+
+ override def nullable: Boolean = true
+
+ override def inputTypes: Seq[DataType] = Seq(BooleanType)
+
+ override def dataType: DataType = NullType
+
+ override def prettyName: String = "assert_true"
+
+ override def eval(input: InternalRow) : Any = {
+ val v = child.eval(input)
+ if (v == null || java.lang.Boolean.FALSE.equals(v)) {
+ throw new RuntimeException(s"'${child.simpleString}' is not true!")
+ } else {
+ null
+ }
+ }
+
+ override def genCode(ctx: CodegenContext, ev: ExprCode): String = {
+ val eval = child.gen(ctx)
+ ev.isNull = "true"
+ ev.value = "null"
+ s"""${eval.code}
+ |if (${eval.isNull} || java.lang.Boolean.FALSE.equals("${eval.value}")) {
--- End diff --
That was due to my bads. I made my testcases by using only a primitive value. So, it works.
When I added 'cast(...)' expression as a child, I could find that.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by dongjoon-hyun <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210592040
Thanks to you, I changed the followings so far.
* Move testcase into `HiveTypeCoercionSuite` in sql module.
* Create [SPARK-14655](https://issues.apache.org/jira/browse/SPARK-14655) for `assert_true` and implement `AssertTrue` expression.
* Use `RuntimeException` describing condition.
* Fix codeGen.
* Remove `checkInputDataTypes` and handle false, 0, “”, and null as FALSE condition.
Thank you.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by dongjoon-hyun <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by SparkQA <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by SparkQA <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by AmplabJenkins <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210280000
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by rxin <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by AmplabJenkins <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210785244
**[Test build #56005 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56005/consoleFull)** for PR 12340 at commit [`8bb7f81`](https://github.com/apache/spark/commit/8bb7f814ddbb76fe7fc745d15c00ec2af15d2e68).
* 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by dongjoon-hyun <gi...@git.apache.org>.
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 (0x00007f1704216390)]
Framesize: 2
Runtime Stub (0x00007f1704216390): 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by AmplabJenkins <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-211550949
Oops. Sorry 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by rxin <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-211538667
Thanks - merging in master.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210762099
I see. I'll use `with ImplicitCastInputTypes`.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by dongjoon-hyun <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by rxin <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210414566
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55915/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210682394
Hi, @rxin .
Do you think we need to implement `assert_true` with different behavior in Spark?
If then, please let me know. I'll change that.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210775563
Now, `assert_true` uses implicit type casts. Code is much cleaner.
Also, during reviews, I fixed my critical bug: I missed adding generated child code.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by dongjoon-hyun <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12340#discussion_r59979739
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala ---
@@ -487,6 +487,44 @@ 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 with ImplicitCastInputTypes {
+
+ override def nullable: Boolean = true
+
+ override def inputTypes: Seq[DataType] = Seq(BooleanType)
+
+ override def dataType: DataType = NullType
+
+ override def prettyName: String = "assert_true"
+
+ override def eval(input: InternalRow) : Any = {
+ val v = child.eval(input)
+ if (v == null || java.lang.Boolean.FALSE.equals(v)) {
+ throw new RuntimeException(s"'${child.simpleString}' is not true!")
+ } else {
+ null
+ }
+ }
+
+ override def genCode(ctx: CodegenContext, ev: ExprCode): String = {
+ val eval = child.gen(ctx)
+ ev.isNull = "true"
+ ev.value = "null"
+ s"""${eval.code}
+ |if (${eval.isNull} || java.lang.Boolean.FALSE.equals("${eval.value}")) {
--- End diff --
shouldn't this be
```scala
if (${eval.isNull} || !${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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by dongjoon-hyun <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by SparkQA <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210785314
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/12340#discussion_r60098395
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala ---
@@ -487,6 +487,44 @@ 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 with ImplicitCastInputTypes {
+
+ override def nullable: Boolean = true
+
+ override def inputTypes: Seq[DataType] = Seq(BooleanType)
+
+ override def dataType: DataType = NullType
+
+ override def prettyName: String = "assert_true"
+
+ override def eval(input: InternalRow) : Any = {
+ val v = child.eval(input)
+ if (v == null || java.lang.Boolean.FALSE.equals(v)) {
+ throw new RuntimeException(s"'${child.simpleString}' is not true!")
+ } else {
+ null
+ }
+ }
+
+ override def genCode(ctx: CodegenContext, ev: ExprCode): String = {
+ val eval = child.gen(ctx)
+ ev.isNull = "true"
+ ev.value = "null"
+ s"""${eval.code}
+ |if (${eval.isNull} || java.lang.Boolean.FALSE.equals("${eval.value}")) {
--- End diff --
And, I fixed the redundant boolean comparision as you told.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12340#discussion_r59979748
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/misc.scala ---
@@ -487,6 +487,44 @@ 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 with ImplicitCastInputTypes {
+
+ override def nullable: Boolean = true
+
+ override def inputTypes: Seq[DataType] = Seq(BooleanType)
+
+ override def dataType: DataType = NullType
+
+ override def prettyName: String = "assert_true"
+
+ override def eval(input: InternalRow) : Any = {
+ val v = child.eval(input)
+ if (v == null || java.lang.Boolean.FALSE.equals(v)) {
+ throw new RuntimeException(s"'${child.simpleString}' is not true!")
+ } else {
+ null
+ }
+ }
+
+ override def genCode(ctx: CodegenContext, ev: ExprCode): String = {
+ val eval = child.gen(ctx)
+ ev.isNull = "true"
+ ev.value = "null"
+ s"""${eval.code}
+ |if (${eval.isNull} || java.lang.Boolean.FALSE.equals("${eval.value}")) {
--- End diff --
can you look into why the test cases passed given the codegen i think is wrong? did i miss something?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210775598
**[Test build #56005 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/56005/consoleFull)** for PR 12340 at commit [`8bb7f81`](https://github.com/apache/spark/commit/8bb7f814ddbb76fe7fc745d15c00ec2af15d2e68).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-211538391
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210609296
It's because that is the real behavior of `assert_null`. When I used implicit type cast yesterday. It works differently with Hive assert_null. For example,
```
hive> select assert_true(1);
OK
NULL
hive> select assert_true(0);
OK
Failed with exception java.io.IOException:org.apache.hadoop.hive.ql.metadata.HiveException: ASSERT_TRUE(): assertion failed.
Time taken: 0.062 seconds
```
I didn't have the old code, but maybe what you mean is the following, right?
```
hive> select not(0);
FAILED: ClassCastException org.apache.hadoop.hive.serde2.objectinspector.primitive.WritableConstantIntObjectInspector cannot be cast to org.apache.hadoop.hive.serde2.objectinspector.primitive.BooleanObjectInspector
```
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210594102
Actually @dongjoon-hyun why wouldn't you just use the implicit type cast to boolean type?
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210373814
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55907/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210785315
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/56005/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210593850
Thanks - merging in master.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by rxin <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210383165
**[Test build #55915 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55915/consoleFull)** for PR 12340 at commit [`bdcef78`](https://github.com/apache/spark/commit/bdcef787efcb0faff56d14f89d260773056cafec).
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by dongjoon-hyun <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by dongjoon-hyun <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by SparkQA <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by rxin <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by rxin <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by dongjoon-hyun <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by dongjoon-hyun <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by dongjoon-hyun <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210373810
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by SparkQA <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210729011
Yea I'd say we just support boolean type here for now. If people run into issue with this we can add other things in the future. I'd much rather have a simpler implementation for esoteric features.
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by AmplabJenkins <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-210610347
In Spark,
```
scala> sql("select not(0)").head
org.apache.spark.sql.AnalysisException: cannot resolve '(NOT 0)' due to data type mismatch: argument 1 requires boolean type, however, '1' is of int type.; line 1 pos 7
```
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-211548275
@dongjoon-hyun Seems this breaks scala 2.10 test? Can you take a look? Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by rxin <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by SparkQA <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SQL] Hive IfCoercion should pres...
Posted by rxin <gi...@git.apache.org>.
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/12340
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14580][SPARK-14655][SQL] Hive IfCoercio...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12340#issuecomment-211540230
Thank you so much for your reviewing and merging, @rxin !
---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org