[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21848 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional comma

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21848 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/1241/

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread SparkQA
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/21848 **[Test build #93454 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93454/testReport)** for PR 21848 at commit [`30ca8cf`](https://github.com/apache/spark/commit/30

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread viirya
Github user viirya commented on the issue: https://github.com/apache/spark/pull/21848 LGTM --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread dongjoon-hyun
Github user dongjoon-hyun commented on the issue: https://github.com/apache/spark/pull/21848 Since this skips the evaluation of `if` condition, this will cause the following difference. **This PR** ``` scala> sql("select * from t").show ++ | a| ++

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread viirya
Github user viirya commented on the issue: https://github.com/apache/spark/pull/21848 This is a good point. On Mon, Jul 23, 2018, 12:03 PM Dongjoon Hyun wrote: > Since this skips the evaluation of if condition, this will cause the > following difference. >

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread SparkQA
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/21848 **[Test build #93454 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93454/testReport)** for PR 21848 at commit [`30ca8cf`](https://github.com/apache/spark/commit/3

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21848 Merged build finished. Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional comma

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21848 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/93454/ Test FAILed. ---

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread viirya
Github user viirya commented on the issue: https://github.com/apache/spark/pull/21848 For now, seems we don't have a good way to know if an expression has side effect. Some expressions like `AssertTrue` should be marked as one with side effect. Maybe we should create a trait for this

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread dbtsai
Github user dbtsai commented on the issue: https://github.com/apache/spark/pull/21848 @gatorsmile this can remove some of the expensive condition expressions, so I would like to find a way to properly implement this. Thank you all for chiming in with many good points. Let me

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread gatorsmile
Github user gatorsmile commented on the issue: https://github.com/apache/spark/pull/21848 Currently, we are setting the expressions `deterministic ` to false when they are either having side effect or non-deterministic. We already did it for Hive UDFs who have stateful tags.

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread dbtsai
Github user dbtsai commented on the issue: https://github.com/apache/spark/pull/21848 This will simply the scope of this PR a lot by just having both `AssertTrue` and `AssertNotNull` as `non-deterministic` expression. My concern is the more `non-deterministic` expressions we have, th

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21848 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional comma

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21848 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/1253/

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread SparkQA
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/21848 **[Test build #93471 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93471/testReport)** for PR 21848 at commit [`bf0b2d9`](https://github.com/apache/spark/commit/bf

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21848 Merged build finished. Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional comma

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread SparkQA
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/21848 **[Test build #93471 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93471/testReport)** for PR 21848 at commit [`bf0b2d9`](https://github.com/apache/spark/commit/b

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21848 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/93471/ Test FAILed. ---

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-23 Thread viirya
Github user viirya commented on the issue: https://github.com/apache/spark/pull/21848 Hmm, seems we have limitation on where non deterministic expressions can be in. --- - To unsubscribe, e-mail: reviews-unsubscr...

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-24 Thread kiszk
Github user kiszk commented on the issue: https://github.com/apache/spark/pull/21848 @dbtsai I have a question. How does the current code check the following condition? > Stateful expression must have a side effect. --- ---

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-24 Thread SparkQA
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/21848 **[Test build #93523 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93523/testReport)** for PR 21848 at commit [`b4f1431`](https://github.com/apache/spark/commit/b4

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-24 Thread dbtsai
Github user dbtsai commented on the issue: https://github.com/apache/spark/pull/21848 @kiszk `trait Stateful extends Nondeterministic`, and this rule will not be invoked when an expression is nondeterministic. --- -

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-24 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21848 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/1292/

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-24 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21848 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional comma

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-24 Thread dbtsai
Github user dbtsai commented on the issue: https://github.com/apache/spark/pull/21848 Here is a followup PR for making `AssertTrue` and `AssertNotNull` `non-deterministic` https://issues.apache.org/jira/browse/SPARK-24913 --- -

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-24 Thread SparkQA
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/21848 **[Test build #93523 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/93523/testReport)** for PR 21848 at commit [`b4f1431`](https://github.com/apache/spark/commit/b

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-24 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21848 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional comma

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-24 Thread AmplabJenkins
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21848 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/93523/ Test PASSed. ---

[GitHub] spark issue #21848: [SPARK-24890] [SQL] Short circuiting the `if` condition ...

2018-07-24 Thread gatorsmile
Github user gatorsmile commented on the issue: https://github.com/apache/spark/pull/21848 LGTM Thanks! Merged to master. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional comm