You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by wangyum <gi...@git.apache.org> on 2017/08/05 09:25:49 UTC

[GitHub] spark pull request #18853: [SPARK-21646][SQL] BinaryComparison shouldn't aut...

GitHub user wangyum opened a pull request:

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

    [SPARK-21646][SQL] BinaryComparison shouldn't auto cast string to int/long

    ## What changes were proposed in this pull request?
    
    How to reproduce:
    hive:
    ```:sql
    $ hive -S
    hive> create table spark_21646(c1 string, c2 string);
    hive> insert into spark_21646 values('92233720368547758071', 'a');
    hive> insert into spark_21646 values('21474836471', 'b');
    hive> insert into spark_21646 values('10', 'c');
    hive> select * from spark_21646 where c1 > 0;
    92233720368547758071	a
    10	c
    21474836471	b
    hive>
    ```
    
    spark-sql:
    ```:sql
    $ spark-sql -S
    spark-sql> select * from spark_21646 where c1 > 0;
    10      c                                                                       
    spark-sql> select * from spark_21646 where c1 > 0L;
    21474836471	b
    10	c
    spark-sql> explain select * from spark_21646 where c1 > 0;
    == Physical Plan ==
    *Project [c1#14, c2#15]
    +- *Filter (isnotnull(c1#14) && (cast(c1#14 as int) > 0))
       +- *FileScan parquet spark_21646[c1#14,c2#15] Batched: true, Format: Parquet, Location: InMemoryFileIndex[viewfs://cluster4/user/hive/warehouse/spark_21646], PartitionFilters: [], PushedFilters: [IsNotNull(c1)], ReadSchema: struct<c1:string,c2:string>
    spark-sql> 
    ```
    
    As you can see, spark auto cast c1 to int type, if this value out of integer range, the result is different from Hive. This PR makes Literal' s type keeps pace with AttributeReference's type.
    
    ## How was this patch tested?
    
    unit tests

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

    $ git pull https://github.com/wangyum/spark SPARK-21646

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

    https://github.com/apache/spark/pull/18853.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 #18853
    
