You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by dbtsai <gi...@git.apache.org> on 2018/05/23 20:45:33 UTC

[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame AP...

GitHub user dbtsai opened a pull request:

    https://github.com/apache/spark/pull/21416

    [SPARK-24371] [SQL] Added isinSet in DataFrame API for Scala and Java.

    ## What changes were proposed in this pull request?
    
    Implemented **`isinSet`** in DataFrame API for both Scala and Java, so users can do
    
    ```scala
    val profileDF = Seq(
      Some(1), Some(2), Some(3), Some(4),
      Some(5), Some(6), Some(7), None
    ).toDF("profileID")
    
    val validUsers: Set[Any] = Set(6, 7.toShort, 8L, "3")
    
    val result = profileDF.withColumn("isValid", $"profileID".isinSet(validUsers))
    
    result.show(10)
    """
    +---------+-------+
    |profileID|isValid|
    +---------+-------+
    |        1|  false|
    |        2|  false|
    |        3|   true|
    |        4|  false|
    |        5|  false|
    |        6|   true|
    |        7|   true|
    |     null|   null|
    +---------+-------+
     """.stripMargin
    ```
    
    Two new rules in the logical plan optimizers are added.
     
    1. When there is only one element in the **`Set`**, the physical plan will be optimized to **`EqualTo`**, so predicate pushdown can be used. 
    
    ```scala
        profileDF.filter( $"profileID".isinSet(Set(6))).explain(true)
        """
          |== Physical Plan ==
          |*(1) Project [profileID#0]
          |+- *(1) Filter (isnotnull(profileID#0) && (profileID#0 = 6))
          |   +- *(1) FileScan parquet [profileID#0] Batched: true, Format: Parquet, 
          |     PartitionFilters: [], 
          |     PushedFilters: [IsNotNull(profileID), EqualTo(profileID,6)],
          |     ReadSchema: struct<profileID:int>
        """.stripMargin
    ```
    
    2. When the **`Set`** is empty, and the input is nullable, the logical plan will be simplified to 
    
    ```scala
        profileDF.filter( $"profileID".isinSet(Set())).explain(true)
        """
          |== Optimized Logical Plan ==
          |Filter if (isnull(profileID#0)) null else false
          |+- Relation[profileID#0] parquet
        """.stripMargin
    ```
    
    TODO:
    
    1. For multiple conditions with numbers less than certain thresholds, we should still allow predicate pushdown.
    2. Optimize the **`In`** using **`tableswitch`** or **`lookupswitch`** when the numbers of the categories are low, and they are **`Int`**, **`Long`**.
    3. The default immutable hash trees set is slow for query, and we should do benchmark for using different set implementation for faster query.
    4. **`filter(if (condition) null else false)`** can be optimized to false.
    
    ## How was this patch tested?
    
    Several unit tests are added.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/dbtsai/spark optimize-set

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/21416.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #21416
    
----
commit ec91220d5e8658500be6d049f8ab3496fc8a914e
Author: DB Tsai <d_...@...>
Date:   2018-05-17T00:21:14Z

    Added isinSet in DataFrame API for Scala and Java.

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91091/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    retest this please


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    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/3542/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame AP...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r190507907
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/Column.scala ---
    @@ -786,6 +787,24 @@ class Column(val expr: Expression) extends Logging {
       @scala.annotation.varargs
       def isin(list: Any*): Column = withExpr { In(expr, list.map(lit(_).expr)) }
     
    +  /**
    +   * A boolean expression that is evaluated to true if the value of this expression is contained
    +   * by the provided Set.
    +   *
    +   * @group expr_ops
    +   * @since 2.4.0
    +   */
    +  def isinSet(values: scala.collection.Set[_]): Column = isin(values.toSeq: _*)
    --- End diff --
    
    the name can be `isInCollection`


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91145 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91145/testReport)** for PR 21416 at commit [`6ff2806`](https://github.com/apache/spark/commit/6ff280616bab4f38d338b9b3aca38c4affd86cd8).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame AP...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r190717637
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala ---
    @@ -219,7 +219,11 @@ object ReorderAssociativeOperator extends Rule[LogicalPlan] {
     object OptimizeIn extends Rule[LogicalPlan] {
       def apply(plan: LogicalPlan): LogicalPlan = plan transform {
         case q: LogicalPlan => q transformExpressionsDown {
    -      case In(v, list) if list.isEmpty && !v.nullable => FalseLiteral
    +      case In(v, list) if list.isEmpty =>
    +        // When v is not nullable, the following expression will be optimized
    +        // to FalseLiteral which is tested in OptimizeInSuite.scala
    +        If(IsNotNull(v), FalseLiteral, Literal(null, BooleanType))
    +      case In(v, list) if list.length == 1 => EqualTo(v, list.head)
    --- End diff --
    
    I got it. Thanks.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    @rxin I simplified the test cases as you suggested. Thanks.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isInCollection in DataF...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r191101660
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala ---
    @@ -219,7 +218,14 @@ object ReorderAssociativeOperator extends Rule[LogicalPlan] {
     object OptimizeIn extends Rule[LogicalPlan] {
       def apply(plan: LogicalPlan): LogicalPlan = plan transform {
         case q: LogicalPlan => q transformExpressionsDown {
    -      case In(v, list) if list.isEmpty && !v.nullable => FalseLiteral
    +      case In(v, list) if list.isEmpty =>
    --- End diff --
    
    this improvement looks reasonable, but can we move them to a separated PR? it's not related to adding `isInCollection`.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91145/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91220 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91220/testReport)** for PR 21416 at commit [`1332406`](https://github.com/apache/spark/commit/1332406d7f4ca7a9a4a85338f758430ecc334ff8).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    I just found that PySpark's `isin` is capable of similar feature. One issue is Scala/Java API now doesn't match with PySpark API.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91242 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91242/testReport)** for PR 21416 at commit [`fed2846`](https://github.com/apache/spark/commit/fed2846fe7c9ca2cb4534b23803cd29d5a18d4f9).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91242/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    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/3604/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91242 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91242/testReport)** for PR 21416 at commit [`fed2846`](https://github.com/apache/spark/commit/fed2846fe7c9ca2cb4534b23803cd29d5a18d4f9).
     * This patch **fails due to an unknown error code, -9**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isInCollection in DataF...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r191306654
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/ColumnExpressionSuite.scala ---
    @@ -392,9 +396,97 @@ class ColumnExpressionSuite extends QueryTest with SharedSQLContext {
     
         val df2 = Seq((1, Seq(1)), (2, Seq(2)), (3, Seq(3))).toDF("a", "b")
     
    -    intercept[AnalysisException] {
    +    val e = intercept[AnalysisException] {
           df2.filter($"a".isin($"b"))
         }
    +    Seq("cannot resolve", "due to data type mismatch: Arguments must be same type but were")
    +      .foreach { s =>
    +        assert(e.getMessage.toLowerCase(Locale.ROOT).contains(s.toLowerCase(Locale.ROOT)))
    +      }
    +  }
    +
    +  test("isInCollection: Scala Collection") {
    --- End diff --
    
    can we simplify the test cases? you are just testing this api as a wrapper. you don't need to run so many queries for type coercion.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged into master. Thank you everyone for reviewing.
    
    Followup PR will be created for
    
    1. Adding tests in Java.
    2. Adding docs about automagical type casting.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91147 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91147/testReport)** for PR 21416 at commit [`6ff2806`](https://github.com/apache/spark/commit/6ff280616bab4f38d338b9b3aca38c4affd86cd8).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    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/3525/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91147 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91147/testReport)** for PR 21416 at commit [`6ff2806`](https://github.com/apache/spark/commit/6ff280616bab4f38d338b9b3aca38c4affd86cd8).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame AP...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r190799418
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/subquery.scala ---
    @@ -149,7 +149,7 @@ object RewritePredicateSubquery extends Rule[LogicalPlan] with PredicateHelper {
               newPlan = dedupJoin(
                 Join(newPlan, sub, ExistenceJoin(exists), conditions.reduceLeftOption(And)))
               exists
    -        case In(value, Seq(ListQuery(sub, conditions, _, _))) =>
    +        case EqualTo(value, ListQuery(sub, conditions, _, _)) =>
    --- End diff --
    
    @dongjoon-hyun this should fix the test. I'll add one test for this.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame AP...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r190759741
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala ---
    @@ -220,6 +219,7 @@ object OptimizeIn extends Rule[LogicalPlan] {
       def apply(plan: LogicalPlan): LogicalPlan = plan transform {
         case q: LogicalPlan => q transformExpressionsDown {
           case In(v, list) if list.isEmpty && !v.nullable => FalseLiteral
    +      case In(v, list) if list.length == 1 => EqualTo(v, list.head)
    --- End diff --
    
    Yep. This is that one.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    retest this please


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    retest this please


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame AP...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r190754229
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala ---
    @@ -220,6 +219,7 @@ object OptimizeIn extends Rule[LogicalPlan] {
       def apply(plan: LogicalPlan): LogicalPlan = plan transform {
         case q: LogicalPlan => q transformExpressionsDown {
           case In(v, list) if list.isEmpty && !v.nullable => FalseLiteral
    +      case In(v, list) if list.length == 1 => EqualTo(v, list.head)
    --- End diff --
    
    In fact, I'm debugging this StackOverflowError issue shown in Hive test. Thanks for this which helps me a lot for local testing. 


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    test this again


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91089 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91089/testReport)** for PR 21416 at commit [`de3296c`](https://github.com/apache/spark/commit/de3296c116b07d6560b1f89ca97c2014d36d10a8).
     * This patch **fails due to an unknown error code, -9**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91220/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    retest this please
    



---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    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/3637/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    retest this please
    



---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91068/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isInCollection in DataF...

Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r191488193
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/ColumnExpressionSuite.scala ---
    @@ -390,11 +394,67 @@ class ColumnExpressionSuite extends QueryTest with SharedSQLContext {
         checkAnswer(df.filter($"b".isin("z", "y")),
           df.collect().toSeq.filter(r => r.getString(1) == "z" || r.getString(1) == "y"))
     
    +    // Auto casting should work with mixture of different types in collections
    +    checkAnswer(df.filter($"a".isin(1.toShort, "2")),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 1 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isin("3", 2.toLong)),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isin(3, "1")),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 1))
    +
         val df2 = Seq((1, Seq(1)), (2, Seq(2)), (3, Seq(3))).toDF("a", "b")
     
    -    intercept[AnalysisException] {
    +    val e = intercept[AnalysisException] {
           df2.filter($"a".isin($"b"))
         }
    +    Seq("cannot resolve", "due to data type mismatch: Arguments must be same type but were")
    +      .foreach { s =>
    +        assert(e.getMessage.toLowerCase(Locale.ROOT).contains(s.toLowerCase(Locale.ROOT)))
    +      }
    +  }
    +
    +  test("isInCollection: Scala Collection") {
    +    val df = Seq((1, "x"), (2, "y"), (3, "z")).toDF("a", "b")
    +    // Test with different types of collections
    +    checkAnswer(df.filter($"a".isInCollection(Seq(3, 1))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 1))
    +    checkAnswer(df.filter($"a".isInCollection(Seq(1, 2).toSet)),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 1 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isInCollection(Seq(3, 2).toArray)),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isInCollection(Seq(3, 1).toList)),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 1))
    +
    +    val df2 = Seq((1, Seq(1)), (2, Seq(2)), (3, Seq(3))).toDF("a", "b")
    +
    +    val e = intercept[AnalysisException] {
    +      df2.filter($"a".isInCollection(Seq($"b")))
    +    }
    +    Seq("cannot resolve", "due to data type mismatch: Arguments must be same type but were")
    +      .foreach { s =>
    +        assert(e.getMessage.toLowerCase(Locale.ROOT).contains(s.toLowerCase(Locale.ROOT)))
    +      }
    +  }
    +
    +  test("isInCollection: Java Collection") {
    --- End diff --
    
    As stated up above, maybe this would make sense to do in Java, but your call.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame AP...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r190722260
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala ---
    @@ -220,6 +219,7 @@ object OptimizeIn extends Rule[LogicalPlan] {
       def apply(plan: LogicalPlan): LogicalPlan = plan transform {
         case q: LogicalPlan => q transformExpressionsDown {
           case In(v, list) if list.isEmpty && !v.nullable => FalseLiteral
    +      case In(v, list) if list.length == 1 => EqualTo(v, list.head)
    --- End diff --
    
    Could you add the following test case, too?
    ```scala
    scala> sql("select * from t group by a having count(*) = (select count(*) from t)").explain
    == Physical Plan ==
    *(2) Project [a#2L]
    +- *(2) Filter (count(1)#75L = Subquery subquery62)
       :  +- Subquery subquery62
       :     +- *(2) HashAggregate(keys=[], functions=[count(1)])
       :        +- Exchange SinglePartition
       :           +- *(1) HashAggregate(keys=[], functions=[partial_count(1)])
       :              +- *(1) Project
       :                 +- *(1) Range (0, 1, step=1, splits=8)
       +- *(2) HashAggregate(keys=[a#2L], functions=[count(1)])
          +- Exchange hashpartitioning(a#2L, 200)
             +- *(1) HashAggregate(keys=[a#2L], functions=[partial_count(1)])
                +- *(1) Project [id#0L AS a#2L]
                   +- *(1) Range (0, 1, step=1, splits=8)
    
    scala> sql("select * from t group by a having count(*) in (select count(*) from t)").explain
    java.lang.StackOverflowError
      at scala.collection.IndexedSeqOptimized$class.foreach(IndexedSeqOptimized.scala:33)
    ```


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    retest this please


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    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/3629/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91143 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91143/testReport)** for PR 21416 at commit [`da10307`](https://github.com/apache/spark/commit/da103073f847b45d64a1014d99b4a21ccf3f3533).
     * This patch **fails due to an unknown error code, -9**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame AP...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r190470525
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/ColumnExpressionSuite.scala ---
    @@ -397,6 +399,68 @@ class ColumnExpressionSuite extends QueryTest with SharedSQLContext {
         }
       }
     
    +  test("isinSet: Scala Set") {
    +    val df = Seq((1, "x"), (2, "y"), (3, "z")).toDF("a", "b")
    +    checkAnswer(df.filter($"a".isinSet(Set(1, 2))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 1 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isinSet(Set(3, 2))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isinSet(Set(3, 1))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 1))
    +
    +    // Auto casting should work with mixture of different types in Set
    +    checkAnswer(df.filter($"a".isinSet(Set(1.toShort, "2"))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 1 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isinSet(Set("3", 2.toLong))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isinSet(Set(3, "1"))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 1))
    +
    +    checkAnswer(df.filter($"b".isinSet(Set("y", "x"))),
    +      df.collect().toSeq.filter(r => r.getString(1) == "y" || r.getString(1) == "x"))
    +    checkAnswer(df.filter($"b".isinSet(Set("z", "x"))),
    +      df.collect().toSeq.filter(r => r.getString(1) == "z" || r.getString(1) == "x"))
    +    checkAnswer(df.filter($"b".isinSet(Set("z", "y"))),
    +      df.collect().toSeq.filter(r => r.getString(1) == "z" || r.getString(1) == "y"))
    +
    +    val df2 = Seq((1, Seq(1)), (2, Seq(2)), (3, Seq(3))).toDF("a", "b")
    +
    +    intercept[AnalysisException] {
    +      df2.filter($"a".isinSet(Set($"b")))
    +    }
    --- End diff --
    
    Addressed


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isInCollection in DataF...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r191317978
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/ColumnExpressionSuite.scala ---
    @@ -392,9 +396,97 @@ class ColumnExpressionSuite extends QueryTest with SharedSQLContext {
     
         val df2 = Seq((1, Seq(1)), (2, Seq(2)), (3, Seq(3))).toDF("a", "b")
     
    -    intercept[AnalysisException] {
    +    val e = intercept[AnalysisException] {
           df2.filter($"a".isin($"b"))
         }
    +    Seq("cannot resolve", "due to data type mismatch: Arguments must be same type but were")
    +      .foreach { s =>
    +        assert(e.getMessage.toLowerCase(Locale.ROOT).contains(s.toLowerCase(Locale.ROOT)))
    +      }
    +  }
    +
    +  test("isInCollection: Scala Collection") {
    +    val df = Seq((1, "x"), (2, "y"), (3, "z")).toDF("a", "b")
    +    checkAnswer(df.filter($"a".isInCollection(Seq(1, 2))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 1 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isInCollection(Seq(3, 2))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isInCollection(Seq(3, 1))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 1))
    +
    +    // Auto casting should work with mixture of different types in collections
    +    checkAnswer(df.filter($"a".isInCollection(Seq(1.toShort, "2"))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 1 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isInCollection(Seq("3", 2.toLong))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isInCollection(Seq(3, "1"))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 1))
    +
    +    checkAnswer(df.filter($"b".isInCollection(Seq("y", "x"))),
    +      df.collect().toSeq.filter(r => r.getString(1) == "y" || r.getString(1) == "x"))
    +    checkAnswer(df.filter($"b".isInCollection(Seq("z", "x"))),
    +      df.collect().toSeq.filter(r => r.getString(1) == "z" || r.getString(1) == "x"))
    +    checkAnswer(df.filter($"b".isInCollection(Seq("z", "y"))),
    +      df.collect().toSeq.filter(r => r.getString(1) == "z" || r.getString(1) == "y"))
    +
    +    // Test with different types of collections
    +    checkAnswer(df.filter($"a".isInCollection(Seq(1, 2).toSet)),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 1 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isInCollection(Seq(3, 2).toArray)),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isInCollection(Seq(3, 1).toList)),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 1))
    +
    +    val df2 = Seq((1, Seq(1)), (2, Seq(2)), (3, Seq(3))).toDF("a", "b")
    +
    +    val e = intercept[AnalysisException] {
    +      df2.filter($"a".isInCollection(Seq($"b")))
    +    }
    +    Seq("cannot resolve", "due to data type mismatch: Arguments must be same type but were")
    +      .foreach { s =>
    +        assert(e.getMessage.toLowerCase(Locale.ROOT).contains(s.toLowerCase(Locale.ROOT)))
    +      }
    +  }
    +
    +  test("isInCollection: Java Collection") {
    +    val df = Seq((1, "x"), (2, "y"), (3, "z")).toDF("a", "b")
    --- End diff --
    
    Done.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91069/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91141 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91141/testReport)** for PR 21416 at commit [`20650d6`](https://github.com/apache/spark/commit/20650d60d594de6943a7d9b4283916fc08a54f6f).
     * This patch **fails due to an unknown error code, -9**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    @cloud-fan unfortunately,  scala vararg can not be overloaded, and scala will return the following error.
    
    ```scala
    Error:(410, 32) ambiguous reference to overloaded definition,
    both method isin in class Column of type (values: Iterable[_])org.apache.spark.sql.Column
    and  method isin in class Column of type (list: Any*)org.apache.spark.sql.Column
    match argument types (Seq[Int])
        checkAnswer(df.filter($"a".isin(Seq(1, 2))),
    ```
    
    I'm leaning toward to using `isInCollection` now, and implement the corresponding python APIs.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isInCollection in DataF...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/21416


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isInCollection in DataF...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r191306678
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/ColumnExpressionSuite.scala ---
    @@ -392,9 +396,97 @@ class ColumnExpressionSuite extends QueryTest with SharedSQLContext {
     
         val df2 = Seq((1, Seq(1)), (2, Seq(2)), (3, Seq(3))).toDF("a", "b")
     
    -    intercept[AnalysisException] {
    +    val e = intercept[AnalysisException] {
           df2.filter($"a".isin($"b"))
         }
    +    Seq("cannot resolve", "due to data type mismatch: Arguments must be same type but were")
    +      .foreach { s =>
    +        assert(e.getMessage.toLowerCase(Locale.ROOT).contains(s.toLowerCase(Locale.ROOT)))
    +      }
    +  }
    +
    +  test("isInCollection: Scala Collection") {
    +    val df = Seq((1, "x"), (2, "y"), (3, "z")).toDF("a", "b")
    +    checkAnswer(df.filter($"a".isInCollection(Seq(1, 2))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 1 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isInCollection(Seq(3, 2))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isInCollection(Seq(3, 1))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 1))
    +
    +    // Auto casting should work with mixture of different types in collections
    +    checkAnswer(df.filter($"a".isInCollection(Seq(1.toShort, "2"))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 1 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isInCollection(Seq("3", 2.toLong))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isInCollection(Seq(3, "1"))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 1))
    +
    +    checkAnswer(df.filter($"b".isInCollection(Seq("y", "x"))),
    +      df.collect().toSeq.filter(r => r.getString(1) == "y" || r.getString(1) == "x"))
    +    checkAnswer(df.filter($"b".isInCollection(Seq("z", "x"))),
    +      df.collect().toSeq.filter(r => r.getString(1) == "z" || r.getString(1) == "x"))
    +    checkAnswer(df.filter($"b".isInCollection(Seq("z", "y"))),
    +      df.collect().toSeq.filter(r => r.getString(1) == "z" || r.getString(1) == "y"))
    +
    +    // Test with different types of collections
    +    checkAnswer(df.filter($"a".isInCollection(Seq(1, 2).toSet)),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 1 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isInCollection(Seq(3, 2).toArray)),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isInCollection(Seq(3, 1).toList)),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 1))
    +
    +    val df2 = Seq((1, Seq(1)), (2, Seq(2)), (3, Seq(3))).toDF("a", "b")
    +
    +    val e = intercept[AnalysisException] {
    +      df2.filter($"a".isInCollection(Seq($"b")))
    +    }
    +    Seq("cannot resolve", "due to data type mismatch: Arguments must be same type but were")
    +      .foreach { s =>
    +        assert(e.getMessage.toLowerCase(Locale.ROOT).contains(s.toLowerCase(Locale.ROOT)))
    +      }
    +  }
    +
    +  test("isInCollection: Java Collection") {
    +    val df = Seq((1, "x"), (2, "y"), (3, "z")).toDF("a", "b")
    --- End diff --
    
    same thing here. just run a single test case.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    +@cloud-fan 


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91142 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91142/testReport)** for PR 21416 at commit [`da10307`](https://github.com/apache/spark/commit/da103073f847b45d64a1014d99b4a21ccf3f3533).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91212 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91212/testReport)** for PR 21416 at commit [`1332406`](https://github.com/apache/spark/commit/1332406d7f4ca7a9a4a85338f758430ecc334ff8).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isInCollection in DataF...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r191505830
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/ColumnExpressionSuite.scala ---
    @@ -390,11 +394,67 @@ class ColumnExpressionSuite extends QueryTest with SharedSQLContext {
         checkAnswer(df.filter($"b".isin("z", "y")),
           df.collect().toSeq.filter(r => r.getString(1) == "z" || r.getString(1) == "y"))
     
    +    // Auto casting should work with mixture of different types in collections
    +    checkAnswer(df.filter($"a".isin(1.toShort, "2")),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 1 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isin("3", 2.toLong)),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isin(3, "1")),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 1))
    +
         val df2 = Seq((1, Seq(1)), (2, Seq(2)), (3, Seq(3))).toDF("a", "b")
     
    -    intercept[AnalysisException] {
    +    val e = intercept[AnalysisException] {
           df2.filter($"a".isin($"b"))
         }
    +    Seq("cannot resolve", "due to data type mismatch: Arguments must be same type but were")
    +      .foreach { s =>
    +        assert(e.getMessage.toLowerCase(Locale.ROOT).contains(s.toLowerCase(Locale.ROOT)))
    +      }
    +  }
    +
    +  test("isInCollection: Scala Collection") {
    +    val df = Seq((1, "x"), (2, "y"), (3, "z")).toDF("a", "b")
    +    // Test with different types of collections
    +    checkAnswer(df.filter($"a".isInCollection(Seq(3, 1))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 1))
    +    checkAnswer(df.filter($"a".isInCollection(Seq(1, 2).toSet)),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 1 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isInCollection(Seq(3, 2).toArray)),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isInCollection(Seq(3, 1).toList)),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 1))
    +
    +    val df2 = Seq((1, Seq(1)), (2, Seq(2)), (3, Seq(3))).toDF("a", "b")
    +
    +    val e = intercept[AnalysisException] {
    +      df2.filter($"a".isInCollection(Seq($"b")))
    +    }
    +    Seq("cannot resolve", "due to data type mismatch: Arguments must be same type but were")
    +      .foreach { s =>
    +        assert(e.getMessage.toLowerCase(Locale.ROOT).contains(s.toLowerCase(Locale.ROOT)))
    +      }
    +  }
    +
    +  test("isInCollection: Java Collection") {
    --- End diff --
    
    I totally agree with you that we should have tests natively in Java instead of converting the types to Java in Scala and hope the best that it will work in Java. Let's do it in the followup PR.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91091 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91091/testReport)** for PR 21416 at commit [`de3296c`](https://github.com/apache/spark/commit/de3296c116b07d6560b1f89ca97c2014d36d10a8).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91069 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91069/testReport)** for PR 21416 at commit [`6459015`](https://github.com/apache/spark/commit/6459015fd923371b1515177db477afe1ce2813aa).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    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/3580/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    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/3576/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    @viirya good point. One thing I'm not sure is, does `isin(collection: Iterable)` conflict with `isin(list: Any*)`? if they don't conflict, they we can follow pyspark.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91147/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91068 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91068/testReport)** for PR 21416 at commit [`ec91220`](https://github.com/apache/spark/commit/ec91220d5e8658500be6d049f8ab3496fc8a914e).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91212/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    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/3578/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isInCollection in DataF...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r191317980
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/ColumnExpressionSuite.scala ---
    @@ -392,9 +396,97 @@ class ColumnExpressionSuite extends QueryTest with SharedSQLContext {
     
         val df2 = Seq((1, Seq(1)), (2, Seq(2)), (3, Seq(3))).toDF("a", "b")
     
    -    intercept[AnalysisException] {
    +    val e = intercept[AnalysisException] {
           df2.filter($"a".isin($"b"))
         }
    +    Seq("cannot resolve", "due to data type mismatch: Arguments must be same type but were")
    +      .foreach { s =>
    +        assert(e.getMessage.toLowerCase(Locale.ROOT).contains(s.toLowerCase(Locale.ROOT)))
    +      }
    +  }
    +
    +  test("isInCollection: Scala Collection") {
    --- End diff --
    
    Done. 


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame AP...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r190507754
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/Column.scala ---
    @@ -786,6 +787,24 @@ class Column(val expr: Expression) extends Logging {
       @scala.annotation.varargs
       def isin(list: Any*): Column = withExpr { In(expr, list.map(lit(_).expr)) }
     
    +  /**
    +   * A boolean expression that is evaluated to true if the value of this expression is contained
    +   * by the provided Set.
    +   *
    +   * @group expr_ops
    +   * @since 2.4.0
    +   */
    +  def isinSet(values: scala.collection.Set[_]): Column = isin(values.toSeq: _*)
    --- End diff --
    
    shall we be more generic here and accept `Iterable`? Then Set, Seq, Array can all be accepted.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame AP...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r190496061
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala ---
    @@ -220,6 +219,7 @@ object OptimizeIn extends Rule[LogicalPlan] {
       def apply(plan: LogicalPlan): LogicalPlan = plan transform {
         case q: LogicalPlan => q transformExpressionsDown {
           case In(v, list) if list.isEmpty && !v.nullable => FalseLiteral
    +      case In(v, list) if list.length == 1 => EqualTo(v, list.head)
    --- End diff --
    
    can we update the comment to mention it?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91178 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91178/testReport)** for PR 21416 at commit [`286a468`](https://github.com/apache/spark/commit/286a46849fb8840b6aad243fbc1b8b17b1f6a02b).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91178 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91178/testReport)** for PR 21416 at commit [`286a468`](https://github.com/apache/spark/commit/286a46849fb8840b6aad243fbc1b8b17b1f6a02b).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Test FAILed.
    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/3575/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91144/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91089/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    retest this please


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91141/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91144 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91144/testReport)** for PR 21416 at commit [`730b19b`](https://github.com/apache/spark/commit/730b19ba8b0e5db3c4bfda150cd156370a321fef).
     * This patch **fails due to an unknown error code, -9**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    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/3524/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    LGTM


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isInCollection in DataF...

Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r191491943
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/Column.scala ---
    @@ -786,6 +787,24 @@ class Column(val expr: Expression) extends Logging {
       @scala.annotation.varargs
       def isin(list: Any*): Column = withExpr { In(expr, list.map(lit(_).expr)) }
    --- End diff --
    
    I know this is not your change, but I think (both here and bellow) something about the automagical type casting thats going on should be in the docstring/scaladoc/javadoc because to me its a little surprising how this will compare integers to strings and silently convert the types including if there are no strings which can be converted to integers. And I'd also include that in the isInCollection docstring/scaladoc/javadoc bellow.
    
    I'd also point out that the result of the conversion needs to be of the same type and not of a sequence of the type (although the error message we get is pretty clear so your call).
    
    Just a suggestion for improvement.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91068 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91068/testReport)** for PR 21416 at commit [`ec91220`](https://github.com/apache/spark/commit/ec91220d5e8658500be6d049f8ab3496fc8a914e).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91178/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isInCollection in DataF...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r191506417
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/Column.scala ---
    @@ -786,6 +787,24 @@ class Column(val expr: Expression) extends Logging {
       @scala.annotation.varargs
       def isin(list: Any*): Column = withExpr { In(expr, list.map(lit(_).expr)) }
    --- End diff --
    
    +1 Let's do it in the followup PR. Thanks.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91220 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91220/testReport)** for PR 21416 at commit [`1332406`](https://github.com/apache/spark/commit/1332406d7f4ca7a9a4a85338f758430ecc334ff8).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91142 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91142/testReport)** for PR 21416 at commit [`da10307`](https://github.com/apache/spark/commit/da103073f847b45d64a1014d99b4a21ccf3f3533).
     * This patch **fails due to an unknown error code, -9**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame AP...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r190407851
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala ---
    @@ -219,7 +219,11 @@ object ReorderAssociativeOperator extends Rule[LogicalPlan] {
     object OptimizeIn extends Rule[LogicalPlan] {
       def apply(plan: LogicalPlan): LogicalPlan = plan transform {
         case q: LogicalPlan => q transformExpressionsDown {
    -      case In(v, list) if list.isEmpty && !v.nullable => FalseLiteral
    +      case In(v, list) if list.isEmpty =>
    +        // When v is not nullable, the following expression will be optimized
    +        // to FalseLiteral which is tested in OptimizeInSuite.scala
    +        If(IsNotNull(v), FalseLiteral, Literal(null, BooleanType))
    +      case In(v, list) if list.length == 1 => EqualTo(v, list.head)
    --- End diff --
    
    Ur, @dbtsai . This will cause side-effects on typecasting. For example, please see the following example. Could you add these kind of test cases?
    ```scala
    scala> sql("select '1.1' in (1), '1.1' = 1").collect()
    res0: Array[org.apache.spark.sql.Row] = Array([false,true])
    ```


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91144 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91144/testReport)** for PR 21416 at commit [`730b19b`](https://github.com/apache/spark/commit/730b19ba8b0e5db3c4bfda150cd156370a321fef).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isInCollection in DataF...

Posted by holdenk <gi...@git.apache.org>.
Github user holdenk commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r191486882
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/Column.scala ---
    @@ -786,6 +787,24 @@ class Column(val expr: Expression) extends Logging {
       @scala.annotation.varargs
       def isin(list: Any*): Column = withExpr { In(expr, list.map(lit(_).expr)) }
     
    +  /**
    +   * A boolean expression that is evaluated to true if the value of this expression is contained
    +   * by the provided collection.
    +   *
    +   * @group expr_ops
    +   * @since 2.4.0
    +   */
    +  def isInCollection(values: scala.collection.Iterable[_]): Column = isin(values.toSeq: _*)
    +
    +  /**
    +   * A boolean expression that is evaluated to true if the value of this expression is contained
    +   * by the provided collection.
    +   *
    +   * @group java_expr_ops
    +   * @since 2.4.0
    +   */
    +  def isInCollection(values: java.lang.Iterable[_]): Column = isInCollection(values.asScala)
    --- End diff --
    
    Not that we need it for sure, but in the past some of our Java APIs have been difficult to call from Java and I think that if were making an API designed to be called from Java it might make sense to have a test case for it in Java. Here I understand it's not super important, so just a suggestion.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    LGTM


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    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/3581/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91141 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91141/testReport)** for PR 21416 at commit [`20650d6`](https://github.com/apache/spark/commit/20650d60d594de6943a7d9b4283916fc08a54f6f).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    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/3540/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    LGTM (I didn't look that carefully though)


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91254 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91254/testReport)** for PR 21416 at commit [`fed2846`](https://github.com/apache/spark/commit/fed2846fe7c9ca2cb4534b23803cd29d5a18d4f9).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame AP...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r190472138
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/expressions.scala ---
    @@ -219,7 +219,11 @@ object ReorderAssociativeOperator extends Rule[LogicalPlan] {
     object OptimizeIn extends Rule[LogicalPlan] {
       def apply(plan: LogicalPlan): LogicalPlan = plan transform {
         case q: LogicalPlan => q transformExpressionsDown {
    -      case In(v, list) if list.isEmpty && !v.nullable => FalseLiteral
    +      case In(v, list) if list.isEmpty =>
    +        // When v is not nullable, the following expression will be optimized
    +        // to FalseLiteral which is tested in OptimizeInSuite.scala
    +        If(IsNotNull(v), FalseLiteral, Literal(null, BooleanType))
    +      case In(v, list) if list.length == 1 => EqualTo(v, list.head)
    --- End diff --
    
    Why does it have any implication on typecasting? With this PR, it seems I get the correct result.
    
    ```scala
    == Analyzed Logical Plan ==
    (CAST(1.1 AS STRING) IN (CAST(1 AS STRING))): boolean, (CAST(1.1 AS INT) = 1): boolean
    Project [cast(1.1 as string) IN (cast(1 as string)) AS (CAST(1.1 AS STRING) IN (CAST(1 AS STRING)))#484, (cast(1.1 as int) = 1) AS (CAST(1.1 AS INT) = 1)#485]
    +- OneRowRelation
    
    == Optimized Logical Plan ==
    Project [false AS (CAST(1.1 AS STRING) IN (CAST(1 AS STRING)))#484, true AS (CAST(1.1 AS INT) = 1)#485]
    +- OneRowRelation
    
    == Physical Plan ==
    *(1) Project [false AS (CAST(1.1 AS STRING) IN (CAST(1 AS STRING)))#484, true AS (CAST(1.1 AS INT) = 1)#485]
    +- Scan OneRowRelation[]
    ```


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91254 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91254/testReport)** for PR 21416 at commit [`fed2846`](https://github.com/apache/spark/commit/fed2846fe7c9ca2cb4534b23803cd29d5a18d4f9).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91143 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91143/testReport)** for PR 21416 at commit [`da10307`](https://github.com/apache/spark/commit/da103073f847b45d64a1014d99b4a21ccf3f3533).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91091 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91091/testReport)** for PR 21416 at commit [`de3296c`](https://github.com/apache/spark/commit/de3296c116b07d6560b1f89ca97c2014d36d10a8).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    +1, LGTM.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91145 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91145/testReport)** for PR 21416 at commit [`6ff2806`](https://github.com/apache/spark/commit/6ff280616bab4f38d338b9b3aca38c4affd86cd8).
     * This patch **fails due to an unknown error code, -9**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91212 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91212/testReport)** for PR 21416 at commit [`1332406`](https://github.com/apache/spark/commit/1332406d7f4ca7a9a4a85338f758430ecc334ff8).
     * This patch **fails due to an unknown error code, -9**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91254/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91089 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91089/testReport)** for PR 21416 at commit [`de3296c`](https://github.com/apache/spark/commit/de3296c116b07d6560b1f89ca97c2014d36d10a8).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    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/3667/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame AP...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r190966442
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/Column.scala ---
    @@ -786,6 +787,24 @@ class Column(val expr: Expression) extends Logging {
       @scala.annotation.varargs
       def isin(list: Any*): Column = withExpr { In(expr, list.map(lit(_).expr)) }
     
    +  /**
    +   * A boolean expression that is evaluated to true if the value of this expression is contained
    +   * by the provided Set.
    +   *
    +   * @group expr_ops
    +   * @since 2.4.0
    +   */
    +  def isinSet(values: scala.collection.Set[_]): Column = isin(values.toSeq: _*)
    --- End diff --
    
    Sound good.
    
     How do we want to do the naming? `def isin` has `i` as lower case. If we do `isInCollection`, it will be slightly inconsistent. 


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    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/3656/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by dbtsai <gi...@git.apache.org>.
Github user dbtsai commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    @cloud-fan Let me know if the new API looks good to you. Thanks.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91143/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/91142/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    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/3577/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame AP...

Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21416#discussion_r190399106
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/ColumnExpressionSuite.scala ---
    @@ -397,6 +399,68 @@ class ColumnExpressionSuite extends QueryTest with SharedSQLContext {
         }
       }
     
    +  test("isinSet: Scala Set") {
    +    val df = Seq((1, "x"), (2, "y"), (3, "z")).toDF("a", "b")
    +    checkAnswer(df.filter($"a".isinSet(Set(1, 2))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 1 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isinSet(Set(3, 2))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isinSet(Set(3, 1))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 1))
    +
    +    // Auto casting should work with mixture of different types in Set
    +    checkAnswer(df.filter($"a".isinSet(Set(1.toShort, "2"))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 1 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isinSet(Set("3", 2.toLong))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 2))
    +    checkAnswer(df.filter($"a".isinSet(Set(3, "1"))),
    +      df.collect().toSeq.filter(r => r.getInt(0) == 3 || r.getInt(0) == 1))
    +
    +    checkAnswer(df.filter($"b".isinSet(Set("y", "x"))),
    +      df.collect().toSeq.filter(r => r.getString(1) == "y" || r.getString(1) == "x"))
    +    checkAnswer(df.filter($"b".isinSet(Set("z", "x"))),
    +      df.collect().toSeq.filter(r => r.getString(1) == "z" || r.getString(1) == "x"))
    +    checkAnswer(df.filter($"b".isinSet(Set("z", "y"))),
    +      df.collect().toSeq.filter(r => r.getString(1) == "z" || r.getString(1) == "y"))
    +
    +    val df2 = Seq((1, Seq(1)), (2, Seq(2)), (3, Seq(3))).toDF("a", "b")
    +
    +    intercept[AnalysisException] {
    +      df2.filter($"a".isinSet(Set($"b")))
    +    }
    --- End diff --
    
    Let's check the error message to prevent the future regression like raising different AnalysisException.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isInCollection in DataFrame AP...

Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    Yes. The design of PySpark API seems to be a bit different than Scala/Java API at beginning. If we are going to make them consistent, either we break Scala queries like `col.isin(Array[Byte](1,2,3))` or PySpark queries like `col.isin([1, 2, 3])`.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21416: [SPARK-24371] [SQL] Added isinSet in DataFrame API for S...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21416
  
    **[Test build #91069 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91069/testReport)** for PR 21416 at commit [`6459015`](https://github.com/apache/spark/commit/6459015fd923371b1515177db477afe1ce2813aa).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org