[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user HeartSaVioR commented on the issue: https://github.com/apache/spark/pull/21388 I just provided new patch to remove the comment, as it looks like no longer preferred option. https://github.com/apache/spark/pull/21595 Closing this one. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user HeartSaVioR commented on the issue: https://github.com/apache/spark/pull/21388 @hvanhovell To be honest, I found the rationalization of the issue from a comment in Spark code: https://github.com/apache/spark/blob/4c388bccf1bcac8f833fd9214096dd164c3ea065/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala#L496-L497 and I thought the comment makes sense: it would be beneficial if we just couple matching pair of (LogicalPlan, SparkPlan) for the cases which don't require some transformations while transforming. For the first time, I tried my best to stick with compile-time things, but realized it is not possible to achieve without runtime reflection (at least for me) after couple of hours. So another couple of hours were spent on resolving. I have no strong opinion to adopt reflection on planner (so happy to see the approach got rejected), but if we agree it cannot be handled without reflection, the origin comment should be removed, or describing limitations on addressing it so that others might try out with avoiding limitations. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user hvanhovell commented on the issue: https://github.com/apache/spark/pull/21388 @HeartSaVioR I really don't think we should automate these things at all. The planner is a pretty critical component, and I'd rather be explicit on how a `LogicalPlan` maps to a `SparkPlan` and have the benefit of compile time checks, then have some reflection glue doing this at runtime (which can potentially blow up). What is the problem you are trying to solve here? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user HyukjinKwon commented on the issue: https://github.com/apache/spark/pull/21388 ok to test --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21388 Can one of the admins verify this patch? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user HeartSaVioR commented on the issue: https://github.com/apache/spark/pull/21388 @hvanhovell I also think someone might not want to have reflection magic (I was the one but realized I should do it), so I'm happy to close the PR when others voice same opinion on this too. For me, reflection looks like only way to achieve `Can we automate these 'pass through' operations?`, so if we decide to reject the approach, we might be better to either remove the line, or add description on restriction(s) instead, unless we have another immediate idea to achieve it without reflection. Btw, I'd be very happy if you are happy to spend some time to explain which points make you being concerned about reflection in planner. Maybe adding the description explicitly would avoid the similar trial on contributors and save our time. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21388 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90952/ Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21388 Merged build finished. Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/21388 **[Test build #90952 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90952/testReport)** for PR 21388 at commit [`6e6c375`](https://github.com/apache/spark/commit/6e6c375a1c8846e9421d873dd27a880698464c8f). * This patch **fails Spark unit 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 #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user hvanhovell commented on the issue: https://github.com/apache/spark/pull/21388 I donât think it is a good a idea to put reflection magic in the planner. If you want to add cases to the planner please use the existing hooks (SparkSessionExtensions, ExperimentalMethods or override SparkSessionBuilder). -1 on this PR. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/21388 **[Test build #90952 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90952/testReport)** for PR 21388 at commit [`6e6c375`](https://github.com/apache/spark/commit/6e6c375a1c8846e9421d873dd27a880698464c8f). --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user HeartSaVioR commented on the issue: https://github.com/apache/spark/pull/21388 retest this please --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21388 Merged build finished. Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21388 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90946/ Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/21388 **[Test build #90946 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90946/testReport)** for PR 21388 at commit [`6e6c375`](https://github.com/apache/spark/commit/6e6c375a1c8846e9421d873dd27a880698464c8f). * This patch **fails Spark unit 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 #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user HeartSaVioR commented on the issue: https://github.com/apache/spark/pull/21388 Thanks @HyukjinKwon for reviewing. Addressed review comments. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/21388 **[Test build #90946 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90946/testReport)** for PR 21388 at commit [`6e6c375`](https://github.com/apache/spark/commit/6e6c375a1c8846e9421d873dd27a880698464c8f). --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21388 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 #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/21388 **[Test build #90922 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90922/testReport)** for PR 21388 at commit [`139aefa`](https://github.com/apache/spark/commit/139aefa4604c2cbaacae884499f321be0e1324c8). * 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 #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21388 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90922/ Test PASSed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21388 Merged build finished. Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21388 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90924/ Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/21388 **[Test build #90924 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90924/testReport)** for PR 21388 at commit [`971abb6`](https://github.com/apache/spark/commit/971abb6caf79b9e24c063f6e4bd3d0a6aefe133e). * This patch **fails Spark unit 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 #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/21388 **[Test build #90924 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90924/testReport)** for PR 21388 at commit [`971abb6`](https://github.com/apache/spark/commit/971abb6caf79b9e24c063f6e4bd3d0a6aefe133e). --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/21388 **[Test build #90922 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90922/testReport)** for PR 21388 at commit [`139aefa`](https://github.com/apache/spark/commit/139aefa4604c2cbaacae884499f321be0e1324c8). --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #21388: [SPARK-24336][SQL] Support 'pass through' transformation...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/21388 Can one of the admins verify this patch? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
