[GitHub] [spark] cloud-fan commented on a change in pull request #26112: [SPARK-29364][SQL] Return an interval from date subtract according to SQL standard

2019-10-15 Thread GitBox
cloud-fan commented on a change in pull request #26112: [SPARK-29364][SQL] 
Return an interval from date subtract according to SQL standard
URL: https://github.com/apache/spark/pull/26112#discussion_r335263183
 
 

 ##
 File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
 ##
 @@ -849,12 +850,13 @@ object TypeCoercion {
   case Add(l @ DateType(), r @ IntegerType()) => DateAdd(l, r)
   case Add(l @ IntegerType(), r @ DateType()) => DateAdd(r, l)
   case Subtract(l @ DateType(), r @ IntegerType()) => DateSub(l, r)
-  case Subtract(l @ DateType(), r @ DateType()) => DateDiff(l, r)
-  case Subtract(l @ TimestampType(), r @ TimestampType()) => 
TimestampDiff(l, r)
+  case Subtract(l @ DateType(), r @ DateType()) => SubtractDates(l, r)
 
 Review comment:
   seems like we should use `DateDiff` if the dialect is pgsql.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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



[GitHub] [spark] cloud-fan commented on a change in pull request #26112: [SPARK-29364][SQL] Return an interval from date subtract according to SQL standard

2019-10-15 Thread GitBox
cloud-fan commented on a change in pull request #26112: [SPARK-29364][SQL] 
Return an interval from date subtract according to SQL standard
URL: https://github.com/apache/spark/pull/26112#discussion_r335011887
 
 

 ##
 File path: docs/sql-migration-guide.md
 ##
 @@ -217,6 +217,8 @@ license: |
 
   - Since Spark 3.0, the `size` function returns `NULL` for the `NULL` input. 
In Spark version 2.4 and earlier, this function gives `-1` for the same input. 
To restore the behavior before Spark 3.0, you can set 
`spark.sql.legacy.sizeOfNull` to `true`.
 
+  - In Spark version 2.4 and earlier, dates subtraction `date1` - `date2` 
gives the number of days from `date1` to `date2`. Since Spark 3.0, the 
expression has the `INTERVAL` type and returns an interval between two dates. 
To get the number of days, you can set 
`spark.sql.legacy.datesSubtraction.enabled` to `true` or use the `datediff` 
function.
 
 Review comment:
   good news! then we can remove the config and migration guide :)


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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



[GitHub] [spark] cloud-fan commented on a change in pull request #26112: [SPARK-29364][SQL] Return an interval from date subtract according to SQL standard

2019-10-15 Thread GitBox
cloud-fan commented on a change in pull request #26112: [SPARK-29364][SQL] 
Return an interval from date subtract according to SQL standard
URL: https://github.com/apache/spark/pull/26112#discussion_r334904771
 
 

 ##
 File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
 ##
 @@ -849,12 +850,18 @@ object TypeCoercion {
   case Add(l @ DateType(), r @ IntegerType()) => DateAdd(l, r)
   case Add(l @ IntegerType(), r @ DateType()) => DateAdd(r, l)
   case Subtract(l @ DateType(), r @ IntegerType()) => DateSub(l, r)
-  case Subtract(l @ DateType(), r @ DateType()) => DateDiff(l, r)
-  case Subtract(l @ TimestampType(), r @ TimestampType()) => 
TimestampDiff(l, r)
+  case Subtract(l @ DateType(), r @ DateType()) =>
+if (SQLConf.get.getConf(SQLConf.LEGACY_DATES_SUBTRACTION)) {
 
 Review comment:
   We want to switch to the new behavior by default, with a legacy config to 
restore the behavior. This is different from other ANSI behaviors, which we 
don't want to switch to by default.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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



[GitHub] [spark] cloud-fan commented on a change in pull request #26112: [SPARK-29364][SQL] Return an interval from date subtract according to SQL standard

2019-10-15 Thread GitBox
cloud-fan commented on a change in pull request #26112: [SPARK-29364][SQL] 
Return an interval from date subtract according to SQL standard
URL: https://github.com/apache/spark/pull/26112#discussion_r334904771
 
 

 ##
 File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
 ##
 @@ -849,12 +850,18 @@ object TypeCoercion {
   case Add(l @ DateType(), r @ IntegerType()) => DateAdd(l, r)
   case Add(l @ IntegerType(), r @ DateType()) => DateAdd(r, l)
   case Subtract(l @ DateType(), r @ IntegerType()) => DateSub(l, r)
-  case Subtract(l @ DateType(), r @ DateType()) => DateDiff(l, r)
-  case Subtract(l @ TimestampType(), r @ TimestampType()) => 
TimestampDiff(l, r)
+  case Subtract(l @ DateType(), r @ DateType()) =>
+if (SQLConf.get.getConf(SQLConf.LEGACY_DATES_SUBTRACTION)) {
 
 Review comment:
   We want to switch to the new behavior by default, with a legacy config to 
restore the old behavior. This is different from other ANSI behaviors, which we 
don't want to switch to by default.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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



[GitHub] [spark] cloud-fan commented on a change in pull request #26112: [SPARK-29364][SQL] Return an interval from date subtract according to SQL standard

2019-10-14 Thread GitBox
cloud-fan commented on a change in pull request #26112: [SPARK-29364][SQL] 
Return an interval from date subtract according to SQL standard
URL: https://github.com/apache/spark/pull/26112#discussion_r334563464
 
 

 ##
 File path: docs/sql-migration-guide.md
 ##
 @@ -217,6 +217,8 @@ license: |
 
   - Since Spark 3.0, the `size` function returns `NULL` for the `NULL` input. 
In Spark version 2.4 and earlier, this function gives `-1` for the same input. 
To restore the behavior before Spark 3.0, you can set 
`spark.sql.legacy.sizeOfNull` to `true`.
 
+  - In Spark version 2.4 and earlier, dates subtraction `date1` - `date2` 
gives the number of days from `date1` to `date2`. Since Spark 3.0, the 
expression has the `INTERVAL` type and returns an interval between two dates. 
To get the number of days, use the `datediff` function.
 
 Review comment:
   shall we add a legacy config? 


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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



[GitHub] [spark] cloud-fan commented on a change in pull request #26112: [SPARK-29364][SQL] Return an interval from date subtract according to SQL standard

2019-10-14 Thread GitBox
cloud-fan commented on a change in pull request #26112: [SPARK-29364][SQL] 
Return an interval from date subtract according to SQL standard
URL: https://github.com/apache/spark/pull/26112#discussion_r334563257
 
 

 ##
 File path: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/TypeCoercion.scala
 ##
 @@ -849,7 +849,7 @@ object TypeCoercion {
   case Add(l @ DateType(), r @ IntegerType()) => DateAdd(l, r)
   case Add(l @ IntegerType(), r @ DateType()) => DateAdd(r, l)
   case Subtract(l @ DateType(), r @ IntegerType()) => DateSub(l, r)
-  case Subtract(l @ DateType(), r @ DateType()) => DateDiff(l, r)
+  case Subtract(l @ DateType(), r @ DateType()) => SubtractDates(l, r)
   case Subtract(l @ TimestampType(), r @ TimestampType()) => 
TimestampDiff(l, r)
 
 Review comment:
   yea please do.


This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

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