[GitHub] spark issue #20085: [SPARK-22739][Catalyst][WIP] Additional Expression Suppo...
Github user bdrillard commented on the issue: https://github.com/apache/spark/pull/20085 Closing this PR in favor of #21348. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20085: [SPARK-22739][Catalyst][WIP] Additional Expression Suppo...
Github user cloud-fan commented on the issue: https://github.com/apache/spark/pull/20085 Hi @bdrillard , sorry for the late reply, as I was thinking hard about this problem. I think we all agree that we should have more object-related expressions, so that it's more flexible for Spark and other projects to do many things with the codegen-able expressions. However we should think hard about what object-related expressions Spark should provide, and make sure they are orthogonal and composable. I'm OK with most of the expressions you added, but have some other thoughts about `InitializeObject`. I propose to improve the existing `NewObject`, and introduce a new phase: initialize phase. So `NewObject` should have a list of expression as its constructor parameters, and another list of expressions as post-hoc initializaion. To create a case class, the construct parameters expressions are not empty and initializing expressions are empty. To create a java bean, it's the opposite. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20085: [SPARK-22739][Catalyst][WIP] Additional Expression Suppo...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20085 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 #20085: [SPARK-22739][Catalyst][WIP] Additional Expression Suppo...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20085 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 #20085: [SPARK-22739][Catalyst][WIP] Additional Expression Suppo...
Github user marmbrus commented on the issue: https://github.com/apache/spark/pull/20085 This blocks better support for encoders on spark-avro, and seems safe, so I'd really like to include it in possible. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20085: [SPARK-22739][Catalyst][WIP] Additional Expression Suppo...
Github user viirya commented on the issue: https://github.com/apache/spark/pull/20085 Seems to me this doesn't need to be urgent to be in 2.3. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20085: [SPARK-22739][Catalyst][WIP] Additional Expression Suppo...
Github user sameeragarwal commented on the issue: https://github.com/apache/spark/pull/20085 @bdrillard @viirya @cloud-fan are we still targeting this for 2.3? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20085: [SPARK-22739][Catalyst][WIP] Additional Expression Suppo...
Github user bdrillard commented on the issue: https://github.com/apache/spark/pull/20085 I've added some comments describing an issue I've had with generalizing `InitializeJavaBean`, which I thought I'd added to this PR earlier but seem to have not been submitted. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20085: [SPARK-22739][Catalyst][WIP] Additional Expression Suppo...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20085 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 #20085: [SPARK-22739][Catalyst][WIP] Additional Expression Suppo...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/20085 **[Test build #85642 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85642/testReport)** for PR 20085 at commit [`4b07b66`](https://github.com/apache/spark/commit/4b07b6639ef08f0ba7560c7027c1dbdae8e2f090). * This patch **fails RAT tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `case class InstanceOf(` --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20085: [SPARK-22739][Catalyst][WIP] Additional Expression Suppo...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20085 Test FAILed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85642/ Test FAILed. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20085: [SPARK-22739][Catalyst][WIP] Additional Expression Suppo...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/20085 **[Test build #85642 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85642/testReport)** for PR 20085 at commit [`4b07b66`](https://github.com/apache/spark/commit/4b07b6639ef08f0ba7560c7027c1dbdae8e2f090). --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20085: [SPARK-22739][Catalyst][WIP] Additional Expression Suppo...
Github user sameeragarwal commented on the issue: https://github.com/apache/spark/pull/20085 jenkins add to whitelist --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20085: [SPARK-22739][Catalyst][WIP] Additional Expression Suppo...
Github user marmbrus commented on the issue: https://github.com/apache/spark/pull/20085 /cc @cloud-fan @sameeragarwal --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20085: [SPARK-22739][Catalyst][WIP] Additional Expression Suppo...
Github user bdrillard commented on the issue: https://github.com/apache/spark/pull/20085 @viirya I've found the same intent of a `ValueIfType` function can be attained by adding a simpler `InstanceOf` [expressions](https://github.com/apache/spark/pull/20085/files#diff-e436c96ea839dfe446837ab2a3531f93R265) that can be used as the predicate to the existing `If` expression, and then using `ObjectCast` on the results. That approach handles your [first question](https://github.com/apache/spark/pull/20085#discussion_r158760292). To your [second question](https://github.com/apache/spark/pull/20085#discussion_r158760302), it makes sense the input value expression should always have a DataType of ObjectType. Is there a way you'd prefer to make that check? Or throw some kind of exception of `value.dataType != ObjectType`? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #20085: [SPARK-22739][Catalyst][WIP] Additional Expression Suppo...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/20085 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 #20085: [SPARK-22739][Catalyst][WIP] Additional Expression Suppo...
Github user bdrillard commented on the issue: https://github.com/apache/spark/pull/20085 cc: @marmbrus --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org