Skip to content

[SPARK-29774][SQL] Date and Timestamp type +/- null should be null as Postgres #26412

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
yaooqinn wants to merge 32 commits into from
Closed

[SPARK-29774][SQL] Date and Timestamp type +/- null should be null as Postgres #26412

Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
0b293db
[SPARK-29774][SQL] Date and Timestamp type +/- null should be null as…
yaooqinn Nov 6, 2019
f726297
Merge branch 'master' into SPARK-29774
yaooqinn Nov 27, 2019
e7225a3
regen golden file
yaooqinn Nov 27, 2019
b925517
null - dates
yaooqinn Nov 27, 2019
cd49411
Merge branch 'master' into SPARK-29774
yaooqinn Dec 2, 2019
57b13e9
support +/-
yaooqinn Dec 2, 2019
eab6a83
support ×/÷
yaooqinn Dec 2, 2019
e8b75ba
import
yaooqinn Dec 2, 2019
02b3738
childResolved required
yaooqinn Dec 2, 2019
e89d806
regen golden file
yaooqinn Dec 2, 2019
0694e07
update comments
yaooqinn Dec 2, 2019
0f5618b
fix tests
yaooqinn Dec 3, 2019
efab3ec
fix tests
yaooqinn Dec 3, 2019
1c27be1
refine case match pattern
yaooqinn Dec 3, 2019
5df6980
fix ut
yaooqinn Dec 3, 2019
9817d2d
hack assert Equal
yaooqinn Dec 3, 2019
9808b9c
regen g f
yaooqinn Dec 3, 2019
b190612
AnalysisTest
yaooqinn Dec 3, 2019
e544137
regen g f
yaooqinn Dec 3, 2019
83705fd
fix test
yaooqinn Dec 4, 2019
846802d
date add/sub only work for int/smallint/tinyint
yaooqinn Dec 4, 2019
4af7edb
regen g f
yaooqinn Dec 4, 2019
a67be30
refine
yaooqinn Dec 4, 2019
9a1affd
type coercion for subtract timestamp
yaooqinn Dec 4, 2019
ae70022
add and reorgnize tests in datetime.sql
yaooqinn Dec 4, 2019
571225b
DateExpressionsSuite
yaooqinn Dec 4, 2019
6052e5a
fix py
yaooqinn Dec 4, 2019
928fd86
fix py
yaooqinn Dec 4, 2019
254d2d2
Revert "fix py"
yaooqinn Dec 5, 2019
5dd632c
fix py
yaooqinn Dec 5, 2019
c84d46e
rm unresolved binary arithmetic
yaooqinn Dec 5, 2019
a44948e
import
yaooqinn Dec 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
childResolved required
  • Loading branch information
@yaooqinn
yaooqinn committed Dec 2, 2019
commit 02b3738aa033ce1ff67967027e084f41686542a9
8 changes: 4 additions & 4 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ class Analyzer(
case class ResolveBinaryArithmetic(conf: SQLConf) extends Rule[LogicalPlan] {
override def apply(plan: LogicalPlan): LogicalPlan = plan.resolveOperatorsUp {
case p: LogicalPlan => p.transformExpressionsUp {
case UnresolvedAdd(l, r) => (l.dataType, r.dataType) match {
case u @ UnresolvedAdd(l, r) if u.childrenResolved => (l.dataType, r.dataType) match {
case (TimestampType | DateType | StringType, CalendarIntervalType) =>
Copy link
Contributor

@cloud-fan cloud-fan Dec 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

according to the above discussion, I think it's clearer to do

case (CalendarIntervalType, CalendarIntervalType) => Add
case (_, CalendarIntervalType) => TimeAdd
case (CalendarIntervalType, _) => TimeAdd
case (_, DateType) => DateAdd
case (DateType, _) => DateAdd
case _ => Add

Cast(TimeAdd(l, r), l.dataType)
case (CalendarIntervalType, TimestampType | DateType | StringType) =>
Expand All @@ -268,7 +268,7 @@ class Analyzer(
case (_, DateType) => DateAdd(r, l)
case (_, _) => Add(l, r)
}
case UnresolvedSubtract(l, r) => (l.dataType, r.dataType) match {
case u @ UnresolvedSubtract(l, r) if u.childrenResolved => (l.dataType, r.dataType) match {
case (TimestampType | DateType | StringType, CalendarIntervalType) =>
Cast(TimeSub(l, r), l.dataType)
Copy link
Member Author

@yaooqinn yaooqinn Dec 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I notice that TimeSub is replaceable by TimeAdd(l, UnaryMinus(r)), which make it useless

case (CalendarIntervalType, TimestampType | DateType | StringType) =>
Expand All @@ -283,12 +283,12 @@ class Analyzer(
case (DateType, _) => DateSub(l, r)
case (_, _) => Subtract(l, r)
}
case UnresolvedMultiply(l, r) => (l.dataType, r.dataType) match {
case u @ UnresolvedMultiply(l, r) if u.childrenResolved => (l.dataType, r.dataType) match {
case (CalendarIntervalType, _: NumericType | NullType) => MultiplyInterval(l, r)
Copy link
Contributor

@cloud-fan cloud-fan Dec 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

case (_: NumericType | NullType, CalendarIntervalType) => MultiplyInterval(r, l)
Copy link
Member Author

@yaooqinn yaooqinn Dec 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is another thing to discuss, do we have to only support interval * numeric andinterval / numeric, this is not same with other type coercion rules, e.g. 1 / '2' is valid, so interval 1 day / '2' should be valid too.

In PostgreSQL, also valid

postgres=# select interval '1' day * 2;
 ?column?
----------
 2 days
(1 row)

postgres=# select interval '1' day * '2';
 ?column?
----------
 2 days
(1 row)

postgres=# select interval '1' day / '2';
 ?column?
----------
 12:00:00
(1 row)

Copy link
Member Author

@yaooqinn yaooqinn Dec 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

case (_, CalendarIntervalType) => MultiplyInterval(r, l) might be enough for this case.

case (_, _) => Multiply(l, r)
}
case UnresolvedDivide(l, r) => (l.dataType, r.dataType) match {
case u @ UnresolvedDivide(l, r) if u.childrenResolved => (l.dataType, r.dataType) match {
case (CalendarIntervalType, _: NumericType | NullType) => DivideInterval(l, r)
case (_, _) => Divide(l, r)
}
Expand Down