----
commit f59a213027968f5fe157376db51d0e15b68b47db
Author: Yuming Wang <wg...@gmail.com>
Date:   2017-08-05T09:20:17Z

    BinaryComparison shouldn't auto cast string to int/long

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request #18853: [SPARK-21646][SQL] CommonType for binary comparis...

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

    https://github.com/apache/spark/pull/18853#discussion_r140799432
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala ---
    @@ -115,21 +115,46 @@ object TypeCoercion {
        * is a String and the other is not. It also handles when one op is a Date and the
        * other is a Timestamp by making the target type to be String.
        */
    -  val findCommonTypeForBinaryComparison: (DataType, DataType) => Option[DataType] = {
    -    // We should cast all relative timestamp/date/string comparison into string comparisons
    -    // This behaves as a user would expect because timestamp strings sort lexicographically.
    -    // i.e. TimeStamp(2013-01-01 00:00 ...) < "2014" = true
    -    case (StringType, DateType) => Some(StringType)
    -    case (DateType, StringType) => Some(StringType)
    -    case (StringType, TimestampType) => Some(StringType)
    -    case (TimestampType, StringType) => Some(StringType)
    -    case (TimestampType, DateType) => Some(StringType)
    -    case (DateType, TimestampType) => Some(StringType)
    -    case (StringType, NullType) => Some(StringType)
    -    case (NullType, StringType) => Some(StringType)
    -    case (l: StringType, r: AtomicType) if r != StringType => Some(r)
    -    case (l: AtomicType, r: StringType) if (l != StringType) => Some(l)
    -    case (l, r) => None
    +  private def findCommonTypeForBinaryComparison(
    +      plan: LogicalPlan,
    +      l: DataType,
    +      r: DataType): Option[DataType] =
    +    if (!plan.conf.isHiveTypeCoercionMode) {
    +      (l, r) match {
    +        // We should cast all relative timestamp/date/string comparison into string comparisons
    +        // This behaves as a user would expect because timestamp strings sort lexicographically.
    +        // i.e. TimeStamp(2013-01-01 00:00 ...) < "2014" = true
    --- End diff --
    
    I think this comment should be updated given the latest investigation, explaining both the original motivation, and how it is flawed.  Eg.
    
    "Originally spark cast all relative timestamp/date/string comparison into string comparisons.  The motivation was that this would lead to natural comparisons on simple string inputs for times, eg. TimeStamp(2013-01-01 00:00 ...) < "2014" = true.  However, this leads to other logical inconsistencies, eg. TimeStamp(2013-01-01 00:00 ...) < "5" = true.  Also, equals is not consistent with other binary comparisions.  Futhermore, comparing to a string that does not look like a time at all will still compare, just with an unexpected result."


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    Will review it soon. Thanks for your work!


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #82535 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82535/testReport)** for PR 18853 at commit [`53d673f`](https://github.com/apache/spark/commit/53d673f0fd6cdfc06e34147e364c573ada04114c).


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81999/
    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 #18853: [SPARK-21646][SQL] Add new type coercion to compa...

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

    https://github.com/apache/spark/pull/18853#discussion_r155366222
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala ---
    @@ -148,6 +159,25 @@ object TypeCoercion {
         case (l, r) => None
       }
     
    +  val findCommonTypeToCompatibleWithHive: (DataType, DataType) => Option[DataType] = {
    +    // Follow hive's binary comparison action:
    +    // https://github.com/apache/hive/blob/rel/storage-release-2.4.0/ql/src/java/
    +    // org/apache/hadoop/hive/ql/exec/FunctionRegistry.java#L781
    --- End diff --
    
    I saw the change history of this file. It sounds like Hive's type coercion rules are also evolving. 


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/83826/
    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 #18853: [SPARK-21646][SQL] CommonType for binary comparis...

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

    https://github.com/apache/spark/pull/18853#discussion_r139465565
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala ---
    @@ -352,11 +374,16 @@ object TypeCoercion {
             p.makeCopy(Array(Cast(left, TimestampType), right))
           case p @ Equality(left @ TimestampType(), right @ StringType()) =>
             p.makeCopy(Array(left, Cast(right, TimestampType)))
    -
           case p @ BinaryComparison(left, right)
    -        if findCommonTypeForBinaryComparison(left.dataType, right.dataType).isDefined =>
    +        if !plan.conf.binaryComparisonCompatibleWithHive &&
    +          findCommonTypeForBinaryComparison(left.dataType, right.dataType).isDefined =>
             val commonType = findCommonTypeForBinaryComparison(left.dataType, right.dataType).get
             p.makeCopy(Array(castExpr(left, commonType), castExpr(right, commonType)))
    +      case p @ BinaryComparison(left, right)
    +        if plan.conf.binaryComparisonCompatibleWithHive &&
    --- End diff --
    
    This is hard to maintain and debug. Instead of mixing them together, could you separate it from the current rule Spark uses? 


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **Spark vs Teradata**:
    
    <img width="789" alt="td" src="https://user-images.githubusercontent.com/5399861/40102134-43a138e2-591c-11e8-8bf1-00fb9b72e026.png">
    <img width="622" alt="spark" src="https://user-images.githubusercontent.com/5399861/40102133-436658a8-591c-11e8-950c-9ec95a4c7ed0.png">
    



---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #82554 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82554/testReport)** for PR 18853 at commit [`2ada11a`](https://github.com/apache/spark/commit/2ada11a36d9fe9aa0f1048b535ac2da2a129c49f).


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #83826 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83826/testReport)** for PR 18853 at commit [`22d0355`](https://github.com/apache/spark/commit/22d0355c7fc167194c0e6021e80376badfcd60dc).
     * This patch **fails due to an unknown error code, -9**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class ParquetToSparkSchemaConverter(`
      * `class SparkToParquetSchemaConverter(`


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] BinaryComparison shouldn't auto cast ...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] CommonType for binary comparis...

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

    https://github.com/apache/spark/pull/18853#discussion_r139464231
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
    @@ -925,6 +925,12 @@ object SQLConf {
           .intConf
           .createWithDefault(10000)
     
    +  val BINARY_COMPARISON_COMPATIBLE_WITH_HIVE =
    +    buildConf("spark.sql.binary.comparison.compatible.with.hive")
    +      .doc("Whether compatible with Hive when binary comparison.")
    +      .booleanConf
    +      .createWithDefault(true)
    --- End diff --
    
    This has to be `false`.


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #81613 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81613/testReport)** for PR 18853 at commit [`522c4cd`](https://github.com/apache/spark/commit/522c4cddbd1ac0634fe5381d8c95df64c1eeb9c6).


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    `TypeCoercionModeSuite` is an end-to-end test suite. We really need a test case in `SQLQueryTestSuite`. That means, create a .sql file that contains all the type mapping and implicit casting queries that can be ran in both Hive and Spark. We can easily verify whether we correctly cover all the type coercion compatibility issues. 
    
    Could you please do it? This must take a lot of efforts, but it really helps us to find all the holes. Appreciate it!


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #95733 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/95733/testReport)** for PR 18853 at commit [`d0a2089`](https://github.com/apache/spark/commit/d0a2089b8c52a3be3977f0bfc5538758ef7d2b55).
     * This patch **fails Spark unit tests**.
     * This patch **does not merge 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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #81871 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81871/testReport)** for PR 18853 at commit [`844aec7`](https://github.com/apache/spark/commit/844aec7f4022140921d485b89fc240846bd05ac3).


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #84516 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84516/testReport)** for PR 18853 at commit [`663eb35`](https://github.com/apache/spark/commit/663eb3559c32058a2b85dc96a5fbf86ee5a6df4d).
     * 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 #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #81606 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81606/testReport)** for PR 18853 at commit [`8d37c72`](https://github.com/apache/spark/commit/8d37c720b9cb1e3e4b8c4cb6a486ee9c1d07d59c).


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] BinaryComparison shouldn't auto cast ...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    Thanks for your work! 
    
    Could you please write a design doc to summarize the newly introduced Hive-specific `typeCoercion`? Also document what is the difference between Hive and Spark. Thanks!


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] BinaryComparison shouldn't auto cast ...

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

    https://github.com/apache/spark/pull/18853
  
    Thanks @maropu,  There are some problems:
    ```:sql
    spark-sql> select "20" > "100";
    true
    spark-sql> 
    ```
    So [`tmap.tkey < 100`](https://github.com/apache/spark/blob/v2.2.0/sql/hive/src/test/resources/ql/src/test/queries/clientpositive/input14.q#L18)'s [result](https://github.com/apache/spark/blob/v2.2.0/sql/hive/src/test/resources/golden/input14-3-adc1ec67836b26b60d8547c4996bfd8f#L1-L4) is not we expected. Do you have any idea?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #81638 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81638/testReport)** for PR 18853 at commit [`3bec6a2`](https://github.com/apache/spark/commit/3bec6a22565c58ad2da18d66e1e285d644f3577a).


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] BinaryComparison shouldn't auto cast ...

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

    https://github.com/apache/spark/pull/18853
  
    How about casting the `int` values into `string` ones in that case you described in the description, and then comparing them by a lexicographical order?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/81606/
    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 #18853: [SPARK-21646][SQL] Add new type coercion to compa...

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/18853#discussion_r150227062
  
    --- Diff: docs/sql-programming-guide.md ---
    @@ -1460,6 +1460,13 @@ that these options will be deprecated in future release as more optimizations ar
           Configures the number of partitions to use when shuffling data for joins or aggregations.
         </td>
       </tr>
    +  <tr>
    +    <td><code>spark.sql.typeCoercion.mode</code></td>
    +    <td><code>legacy</code></td>
    +    <td>
    +        The <code>legacy</code> type coercion mode was used in spark prior to 2.3, and so it continues to be the default to avoid breaking behavior. However, it has logical inconsistencies. The <code>hive</code> mode is preferred for most new applications, though it may require additional manual casting.
    --- End diff --
    
    I don't agree hive's type coercion rule is the most reasonable. One example is casting both sides to double when comparing string and long, which may lead to wrong result because of precision lose.
    
    I'd like to be neutral here, just say users can choose different type coercion mode, like hive, mysql, etc. By default it's spark.


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #82535 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82535/testReport)** for PR 18853 at commit [`53d673f`](https://github.com/apache/spark/commit/53d673f0fd6cdfc06e34147e364c573ada04114c).
     * 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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] BinaryComparison shouldn't auto cast ...

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

    https://github.com/apache/spark/pull/18853
  
    Currently, the type casting has a few issues when types are different. So far, we do not have any good option to resolve all the issues. Thus, we are hesitant to introduce any behavior change unless this is well defined. Could you do a research to see how the others behave? Any rule?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] BinaryComparison shouldn't auto cast ...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #81605 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81605/testReport)** for PR 18853 at commit [`cedb239`](https://github.com/apache/spark/commit/cedb2397dcd3f68d313fb447675d923eea628ebf).
     * This patch **fails Spark unit tests**.
     * This patch **does not merge 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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #83826 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83826/testReport)** for PR 18853 at commit [`22d0355`](https://github.com/apache/spark/commit/22d0355c7fc167194c0e6021e80376badfcd60dc).


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #82676 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82676/testReport)** for PR 18853 at commit [`d34f294`](https://github.com/apache/spark/commit/d34f294affe2722811e9e09ff2e4d289c35b1319).
     * 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 #18853: [SPARK-21646][SQL] Add new type coercion to compa...

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

    https://github.com/apache/spark/pull/18853#discussion_r154515053
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala ---
    @@ -62,6 +61,18 @@ object TypeCoercion {
           WindowFrameCoercion ::
           Nil
     
    +  def rules(conf: SQLConf): List[Rule[LogicalPlan]] = {
    +    if (conf.isHiveTypeCoercionMode) {
    +      commonTypeCoercionRules :+
    +        HiveInConversion :+
    +        HivePromoteStrings
    +    } else {
    +      commonTypeCoercionRules :+
    +        InConversion :+
    +        PromoteStrings
    --- End diff --
    
    These two rules are moved to the end of the whole batch. It should be fine after https://github.com/apache/spark/pull/19867.


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] BinaryComparison shouldn't auto cast ...

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

    https://github.com/apache/spark/pull/18853
  
    Casting the `int` values into `string` can works, but filter a int column with string type feels terrible.
    My opinion is cast filter value to column type. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] BinaryComparison shouldn't auto cast ...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #84516 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84516/testReport)** for PR 18853 at commit [`663eb35`](https://github.com/apache/spark/commit/663eb3559c32058a2b85dc96a5fbf86ee5a6df4d).


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #81638 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81638/testReport)** for PR 18853 at commit [`3bec6a2`](https://github.com/apache/spark/commit/3bec6a22565c58ad2da18d66e1e285d644f3577a).
     * 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 #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    Thanks for working on it!
    
    I checked the related codes in HIve. Does the current fix resolve all the differences between Hive-speicific type casting/coercion? Please list all the difference between Hive and current Spark SQL semantics. 
    
    We might need a new batch to replace the existing `typeCoercionRules` for Hive compatibility. 


---

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


[GitHub] spark pull request #18853: [SPARK-21646][SQL] CommonType for binary comparis...

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

    https://github.com/apache/spark/pull/18853#discussion_r139821539
  
    --- Diff: docs/sql-programming-guide.md ---
    @@ -968,6 +968,13 @@ Configuration of Parquet can be done using the `setConf` method on `SparkSession
         </p>
       </td>
     </tr>
    +<tr>
    +  <td><code>spark.sql.autoTypeCastingCompatibility</code></td>
    --- End diff --
    
    `spark.sql.typeCoercion.mode`


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #84519 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84519/testReport)** for PR 18853 at commit [`7802483`](https://github.com/apache/spark/commit/7802483075e4328b3a2299413376cf2c6440a2b0).


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #85841 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85841/testReport)** for PR 18853 at commit [`408e889`](https://github.com/apache/spark/commit/408e889caa8d61b7267f0f391be4af5fde82a0c9).


---

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


[GitHub] spark pull request #18853: [SPARK-21646][SQL] CommonType for binary comparis...

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

    https://github.com/apache/spark/pull/18853#discussion_r139719600
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala ---
    @@ -352,11 +374,16 @@ object TypeCoercion {
             p.makeCopy(Array(Cast(left, TimestampType), right))
           case p @ Equality(left @ TimestampType(), right @ StringType()) =>
             p.makeCopy(Array(left, Cast(right, TimestampType)))
    -
           case p @ BinaryComparison(left, right)
    -        if findCommonTypeForBinaryComparison(left.dataType, right.dataType).isDefined =>
    +        if !plan.conf.binaryComparisonCompatibleWithHive &&
    +          findCommonTypeForBinaryComparison(left.dataType, right.dataType).isDefined =>
             val commonType = findCommonTypeForBinaryComparison(left.dataType, right.dataType).get
             p.makeCopy(Array(castExpr(left, commonType), castExpr(right, commonType)))
    +      case p @ BinaryComparison(left, right)
    +        if plan.conf.binaryComparisonCompatibleWithHive &&
    --- End diff --
    
    It needs 2 `PromoteStrings` and 2 `InConversion` if separate from current rule, So I merge the logic to `findCommonTypeForBinaryComparison`.


---

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


[GitHub] spark pull request #18853: [SPARK-21646][SQL] CommonType for binary comparis...

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

    https://github.com/apache/spark/pull/18853#discussion_r139821887
  
    --- Diff: docs/sql-programming-guide.md ---
    @@ -968,6 +968,13 @@ Configuration of Parquet can be done using the `setConf` method on `SparkSession
         </p>
       </td>
     </tr>
    +<tr>
    +  <td><code>spark.sql.autoTypeCastingCompatibility</code></td>
    +  <td>false</td>
    --- End diff --
    
    `hive, default`
    <td><code> hive,<br/> default</code></td>



---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #81876 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81876/testReport)** for PR 18853 at commit [`844aec7`](https://github.com/apache/spark/commit/844aec7f4022140921d485b89fc240846bd05ac3).
     * 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 #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] BinaryComparison shouldn't auto cast ...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] BinaryComparison shouldn't auto cast ...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/84516/
    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 #18853: [SPARK-21646][SQL] CommonType for binary comparis...

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

    https://github.com/apache/spark/pull/18853#discussion_r139464467
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
    @@ -925,6 +925,12 @@ object SQLConf {
           .intConf
           .createWithDefault(10000)
     
    +  val BINARY_COMPARISON_COMPATIBLE_WITH_HIVE =
    +    buildConf("spark.sql.binary.comparison.compatible.with.hive")
    --- End diff --
    
    -> `spark.sql.autoTypeCastingCompatibility`


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #81999 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81999/testReport)** for PR 18853 at commit [`27d5b13`](https://github.com/apache/spark/commit/27d5b13190267637ed9dae8d7cb6aa762d7fa320).


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #83687 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83687/testReport)** for PR 18853 at commit [`b99fb60`](https://github.com/apache/spark/commit/b99fb60d45d67059cd073027ba197e6bb7383715).
     * 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 #18853: [SPARK-21646][SQL] CommonType for binary comparis...

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

    https://github.com/apache/spark/pull/18853#discussion_r143598538
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala ---
    @@ -412,7 +480,66 @@ object TypeCoercion {
     
             val commonTypes = lhs.zip(rhs).flatMap { case (l, r) =>
               findCommonTypeForBinaryComparison(l.dataType, r.dataType)
    +              .orElse(findTightestCommonType(l.dataType, r.dataType))
    --- End diff --
    
    Could you still try to avoid duplicating the codes?


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] BinaryComparison shouldn't auto cast ...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #83847 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83847/testReport)** for PR 18853 at commit [`22d0355`](https://github.com/apache/spark/commit/22d0355c7fc167194c0e6021e80376badfcd60dc).


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #91633 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/91633/testReport)** for PR 18853 at commit [`d0a2089`](https://github.com/apache/spark/commit/d0a2089b8c52a3be3977f0bfc5538758ef7d2b55).
     * 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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #85840 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85840/testReport)** for PR 18853 at commit [`97a071d`](https://github.com/apache/spark/commit/97a071d91ec25159bba655b2bd9f6e2134d87088).
     * 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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #85849 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85849/testReport)** for PR 18853 at commit [`e763330`](https://github.com/apache/spark/commit/e763330edae88d4dad410214608fb5448d90a989).
     * 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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    Let me open an umbrella JIRA for tracking it. We can do it for both native and Hive compatibility mode.  


---

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


[GitHub] spark pull request #18853: [SPARK-21646][SQL] Add new type coercion to compa...

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

    https://github.com/apache/spark/pull/18853#discussion_r151037844
  
    --- Diff: docs/sql-programming-guide.md ---
    @@ -1490,6 +1490,13 @@ that these options will be deprecated in future release as more optimizations ar
           Configures the number of partitions to use when shuffling data for joins or aggregations.
         </td>
       </tr>
    +  <tr>
    +    <td><code>spark.sql.typeCoercion.mode</code></td>
    +    <td><code>default</code></td>
    +    <td>
    +        The <code>default</code> type coercion mode was used in spark prior to 2.3, and so it continues to be the default to avoid breaking behavior. However, it has logical inconsistencies. The <code>hive</code> mode is preferred for most new applications, though it may require additional manual casting.
    --- End diff --
    
    2.3 -> 2.3.0 to be clear?


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #88780 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88780/testReport)** for PR 18853 at commit [`81067b9`](https://github.com/apache/spark/commit/81067b984b95e04429db1ad19e6161c31a8228c0).


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #84519 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/84519/testReport)** for PR 18853 at commit [`7802483`](https://github.com/apache/spark/commit/7802483075e4328b3a2299413376cf2c6440a2b0).
     * 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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #83847 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/83847/testReport)** for PR 18853 at commit [`22d0355`](https://github.com/apache/spark/commit/22d0355c7fc167194c0e6021e80376badfcd60dc).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class ParquetToSparkSchemaConverter(`
      * `class SparkToParquetSchemaConverter(`


---

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


[GitHub] spark pull request #18853: [SPARK-21646][SQL] Add new type coercion to compa...

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

    https://github.com/apache/spark/pull/18853#discussion_r155364153
  
    --- Diff: docs/sql-programming-guide.md ---
    @@ -1490,6 +1490,16 @@ that these options will be deprecated in future release as more optimizations ar
           Configures the number of partitions to use when shuffling data for joins or aggregations.
         </td>
       </tr>
    +  <tr>
    +    <td><code>spark.sql.typeCoercion.mode</code></td>
    +    <td><code>default</code></td>
    +    <td>
    +      The <code>default</code> type coercion mode was used in spark prior to 2.3.0, and so it
    +      continues to be the default to avoid breaking behavior. However, it has logical
    +      inconsistencies. The <code>hive</code> mode is preferred for most new applications, though
    +      it may require additional manual casting.
    --- End diff --
    
    > Since Spark 2.3, the <code>hive</code> mode is introduced for Hive compatiblity. Spark SQL has its native type cocersion mode, which is enabled by default.


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #85846 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85846/testReport)** for PR 18853 at commit [`408e889`](https://github.com/apache/spark/commit/408e889caa8d61b7267f0f391be4af5fde82a0c9).


---

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


[GitHub] spark pull request #18853: [SPARK-21646][SQL] Add new type coercion to compa...

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

    https://github.com/apache/spark/pull/18853#discussion_r155123662
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala ---
    @@ -45,10 +46,9 @@ import org.apache.spark.sql.types._
      */
     object TypeCoercion {
     
    -  val typeCoercionRules =
    +  private val commonTypeCoercionRules =
         InConversion ::
    --- End diff --
    
    Do you need to run this rule twice?


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] Add new type coercion to compa...

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

    https://github.com/apache/spark/pull/18853#discussion_r150704512
  
    --- Diff: docs/sql-programming-guide.md ---
    @@ -1490,6 +1490,13 @@ that these options will be deprecated in future release as more optimizations ar
           Configures the number of partitions to use when shuffling data for joins or aggregations.
         </td>
       </tr>
    +  <tr>
    +    <td><code>spark.sql.typeCoercion.mode</code></td>
    +    <td><code>legacy</code></td>
    --- End diff --
    
    -> `default`


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #81876 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81876/testReport)** for PR 18853 at commit [`844aec7`](https://github.com/apache/spark/commit/844aec7f4022140921d485b89fc240846bd05ac3).


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #81613 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81613/testReport)** for PR 18853 at commit [`522c4cd`](https://github.com/apache/spark/commit/522c4cddbd1ac0634fe5381d8c95df64c1eeb9c6).
     * 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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #88780 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88780/testReport)** for PR 18853 at commit [`81067b9`](https://github.com/apache/spark/commit/81067b984b95e04429db1ad19e6161c31a8228c0).
     * 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 #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #85840 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85840/testReport)** for PR 18853 at commit [`97a071d`](https://github.com/apache/spark/commit/97a071d91ec25159bba655b2bd9f6e2134d87088).


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] Add new type coercion to compa...

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

    https://github.com/apache/spark/pull/18853#discussion_r155364744
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
    @@ -1015,6 +1015,18 @@ object SQLConf {
           .booleanConf
           .createWithDefault(true)
     
    +  val typeCoercionMode =
    +    buildConf("spark.sql.typeCoercion.mode")
    +      .doc("The 'default' typeCoercion mode was used in spark prior to 2.3, " +
    +        "and so it continues to be the default to avoid breaking behavior. " +
    +        "However, it has logical inconsistencies. " +
    +        "The 'hive' mode is preferred for most new applications, " +
    +        "though it may require additional manual casting.")
    --- End diff --
    
    The same here.


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #85841 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85841/testReport)** for PR 18853 at commit [`408e889`](https://github.com/apache/spark/commit/408e889caa8d61b7267f0f391be4af5fde82a0c9).
     * 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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/95733/
    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 #18853: [SPARK-21646][SQL] CommonType for binary comparis...

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

    https://github.com/apache/spark/pull/18853#discussion_r139774828
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala ---
    @@ -352,11 +374,16 @@ object TypeCoercion {
             p.makeCopy(Array(Cast(left, TimestampType), right))
           case p @ Equality(left @ TimestampType(), right @ StringType()) =>
             p.makeCopy(Array(left, Cast(right, TimestampType)))
    -
           case p @ BinaryComparison(left, right)
    -        if findCommonTypeForBinaryComparison(left.dataType, right.dataType).isDefined =>
    +        if !plan.conf.binaryComparisonCompatibleWithHive &&
    +          findCommonTypeForBinaryComparison(left.dataType, right.dataType).isDefined =>
             val commonType = findCommonTypeForBinaryComparison(left.dataType, right.dataType).get
             p.makeCopy(Array(castExpr(left, commonType), castExpr(right, commonType)))
    +      case p @ BinaryComparison(left, right)
    +        if plan.conf.binaryComparisonCompatibleWithHive &&
    --- End diff --
    
    I am fine to have two separate rules, but we can call the shared functions. 


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    cc @wangyum 


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #88778 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88778/testReport)** for PR 18853 at commit [`81067b9`](https://github.com/apache/spark/commit/81067b984b95e04429db1ad19e6161c31a8228c0).
     * 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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/83687/
    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 #18853: [SPARK-21646][SQL] CommonType for binary comparis...

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

    https://github.com/apache/spark/pull/18853#discussion_r140801036
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
    @@ -2677,4 +2677,142 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
           checkAnswer(df, Row(1, 1, 1))
         }
       }
    +
    +  test("SPARK-21646: CommonTypeForBinaryComparison: StringType vs NumericType") {
    +    withTempView("v") {
    +      val str1 = Long.MaxValue.toString + "1"
    +      val str2 = Int.MaxValue.toString + "1"
    +      val str3 = "10"
    +      Seq(str1, str2, str3).toDF("c1").createOrReplaceTempView("v")
    +      withSQLConf(SQLConf.typeCoercionMode.key -> "hive") {
    +        checkAnswer(sql("SELECT c1 from v where c1 > 0"),
    +          Row(str1) :: Row(str2) :: Row(str3) :: Nil)
    +        checkAnswer(sql("SELECT c1 from v where c1 > 0L"),
    +          Row(str1) :: Row(str2) :: Row(str3) :: Nil)
    +      }
    +
    +      withSQLConf(SQLConf.typeCoercionMode.key -> "default") {
    +        checkAnswer(sql("SELECT c1 from v where c1 > 0"), Row(str3) :: Nil)
    +        checkAnswer(sql("SELECT c1 from v where c1 > 0L"), Row(str2) :: Row(str3) :: Nil)
    +      }
    +    }
    +  }
    +
    +  test("SPARK-21646: CommonTypeForBinaryComparison: DoubleType vs IntegerType") {
    +    withTempView("v") {
    +      Seq(("0", 1), ("-0.4", 2), ("0.6", 3)).toDF("c1", "c2").createOrReplaceTempView("v")
    +      withSQLConf(SQLConf.typeCoercionMode.key -> "hive") {
    +        checkAnswer(sql("SELECT c1 FROM v WHERE c1 = 0"), Seq(Row("0")))
    +        checkAnswer(sql("SELECT c1 FROM v WHERE c1 = 0L"), Seq(Row("0")))
    +        checkAnswer(sql("SELECT c1 FROM v WHERE c1 = 0.0"), Seq(Row("0")))
    +        checkAnswer(sql("SELECT c1 FROM v WHERE c1 = -0.4"), Seq(Row("-0.4")))
    +        checkAnswer(sql("SELECT count(*) FROM v WHERE c1 > 0"), Row(1) :: Nil)
    +      }
    +
    +      withSQLConf(SQLConf.typeCoercionMode.key -> "default") {
    +        checkAnswer(sql("SELECT c1 FROM v WHERE c1 = 0"), Seq(Row("0"), Row("-0.4"), Row("0.6")))
    +        checkAnswer(sql("SELECT c1 FROM v WHERE c1 = 0L"), Seq(Row("0"), Row("-0.4"), Row("0.6")))
    +        checkAnswer(sql("SELECT c1 FROM v WHERE c1 = 0.0"), Seq(Row("0")))
    +        checkAnswer(sql("SELECT c1 FROM v WHERE c1 = -0.4"), Seq(Row("-0.4")))
    +        checkAnswer(sql("SELECT count(*) FROM v WHERE c1 > 0"), Row(0) :: Nil)
    +      }
    +    }
    +  }
    +
    +  test("SPARK-21646: CommonTypeForBinaryComparison: StringType vs DateType") {
    +    withTempView("v") {
    +      val v1 = Date.valueOf("2017-09-22")
    +      val v2 = Date.valueOf("2017-09-09")
    +      Seq(v1, v2).toDF("c1").createTempView("v")
    +      withSQLConf(SQLConf.typeCoercionMode.key -> "hive") {
    +        checkAnswer(sql("select c1 from v where c1 > '2017-8-1'"), Row(v1) :: Row(v2) :: Nil)
    +        checkAnswer(sql("select c1 from v where c1 > cast('2017-8-1' as date)"),
    +          Row(v1) :: Row(v2) :: Nil)
    +      }
    +
    +      withSQLConf(SQLConf.typeCoercionMode.key -> "default") {
    +        checkAnswer(sql("select c1 from v where c1 > '2017-8-1'"), Nil)
    +        checkAnswer(sql("select c1 from v where c1 > cast('2017-8-1' as date)"),
    +          Row(v1) :: Row(v2) :: Nil)
    +      }
    +    }
    +  }
    +
    +  test("SPARK-21646: CommonTypeForBinaryComparison: StringType vs TimestampType") {
    +    withTempView("v") {
    +      val v1 = Timestamp.valueOf("2017-07-21 23:42:12.123")
    +      val v2 = Timestamp.valueOf("2017-08-21 23:42:12.123")
    +      Seq(v1, v2).toDF("c1").createTempView("v")
    +      withSQLConf(SQLConf.typeCoercionMode.key -> "hive") {
    +        checkAnswer(sql("select c1 from v where c1 > '2017-8-1'"), Row(v2) :: Nil)
    +        checkAnswer(sql("select c1 from v where c1 > cast('2017-8-1' as timestamp)"),
    +          Row(v2) :: Nil)
    +      }
    +
    +      withSQLConf(SQLConf.typeCoercionMode.key -> "default") {
    +        checkAnswer(sql("select c1 from v where c1 > '2017-8-1'"), Nil)
    +        checkAnswer(sql("select c1 from v where c1 > cast('2017-8-1' as timestamp)"),
    +          Row(v2) :: Nil)
    --- End diff --
    
    perhaps there should also be a comparison for a time with more degrees of precision?  It seems from the original discussion in https://issues.apache.org/jira/browse/SPARK-8420?focusedCommentId=14592654&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14592654 this was one of the concerns, eg. with `'1969-12-31 16:00:00'` `'1969-12-31 16:00:00.0'`


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    The JIRA was just opened. I will create an example and open many sub-tasks. Feel free to take them if you have bandwidth. 


---

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


[GitHub] spark pull request #18853: [SPARK-21646][SQL] BinaryComparison shouldn't aut...

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

    https://github.com/apache/spark/pull/18853#discussion_r131929813
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala ---
    @@ -354,6 +354,14 @@ object TypeCoercion {
             p.makeCopy(Array(left, Cast(right, TimestampType)))
     
           case p @ BinaryComparison(left, right)
    +        if left.isInstanceOf[AttributeReference] && right.isInstanceOf[Literal] =>
    +        p.makeCopy(Array(left, castExpr(right, left.dataType)))
    --- End diff --
    
    We need to cover all the same cases, but it seems this fix couldn't do, for example;
    ```
    scala> spark.udf.register("testUdf", () => "85908509832958239058032")
    scala> sql("select * from values (1) where testUdf() > 1").explain
    == Physical Plan ==
    *Filter (cast(UDF:testUdf() as int) > 1)
    +- LocalTableScan [col1#104]
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #81934 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81934/testReport)** for PR 18853 at commit [`7812018`](https://github.com/apache/spark/commit/7812018b41957dbc38bf30f3e31c9c2c49b22c1c).
     * 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 #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #82554 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82554/testReport)** for PR 18853 at commit [`2ada11a`](https://github.com/apache/spark/commit/2ada11a36d9fe9aa0f1048b535ac2da2a129c49f).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class TypeCoercionModeSuite extends SparkFunSuite with BeforeAndAfterAll `


---

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


[GitHub] spark pull request #18853: [SPARK-21646][SQL] CommonType for binary comparis...

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

    https://github.com/apache/spark/pull/18853#discussion_r140802924
  
    --- Diff: docs/sql-programming-guide.md ---
    @@ -1460,6 +1460,13 @@ that these options will be deprecated in future release as more optimizations ar
           Configures the number of partitions to use when shuffling data for joins or aggregations.
         </td>
       </tr>
    +  <tr>
    +    <td><code>spark.sql.typeCoercion.mode</code></td>
    +    <td><code>default</code></td>
    +    <td>
    +        Whether compatible with Hive. Available options are <code>default</code> and <code>hive</code>.
    --- End diff --
    
    This description feels inadequate to me.  I think most users will think "hive" means "old, legacy way of doing things and "default" means "new, better, spark way of doing things".  But I haven't heard an argument in favor of the "default" behavior, just that we don't want to have a breaking change of behavior.
    
    So (a) I'd advocate that we rename "default" to "legacy", or something else along those lines.  I do think it should be the default value, to avoid changing behavior.
    and (b) I think the doc section here should more clearly indicate the difference, eg. "The 'legacy' typeCoercion mode was used in spark prior to 2.3, and so it continues to be the default to avoid breaking behavior.  However, it has logical inconsistencies.  The 'hive' mode is preferred for most new applications, though it may require additional manual casting.
    
    I am even wondering if we should have a 3rd option, to not implicit cast across type categories, eg. like postgres, as this avoids nasty surprises for the user.  While the casts are convenient, when it doesn't work there is very little indication to the user that anything went wrong -- most likely they'll just keep continue processing data though the results don't actually have the semantics they want.


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #81999 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81999/testReport)** for PR 18853 at commit [`27d5b13`](https://github.com/apache/spark/commit/27d5b13190267637ed9dae8d7cb6aa762d7fa320).
     * 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 #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    Thank you for your investigation! I think we need to introduce a type inference conf for it. To avoid impacting the existing Spark users, we should keep the existing behaviors. 


---

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


[GitHub] spark pull request #18853: [SPARK-21646][SQL] CommonType for binary comparis...

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

    https://github.com/apache/spark/pull/18853#discussion_r139464749
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala ---
    @@ -925,6 +925,12 @@ object SQLConf {
           .intConf
           .createWithDefault(10000)
     
    +  val BINARY_COMPARISON_COMPATIBLE_WITH_HIVE =
    +    buildConf("spark.sql.binary.comparison.compatible.with.hive")
    +      .doc("Whether compatible with Hive when binary comparison.")
    --- End diff --
    
    Binary comparison is just one of the implicit type casting cases. 


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #82537 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82537/testReport)** for PR 18853 at commit [`8da0cf0`](https://github.com/apache/spark/commit/8da0cf07416372850fac302b42bebebbcfd88ff4).


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] Add new type coercion to compa...

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

    https://github.com/apache/spark/pull/18853#discussion_r155365351
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala ---
    @@ -61,6 +60,18 @@ object TypeCoercion {
           WindowFrameCoercion ::
           Nil
     
    +  def rules(conf: SQLConf): List[Rule[LogicalPlan]] = {
    +    if (conf.isHiveTypeCoercionMode) {
    +      commonTypeCoercionRules :+
    +        HiveInConversion :+
    +        HivePromoteStrings
    +    } else {
    +      commonTypeCoercionRules :+
    +        InConversion :+
    +        PromoteStrings
    --- End diff --
    
    Rename them to `NativeInConversion` and `NativePromoteStrings `


---

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


[GitHub] spark pull request #18853: [SPARK-21646][SQL] CommonType for binary comparis...

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

    https://github.com/apache/spark/pull/18853#discussion_r137965884
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameSuite.scala ---
    @@ -1966,7 +1966,7 @@ class DataFrameSuite extends QueryTest with SharedSQLContext {
     
       test("SPARK-17913: compare long and string type column may return confusing result") {
         val df = Seq(123L -> "123", 19157170390056973L -> "19157170390056971").toDF("i", "j")
    -    checkAnswer(df.select($"i" === $"j"), Row(true) :: Row(false) :: Nil)
    +    checkAnswer(df.select($"i" === $"j"), Row(true) :: Row(true) :: Nil)
    --- End diff --
    
    To compatible with Hive, MySQL and Oracle:
    <img width="1319" alt="oracle" src="https://user-images.githubusercontent.com/5399861/30254791-f2e6cc28-96cf-11e7-817e-c837c42d7504.png">



---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    @wangyum Could you update the PR based on https://github.com/apache/spark/pull/19874?


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #81871 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81871/testReport)** for PR 18853 at commit [`844aec7`](https://github.com/apache/spark/commit/844aec7f4022140921d485b89fc240846bd05ac3).
     * 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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    cc @gatorsmile 


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    CC @gatorsmile, @cloud-fan


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #82537 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/82537/testReport)** for PR 18853 at commit [`8da0cf0`](https://github.com/apache/spark/commit/8da0cf07416372850fac302b42bebebbcfd88ff4).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `class TypeCoercionSuite extends QueryTest `


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    
    I provide 2 SQL scripts to validate the different result between Spark and Hive:
    
    | Engine | [SPARK_21646_1.txt](https://github.com/apache/spark/files/1305185/SPARK_21646_1.txt) | [SPARK_21646_2.txt](https://github.com/apache/spark/files/1305186/SPARK_21646_2.txt)  |
    | ------------- |:-------------:| -----:|
    |  [Hive-2.2.0](http://mirrors.hust.edu.cn/apache/hive/hive-2.2.0/apache-hive-2.2.0-bin.tar.gz)    | <left>0.1</br>0.6</br>100</br>1111111111111111111111111111111111111111111111111</left> |2017-09-14 |
    |  Spark    | 100     | - |



---

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


[GitHub] spark pull request #18853: [SPARK-21646][SQL] CommonType for binary comparis...

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

    https://github.com/apache/spark/pull/18853#discussion_r140797299
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
    @@ -2677,4 +2677,142 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
           checkAnswer(df, Row(1, 1, 1))
         }
       }
    +
    +  test("SPARK-21646: CommonTypeForBinaryComparison: StringType vs NumericType") {
    +    withTempView("v") {
    +      val str1 = Long.MaxValue.toString + "1"
    +      val str2 = Int.MaxValue.toString + "1"
    +      val str3 = "10"
    +      Seq(str1, str2, str3).toDF("c1").createOrReplaceTempView("v")
    +      withSQLConf(SQLConf.typeCoercionMode.key -> "hive") {
    +        checkAnswer(sql("SELECT c1 from v where c1 > 0"),
    +          Row(str1) :: Row(str2) :: Row(str3) :: Nil)
    +        checkAnswer(sql("SELECT c1 from v where c1 > 0L"),
    +          Row(str1) :: Row(str2) :: Row(str3) :: Nil)
    +      }
    +
    +      withSQLConf(SQLConf.typeCoercionMode.key -> "default") {
    +        checkAnswer(sql("SELECT c1 from v where c1 > 0"), Row(str3) :: Nil)
    +        checkAnswer(sql("SELECT c1 from v where c1 > 0L"), Row(str2) :: Row(str3) :: Nil)
    +      }
    +    }
    +  }
    +
    +  test("SPARK-21646: CommonTypeForBinaryComparison: DoubleType vs IntegerType") {
    +    withTempView("v") {
    +      Seq(("0", 1), ("-0.4", 2), ("0.6", 3)).toDF("c1", "c2").createOrReplaceTempView("v")
    +      withSQLConf(SQLConf.typeCoercionMode.key -> "hive") {
    +        checkAnswer(sql("SELECT c1 FROM v WHERE c1 = 0"), Seq(Row("0")))
    +        checkAnswer(sql("SELECT c1 FROM v WHERE c1 = 0L"), Seq(Row("0")))
    +        checkAnswer(sql("SELECT c1 FROM v WHERE c1 = 0.0"), Seq(Row("0")))
    +        checkAnswer(sql("SELECT c1 FROM v WHERE c1 = -0.4"), Seq(Row("-0.4")))
    +        checkAnswer(sql("SELECT count(*) FROM v WHERE c1 > 0"), Row(1) :: Nil)
    +      }
    +
    +      withSQLConf(SQLConf.typeCoercionMode.key -> "default") {
    +        checkAnswer(sql("SELECT c1 FROM v WHERE c1 = 0"), Seq(Row("0"), Row("-0.4"), Row("0.6")))
    +        checkAnswer(sql("SELECT c1 FROM v WHERE c1 = 0L"), Seq(Row("0"), Row("-0.4"), Row("0.6")))
    +        checkAnswer(sql("SELECT c1 FROM v WHERE c1 = 0.0"), Seq(Row("0")))
    +        checkAnswer(sql("SELECT c1 FROM v WHERE c1 = -0.4"), Seq(Row("-0.4")))
    +        checkAnswer(sql("SELECT count(*) FROM v WHERE c1 > 0"), Row(0) :: Nil)
    +      }
    +    }
    +  }
    +
    +  test("SPARK-21646: CommonTypeForBinaryComparison: StringType vs DateType") {
    +    withTempView("v") {
    +      val v1 = Date.valueOf("2017-09-22")
    +      val v2 = Date.valueOf("2017-09-09")
    +      Seq(v1, v2).toDF("c1").createTempView("v")
    +      withSQLConf(SQLConf.typeCoercionMode.key -> "hive") {
    +        checkAnswer(sql("select c1 from v where c1 > '2017-8-1'"), Row(v1) :: Row(v2) :: Nil)
    +        checkAnswer(sql("select c1 from v where c1 > cast('2017-8-1' as date)"),
    +          Row(v1) :: Row(v2) :: Nil)
    +      }
    +
    +      withSQLConf(SQLConf.typeCoercionMode.key -> "default") {
    +        checkAnswer(sql("select c1 from v where c1 > '2017-8-1'"), Nil)
    +        checkAnswer(sql("select c1 from v where c1 > cast('2017-8-1' as date)"),
    +          Row(v1) :: Row(v2) :: Nil)
    +      }
    +    }
    +  }
    +
    +  test("SPARK-21646: CommonTypeForBinaryComparison: StringType vs TimestampType") {
    +    withTempView("v") {
    +      val v1 = Timestamp.valueOf("2017-07-21 23:42:12.123")
    +      val v2 = Timestamp.valueOf("2017-08-21 23:42:12.123")
    +      Seq(v1, v2).toDF("c1").createTempView("v")
    +      withSQLConf(SQLConf.typeCoercionMode.key -> "hive") {
    +        checkAnswer(sql("select c1 from v where c1 > '2017-8-1'"), Row(v2) :: Nil)
    +        checkAnswer(sql("select c1 from v where c1 > cast('2017-8-1' as timestamp)"),
    +          Row(v2) :: Nil)
    +      }
    +
    +      withSQLConf(SQLConf.typeCoercionMode.key -> "default") {
    +        checkAnswer(sql("select c1 from v where c1 > '2017-8-1'"), Nil)
    +        checkAnswer(sql("select c1 from v where c1 > cast('2017-8-1' as timestamp)"),
    +          Row(v2) :: Nil)
    --- End diff --
    
    I think we should include a comparison which is only the year, eg. ` > '2014'`, as that was listed as the motivation for the "default" behavior in the code comments.


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] BinaryComparison shouldn't auto cast ...

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

    https://github.com/apache/spark/pull/18853
  
    If we change this behaviour, I think we better modify code in `findCommonTypeForBinaryComparison` of `TypeCoercion` instead of your pr: https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala#L130
    ```
      val findCommonTypeForBinaryComparison: (DataType, DataType) => Option[DataType] = {
        ...
        case (l: StringType, r: AtomicType) if r != StringType => Some(r)
        case (l: AtomicType, r: StringType) if (l != StringType) => Some(l)
        ...
      }
    ```
    As another option, we could cast `NumericType` to wider `DecimalType`? Since this change could have some runtime overhead and behaviour change, I'm not sure this is acceptable. cc @gatorsmile @cloud-fan 
    
    
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #85846 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85846/testReport)** for PR 18853 at commit [`408e889`](https://github.com/apache/spark/commit/408e889caa8d61b7267f0f391be4af5fde82a0c9).
     * 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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] BinaryComparison shouldn't auto cast ...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #81606 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/81606/testReport)** for PR 18853 at commit [`8d37c72`](https://github.com/apache/spark/commit/8d37c720b9cb1e3e4b8c4cb6a486ee9c1d07d59c).
     * 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 #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] BinaryComparison shouldn't auto cast ...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

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


---

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


[GitHub] spark issue #18853: [SPARK-21646][SQL] CommonType for binary comparison

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

    https://github.com/apache/spark/pull/18853
  
    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 #18853: [SPARK-21646][SQL] Add new type coercion to compatible w...

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

    https://github.com/apache/spark/pull/18853
  
    **[Test build #88778 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/88778/testReport)** for PR 18853 at commit [`81067b9`](https://github.com/apache/spark/commit/81067b984b95e04429db1ad19e6161c31a8228c0).


---

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