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 (via GitHub)" <gi...@apache.org> on 2023/02/27 09:30:19 UTC
[GitHub] [spark] wangyum opened a new pull request, #40190: [SPARK-42597][SQL] UnwrapCastInBinaryComparison support unwrap timestamp type
wangyum opened a new pull request, #40190:
URL: https://github.com/apache/spark/pull/40190
### What changes were proposed in this pull request?
This PR enhance `UnwrapCastInBinaryComparison` to support unwrap timestamp type.
The unwrap rule is:
```
GreaterThan(Cast(ts, TimestampType), date) -> GreaterThanOrEqual(ts, Cast(date+ 1, TimestampType))
GreaterThanOrEqual(Cast(ts, TimestampType), date) -> GreaterThanOrEqual(ts, Cast(date, TimestampType))
LessThan(Cast(ts, TimestampType), date) -> LessThan(ts, Cast(date, TimestampType))
LessThanOrEqual(Cast(ts, TimestampType), date) -> LessThan(ts, Cast(d, Cast(date + 1, TimestampType)))
EqualTo(Cast(ts, TimestampType), date) -> And(GreaterThanOrEqual(ts, Cast(date, TimestampType)), LessThan(ts, Cast(date + 1,, TimestampType)))
```
### Why are the changes needed?
Improve query performance.
A common use case. We store cold data in HDFS by partition, store hot data in MySQL, and then union all the results. The filter in the MySQL branch cannot be pushed down, which affects performance:
```sql
CREATE TABLE t1(id bigint, ts timestamp, dt date) using parquet PARTITIONED BY (dt);
CREATE TABLE t2(id bigint, ts timestamp) using org.apache.spark.sql.jdbc OPTIONS (`url` '***', `dbtable` 'db.t2', `user` 'spark', `password` '***');
CREATE VIEW v1 AS SELECT * FROM t1 UNION ALL SELECT *, to_date(ts) FROM t2;
SELECT * FROM v1 WHERE dt BETWEEN '2023-02-22' AND '2023-02-24';
```
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Unit test.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #40190: [SPARK-42597][SQL] UnwrapCastInBinaryComparison support unwrap timestamp type
Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1119729036
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -293,6 +299,34 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
}
}
+ /**
+ * Move the cast to the literal side, because we can only get the minimum value of timestamp,
+ * so some BinaryComparison needs to be changed,
+ * such as CAST(ts AS date) > DATE '2023-01-01' ===> ts >= TIMESTAMP '2023-01-02 00:00:00'
+ */
+ private def simplifyDateComparison(
+ exp: BinaryComparison,
+ fromExp: Expression,
+ date: Literal,
+ tz: Option[String],
+ evalMode: EvalMode.Value): Expression = {
+ val dateAddOne = DateAdd(date, Literal(1, IntegerType))
+ exp match {
+ case _: GreaterThan =>
+ GreaterThanOrEqual(fromExp, Cast(dateAddOne, fromExp.dataType, tz, evalMode))
+ case _: GreaterThanOrEqual =>
+ GreaterThanOrEqual(fromExp, Cast(date, fromExp.dataType, tz, evalMode))
+ case _: LessThan =>
+ LessThan(fromExp, Cast(date, fromExp.dataType, tz, evalMode))
+ case _: LessThanOrEqual =>
+ LessThan(fromExp, Cast(dateAddOne, fromExp.dataType, tz, evalMode))
+ case _: EqualTo | _: EqualNullSafe =>
+ And(GreaterThanOrEqual(fromExp, Cast(date, fromExp.dataType, tz, evalMode)),
+ LessThan(fromExp, Cast(dateAddOne, fromExp.dataType, tz, evalMode)))
Review Comment:
`date` should not be null literal here.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #40190: [SPARK-42597][SQL] UnwrapCastInBinaryComparison support unwrap timestamp type
Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1119825935
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -293,6 +299,34 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
}
}
+ /**
+ * Move the cast to the literal side, because we can only get the minimum value of timestamp,
+ * so some BinaryComparison needs to be changed,
+ * such as CAST(ts AS date) > DATE '2023-01-01' ===> ts >= TIMESTAMP '2023-01-02 00:00:00'
Review Comment:
Yes. It is.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #40190: [SPARK-42597][SQL] UnwrapCastInBinaryComparison support unwrap timestamp type
Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1122758398
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -293,6 +299,34 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
}
}
+ /**
+ * Move the cast to the literal side, because we can only get the minimum value of timestamp,
+ * so some BinaryComparison needs to be changed,
+ * such as CAST(ts AS date) > DATE '2023-01-01' ===> ts >= TIMESTAMP '2023-01-02 00:00:00'
Review Comment:
Yes, it should be `'2023-01-02 00:00:00' > '2023-01-01 23:59:59.999999'` if do not change `>` to `>=`.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1129431767
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -293,6 +299,34 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
}
}
+ /**
Review Comment:
Have added it.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #40190: [SPARK-42597][SQL] UnwrapCastInBinaryComparison support unwrap timestamp type
Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1119730106
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -133,6 +133,11 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
if canImplicitlyCast(fromExp, toType, literalType) =>
simplifyNumericComparison(be, fromExp, toType, value)
+ case be @ BinaryComparison(
+ Cast(fromExp, _, timeZoneId, evalMode), date @ Literal(_, DateType))
+ if fromExp.dataType == TimestampType =>
Review Comment:
Added it.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #40190: [SPARK-42597][SQL] UnwrapCastInBinaryComparison support unwrap timestamp type
Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1123984775
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -293,6 +299,34 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
}
}
+ /**
+ * Move the cast to the literal side, because we can only get the minimum value of timestamp,
+ * so some BinaryComparison needs to be changed,
+ * such as CAST(ts AS date) > DATE '2023-01-01' ===> ts >= TIMESTAMP '2023-01-02 00:00:00'
Review Comment:
It is because it is easy to get the start of the day timestamp, not easy to get the end of the day timestamp.
```sql
spark-sql> select cast(date'2023-01-01' as timestamp);
2023-01-01 00:00:00
```
vs
```sql
spark-sql> select cast(concat(cast(date'2023-01-01' as string), ' 23:59:59.999999') as timestamp);
2023-01-01 23:59:59.999999
```
It will truncate if casting from long type to timestamp type:
```sql
spark-sql> select cast(cast(cast(date'2023-01-01' as timestamp) as long) + (24* 60 * 60 - 1) as timestamp);
2023-01-01 23:59:59
```
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1130705751
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -113,7 +114,7 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
// literal and cast side, then process the result and swap the literal and cast again to
// restore the original order.
case BinaryComparison(Literal(_, literalType), Cast(fromExp, toType, _, _))
- if canImplicitlyCast(fromExp, toType, literalType) =>
+ if needToCanonicalizeExpression(fromExp, toType, literalType) =>
Review Comment:
`simplifyNumericComparison` still only handles NumericComparison. Datetimes are handled by `unwrapDateToTimestamp`
<img width="807" alt="image" src="https://user-images.githubusercontent.com/5399861/223976814-d2219e64-38b2-4ded-bd5f-e66705d05756.png">
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #40190: [SPARK-42597][SQL] UnwrapCastInBinaryComparison support unwrap timestamp type
Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1119729827
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -133,6 +133,12 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
if canImplicitlyCast(fromExp, toType, literalType) =>
simplifyNumericComparison(be, fromExp, toType, value)
+ case be @ BinaryComparison(
+ Cast(fromExp, _, timeZoneId, evalMode), date @ Literal(value, DateType))
+ if (fromExp.dataType == TimestampType || fromExp.dataType == TimestampNTZType) &&
+ value != null =>
Review Comment:
null literal should be handled by `NullPropagation`.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1132445403
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparisonSuite.scala:
##########
@@ -368,6 +370,61 @@ class UnwrapCastInBinaryComparisonSuite extends PlanTest with ExpressionEvalHelp
assertEquivalent(castInt(f4) < t, trueIfNotNull(f4))
}
+ test("SPARK-42597: Support unwrap date to timestamp type") {
+ val tsRelation = LocalRelation($"a".timestamp, $"b".timestampNTZ)
+ val dateLit = Literal.create(Date.valueOf("2023-01-01"), DateType)
+ val nullLit = Literal.create(null, DateType)
+
+ def assertUnwrapDateToTimestamp(e1: Expression, e2: Expression): Unit = {
+ val plan = tsRelation.where(e1).analyze
+ val actual = Optimize.execute(plan)
+ val expected = tsRelation.where(e2).analyze
+ comparePlans(actual, expected)
+ }
+
+ assertUnwrapDateToTimestamp(
Review Comment:
or we can add timestamp test data, so that we can evaluate as well.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #40190: [SPARK-42597][SQL] UnwrapCastInBinaryComparison support unwrap timestamp type
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1119500020
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -133,6 +133,11 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
if canImplicitlyCast(fromExp, toType, literalType) =>
simplifyNumericComparison(be, fromExp, toType, value)
+ case be @ BinaryComparison(
+ Cast(fromExp, _, timeZoneId, evalMode), date @ Literal(_, DateType))
+ if fromExp.dataType == TimestampType =>
Review Comment:
we should handle ntz type as well.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a diff in pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1128241190
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparisonSuite.scala:
##########
@@ -368,6 +370,61 @@ class UnwrapCastInBinaryComparisonSuite extends PlanTest with ExpressionEvalHelp
assertEquivalent(castInt(f4) < t, trueIfNotNull(f4))
}
+ test("SPARK-42597: Support unwrap date to timestamp type") {
Review Comment:
could we add some tests in `UnwrapCastInComparisonEndToEndSuite` too?
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -293,6 +299,34 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
}
}
+ /**
Review Comment:
can we also update the doc for `UnwrapCastInBinaryComparison`?
> * Currently this only handles cases where:
> * 1). `fromType` (of `fromExp`) and `toType` are of numeric types (i.e., short, int, float, decimal, etc) or boolean type
> * 2). `fromType` can be safely coerced to `toType` without precision loss (e.g., short to int, int to long, but not long to int, nor int to boolean)
This is no longer true.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -350,7 +384,7 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
literalType: DataType): Boolean = {
toType.sameType(literalType) &&
!fromExp.foldable &&
- toType.isInstanceOf[NumericType] &&
+ (toType.isInstanceOf[NumericType] || toType.isInstanceOf[DateType]) &&
Review Comment:
+1
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum closed pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum closed pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
URL: https://github.com/apache/spark/pull/40190
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1161579303
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -293,6 +297,34 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
}
}
+ /**
+ * Move the cast to the literal side, because we can only get the minimum value of timestamp,
+ * so some BinaryComparison needs to be changed,
+ * such as CAST(ts AS date) > DATE '2023-01-01' ===> ts >= TIMESTAMP '2023-01-02 00:00:00'
+ */
+ private def unwrapDateToTimestamp(
+ exp: BinaryComparison,
+ fromExp: Expression,
+ date: Literal,
+ tz: Option[String],
+ evalMode: EvalMode.Value): Expression = {
+ val dateAddOne = DateAdd(date, Literal(1, IntegerType))
+ exp match {
+ case _: GreaterThan =>
+ GreaterThanOrEqual(fromExp, Cast(dateAddOne, fromExp.dataType, tz, evalMode))
+ case _: GreaterThanOrEqual =>
+ GreaterThanOrEqual(fromExp, Cast(date, fromExp.dataType, tz, evalMode))
+ case Equality(_, _) =>
Review Comment:
Thank you. will fix it soon.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #40190: [SPARK-42597][SQL] UnwrapCastInBinaryComparison support unwrap timestamp type
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1121305509
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -293,6 +299,34 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
}
}
+ /**
+ * Move the cast to the literal side, because we can only get the minimum value of timestamp,
+ * so some BinaryComparison needs to be changed,
+ * such as CAST(ts AS date) > DATE '2023-01-01' ===> ts >= TIMESTAMP '2023-01-02 00:00:00'
Review Comment:
then why do we change `>` to `>=` after the conversion?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on PR #40190:
URL: https://github.com/apache/spark/pull/40190#issuecomment-1467139918
Merged to master.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1129431132
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -361,6 +395,7 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
case (IntegerType, FloatType) => false
case (LongType, FloatType) => false
case (LongType, DoubleType) => false
+ case (TimestampType | TimestampNTZType, DateType) => true
Review Comment:
have moved the logic to a new function: `needToCanonicalizeExpression`.
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparisonSuite.scala:
##########
@@ -368,6 +370,61 @@ class UnwrapCastInBinaryComparisonSuite extends PlanTest with ExpressionEvalHelp
assertEquivalent(castInt(f4) < t, trueIfNotNull(f4))
}
+ test("SPARK-42597: Support unwrap date to timestamp type") {
Review Comment:
OK
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1129677751
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -43,6 +43,7 @@ import org.apache.spark.sql.types._
* decimal, etc) or boolean type
* 2). `fromType` can be safely coerced to `toType` without precision loss (e.g., short to int,
* int to long, but not long to int, nor int to boolean)
+ * 3). `fromExp` is a timestamp type and `toType` is a date type
Review Comment:
this is not a `3)`, but an additional supported case.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1127955295
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -350,7 +384,7 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
literalType: DataType): Boolean = {
toType.sameType(literalType) &&
!fromExp.foldable &&
- toType.isInstanceOf[NumericType] &&
+ (toType.isInstanceOf[NumericType] || toType.isInstanceOf[DateType]) &&
Review Comment:
do we need to change here? The new code does not call `canImplicitlyCast`
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -361,6 +395,7 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
case (IntegerType, FloatType) => false
case (LongType, FloatType) => false
case (LongType, DoubleType) => false
+ case (TimestampType | TimestampNTZType, DateType) => true
Review Comment:
ditto
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1129686241
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -113,7 +114,7 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
// literal and cast side, then process the result and swap the literal and cast again to
// restore the original order.
case BinaryComparison(Literal(_, literalType), Cast(fromExp, toType, _, _))
- if canImplicitlyCast(fromExp, toType, literalType) =>
+ if needToCanonicalizeExpression(fromExp, toType, literalType) =>
Review Comment:
OK now I got it. I feel `needToCanonicalizeExpression` is confusing and updating `canImplicitlyCast` directly is fine. We may need to rename `simplifyNumericComparison` as it handles datetime as well.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1132437369
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -43,6 +43,7 @@ import org.apache.spark.sql.types._
* decimal, etc) or boolean type
* 2). `fromType` can be safely coerced to `toType` without precision loss (e.g., short to int,
* int to long, but not long to int, nor int to boolean)
+ * 3). `fromExp` is a timestamp type and `toType` is a date type
Review Comment:
Can we simply update `1)`? `numeric type` -> `numeric or datetime type`
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1132445884
##########
sql/core/src/test/scala/org/apache/spark/sql/UnwrapCastInComparisonEndToEndSuite.scala:
##########
@@ -240,5 +242,27 @@ class UnwrapCastInComparisonEndToEndSuite extends QueryTest with SharedSparkSess
}
}
+ test("SPARK-42597: Support unwrap date type to timestamp type") {
+ val ts1 = Timestamp.valueOf("2023-01-01 23:59:59.099999")
+ val ts2 = Timestamp.valueOf("2023-01-01 23:59:59.999998")
+ val ts3 = Timestamp.valueOf("2023-01-02 23:59:59.000008")
+
+ withTable(t) {
+ Seq(ts1, ts2, ts3).toDF("ts").write.saveAsTable(t)
+ val df = spark.table(t)
+
+ checkAnswer(
+ df.where("cast(ts as date) > cast('2023-01-01' as date)"), Seq(ts3).map(Row(_)))
Review Comment:
it's more common to write date literal with `date'xxxx'`
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1132444507
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparisonSuite.scala:
##########
@@ -368,6 +370,61 @@ class UnwrapCastInBinaryComparisonSuite extends PlanTest with ExpressionEvalHelp
assertEquivalent(castInt(f4) < t, trueIfNotNull(f4))
}
+ test("SPARK-42597: Support unwrap date to timestamp type") {
+ val tsRelation = LocalRelation($"a".timestamp, $"b".timestampNTZ)
+ val dateLit = Literal.create(Date.valueOf("2023-01-01"), DateType)
+ val nullLit = Literal.create(null, DateType)
+
+ def assertUnwrapDateToTimestamp(e1: Expression, e2: Expression): Unit = {
+ val plan = tsRelation.where(e1).analyze
+ val actual = Optimize.execute(plan)
+ val expected = tsRelation.where(e2).analyze
+ comparePlans(actual, expected)
+ }
+
+ assertUnwrapDateToTimestamp(
Review Comment:
shall we just call `assertEquivalent` with `evaluate = false`?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1129430744
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -350,7 +384,7 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
literalType: DataType): Boolean = {
toType.sameType(literalType) &&
!fromExp.foldable &&
- toType.isInstanceOf[NumericType] &&
+ (toType.isInstanceOf[NumericType] || toType.isInstanceOf[DateType]) &&
Review Comment:
@cloud-fan @sunchao This is used to support `literal BinaryComparator cast(exp as timesatmp)`, e.g.: `DATE '2023-01-01' < CAST(ts AS date)`. I have moved the logic to a new function: `needToCanonicalizeExpression`.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #40190: [SPARK-42597][SQL] UnwrapCastInBinaryComparison support unwrap timestamp type
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1119787925
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -293,6 +299,34 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
}
}
+ /**
+ * Move the cast to the literal side, because we can only get the minimum value of timestamp,
+ * so some BinaryComparison needs to be changed,
+ * such as CAST(ts AS date) > DATE '2023-01-01' ===> ts >= TIMESTAMP '2023-01-02 00:00:00'
Review Comment:
If the date part of `ts` is greater than a date, then `ts` must be greater than the timestamp form of this date, right?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sunchao commented on a diff in pull request #40190: [SPARK-42597][SQL] UnwrapCastInBinaryComparison support unwrap timestamp type
Posted by "sunchao (via GitHub)" <gi...@apache.org>.
sunchao commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1122177569
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -293,6 +299,34 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
}
}
+ /**
+ * Move the cast to the literal side, because we can only get the minimum value of timestamp,
+ * so some BinaryComparison needs to be changed,
+ * such as CAST(ts AS date) > DATE '2023-01-01' ===> ts >= TIMESTAMP '2023-01-02 00:00:00'
Review Comment:
Suppose we have:
```
CAST('2023-01-02 00:00:00' AS DATE) > DATE '2023-01-01'
```
After the rule, it'll become:
```
'2023-01-02 00:00:00' >= '2023-01-02 00:00:00'
```
if changing to `>` this no longer evaluate to true.
I think this is similar to line 281 where we handle the case where a numeric value is rounded up after converting to the `fromType`.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on pull request #40190: [SPARK-42597][SQL] UnwrapCastInBinaryComparison support unwrap timestamp type
Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on PR #40190:
URL: https://github.com/apache/spark/pull/40190#issuecomment-1446348023
cc @sunchao @cloud-fan @huaxingao
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1130941590
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -113,7 +114,7 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
// literal and cast side, then process the result and swap the literal and cast again to
// restore the original order.
case BinaryComparison(Literal(_, literalType), Cast(fromExp, toType, _, _))
- if canImplicitlyCast(fromExp, toType, literalType) =>
+ if needToCanonicalizeExpression(fromExp, toType, literalType) =>
Review Comment:
I see, maybe it's better if `unwrapCast` returns `Option[Expression]`, then here we can just do `unwrapCast(swap(exp)).map(swap)`, and the caller side can do `unwrapCast(e).getOrElse(e)`
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1132439974
##########
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparisonSuite.scala:
##########
@@ -368,6 +370,61 @@ class UnwrapCastInBinaryComparisonSuite extends PlanTest with ExpressionEvalHelp
assertEquivalent(castInt(f4) < t, trueIfNotNull(f4))
}
+ test("SPARK-42597: Support unwrap date to timestamp type") {
+ val tsRelation = LocalRelation($"a".timestamp, $"b".timestampNTZ)
+ val dateLit = Literal.create(Date.valueOf("2023-01-01"), DateType)
Review Comment:
let's use the new `LocalDate` java class.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] wangyum commented on a diff in pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
Posted by "wangyum (via GitHub)" <gi...@apache.org>.
wangyum commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1162942720
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -293,6 +297,34 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
}
}
+ /**
+ * Move the cast to the literal side, because we can only get the minimum value of timestamp,
+ * so some BinaryComparison needs to be changed,
+ * such as CAST(ts AS date) > DATE '2023-01-01' ===> ts >= TIMESTAMP '2023-01-02 00:00:00'
+ */
+ private def unwrapDateToTimestamp(
+ exp: BinaryComparison,
+ fromExp: Expression,
+ date: Literal,
+ tz: Option[String],
+ evalMode: EvalMode.Value): Expression = {
+ val dateAddOne = DateAdd(date, Literal(1, IntegerType))
+ exp match {
+ case _: GreaterThan =>
+ GreaterThanOrEqual(fromExp, Cast(dateAddOne, fromExp.dataType, tz, evalMode))
+ case _: GreaterThanOrEqual =>
+ GreaterThanOrEqual(fromExp, Cast(date, fromExp.dataType, tz, evalMode))
+ case Equality(_, _) =>
Review Comment:
https://github.com/apache/spark/pull/40743
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a diff in pull request #40190: [SPARK-42597][SQL] Support unwrap date type to timestamp type
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #40190:
URL: https://github.com/apache/spark/pull/40190#discussion_r1161440201
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/optimizer/UnwrapCastInBinaryComparison.scala:
##########
@@ -293,6 +297,34 @@ object UnwrapCastInBinaryComparison extends Rule[LogicalPlan] {
}
}
+ /**
+ * Move the cast to the literal side, because we can only get the minimum value of timestamp,
+ * so some BinaryComparison needs to be changed,
+ * such as CAST(ts AS date) > DATE '2023-01-01' ===> ts >= TIMESTAMP '2023-01-02 00:00:00'
+ */
+ private def unwrapDateToTimestamp(
+ exp: BinaryComparison,
+ fromExp: Expression,
+ date: Literal,
+ tz: Option[String],
+ evalMode: EvalMode.Value): Expression = {
+ val dateAddOne = DateAdd(date, Literal(1, IntegerType))
+ exp match {
+ case _: GreaterThan =>
+ GreaterThanOrEqual(fromExp, Cast(dateAddOne, fromExp.dataType, tz, evalMode))
+ case _: GreaterThanOrEqual =>
+ GreaterThanOrEqual(fromExp, Cast(date, fromExp.dataType, tz, evalMode))
+ case Equality(_, _) =>
Review Comment:
I think this is wrong for `EqualNullSafe`. If the column value is null, then `EqualNullSafe` should return false, but the rewritten expression returns null.
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org