[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user ueshin commented on the issue: https://github.com/apache/spark/pull/22131 Thanks! I'd use this one. merging to master. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user mgaido91 commented on the issue: https://github.com/apache/spark/pull/22131 I preferred the previous one (but yes, definitely good to add the check about the args size), but I am fine with this one too. Thanks @ueshin --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94919/ Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/22131 **[Test build #94919 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94919/testReport)** for PR 22131 at commit [`6f9660d`](https://github.com/apache/spark/commit/6f9660d79e2ae8b7c64dbfea850c514ad3404f37). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/22131 **[Test build #94919 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94919/testReport)** for PR 22131 at commit [`6f9660d`](https://github.com/apache/spark/commit/6f9660d79e2ae8b7c64dbfea850c514ad3404f37). --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 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/2295/ Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user ueshin commented on the issue: https://github.com/apache/spark/pull/22131 @mgaido91 @mn-mikke On second thought, how about this? If you don't like it, I'll revert it soon. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94891/ Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/22131 **[Test build #94891 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94891/testReport)** for PR 22131 at commit [`8a844b7`](https://github.com/apache/spark/commit/8a844b7cd661bc23df05372d5559fa91dff35c90). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94888/ Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/22131 **[Test build #94888 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94888/testReport)** for PR 22131 at commit [`22c3f48`](https://github.com/apache/spark/commit/22c3f484f611d75f4fd06ca9178fb0eb16d69692). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94887/ Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/22131 **[Test build #94887 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94887/testReport)** for PR 22131 at commit [`dec6978`](https://github.com/apache/spark/commit/dec6978afe874d556be786e1c81e02a413440733). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user mn-mikke commented on the issue: https://github.com/apache/spark/pull/22131 LGTM too --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user mgaido91 commented on the issue: https://github.com/apache/spark/pull/22131 LGTM --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user mgaido91 commented on the issue: https://github.com/apache/spark/pull/22131 yes, this is my worry too... But I don't have any suggestion for that If you have anyone, that's great :) --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/22131 **[Test build #94891 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94891/testReport)** for PR 22131 at commit [`8a844b7`](https://github.com/apache/spark/commit/8a844b7cd661bc23df05372d5559fa91dff35c90). --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 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/2275/ Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user ueshin commented on the issue: https://github.com/apache/spark/pull/22131 Sounds good. I just hope we will never miss to wrap new functions with it. Thanks! --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/94883/ Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/22131 **[Test build #94883 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94883/testReport)** for PR 22131 at commit [`8154bf4`](https://github.com/apache/spark/commit/8154bf49d6cd14a681f037b1122024a1390f626e). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 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/2274/ Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/22131
sorry, I wasn't very specific in my previous comment. I meant adding a
method like:
```
def validateBinding(e: HigherOrderFunction): Expression = {
e.bind { case (f: LambdaFunction, dataTypes) =>
f.arguments.zip(dataTypes).foreach {
case (arg, (dt, n)) =>
assert(arg.dataType == dt)
assert(arg.nullable == n)
}
f
}
}
```
and then validate all the expressions, so use it like:
```
validateBinding(ArrayTransform())
```
What do you think about this?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/22131 **[Test build #94888 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94888/testReport)** for PR 22131 at commit [`22c3f48`](https://github.com/apache/spark/commit/22c3f484f611d75f4fd06ca9178fb0eb16d69692). --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/22131 **[Test build #94887 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94887/testReport)** for PR 22131 at commit [`dec6978`](https://github.com/apache/spark/commit/dec6978afe874d556be786e1c81e02a413440733). --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 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/2272/ Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user mgaido91 commented on the issue: https://github.com/apache/spark/pull/22131 I see now, thanks. The only thing which I am a bit concerned about is the complexity introduced by this change, in the sense that it is relying very much on the `bind` implementation of each expression. I think it would be great if we could generalize it. Can we add a generic function similar to the `createLambda` in the analyzer for this? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user ueshin commented on the issue: https://github.com/apache/spark/pull/22131 Actually one of my motivations is to prevent a mistake like #22126. To create a test, we needed to do the same thing in `bind`. The other is I wanted to check the exact values each function passes to bind function as in the description, which we can't know with the end-to-end tests. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user mgaido91 commented on the issue: https://github.com/apache/spark/pull/22131 I am not sure about this. I think that the `bind` method itself is tested with the end-to-end tests which have been added. This complicates quite this part which is meant only to test the execution of the functions. I don't see much value added by this tests, but I may miss something. Can you enlighten me @ueshin ? Thanks. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/22131 **[Test build #94883 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/94883/testReport)** for PR 22131 at commit [`8154bf4`](https://github.com/apache/spark/commit/8154bf49d6cd14a681f037b1122024a1390f626e). --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user ueshin commented on the issue: https://github.com/apache/spark/pull/22131 cc @mn-mikke @mgaido91 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 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/2268/ Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #22131: [SPARK-25141][SQL][TEST] Modify tests for higher-order f...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/22131 Merged build finished. Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
