[GitHub] spark issue #20085: [SPARK-22739][Catalyst][WIP] Additional Expression Suppo...

2018-05-16 Thread bdrillard
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...

2018-02-08 Thread cloud-fan
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...

2018-01-18 Thread AmplabJenkins
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...

2018-01-18 Thread AmplabJenkins
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...

2018-01-14 Thread marmbrus
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...

2018-01-13 Thread viirya
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...

2018-01-13 Thread sameeragarwal
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...

2018-01-03 Thread bdrillard
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...

2018-01-03 Thread AmplabJenkins
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...

2018-01-03 Thread SparkQA
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...

2018-01-03 Thread AmplabJenkins
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...

2018-01-03 Thread SparkQA
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...

2018-01-03 Thread sameeragarwal
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...

2018-01-03 Thread marmbrus
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...

2017-12-27 Thread bdrillard
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...

2017-12-26 Thread AmplabJenkins
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...

2017-12-26 Thread bdrillard
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