-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions #27965
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.
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
[SPARK-31205][SQL] support string literal as the second argument of date_add/date_sub functions #27965
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,7 @@ import scala.annotation.tailrec | |
| import scala.collection.mutable | ||
|
|
||
| import org.apache.spark.internal.Logging | ||
| import org.apache.spark.sql.AnalysisException | ||
| import org.apache.spark.sql.catalyst.expressions._ | ||
| import org.apache.spark.sql.catalyst.expressions.aggregate._ | ||
| import org.apache.spark.sql.catalyst.plans.logical._ | ||
|
|
@@ -63,6 +64,7 @@ object TypeCoercion { | |
| ImplicitTypeCasts :: | ||
| DateTimeOperations :: | ||
| WindowFrameCoercion :: | ||
| StringLiteralCoercion :: | ||
| Nil | ||
|
|
||
| // See https://cwiki.apache.org/confluence/display/Hive/LanguageManual+Types. | ||
|
|
@@ -1043,6 +1045,34 @@ object TypeCoercion { | |
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * A special rule to support string literal as the second argument of date_add/date_sub functions, | ||
| * to keep backward compatibility as a temporary workaround. | ||
| * TODO(SPARK-28589): implement ANSI type type coercion and handle string literals. | ||
| */ | ||
| object StringLiteralCoercion extends TypeCoercionRule { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This causes a behavior difference in arithmatic operations, too. Could you describe the following change in the PR description? New one looks reasonable to me. 2.4.5 and 3.0.0-preview2 This PR.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You forgot to write a function name in the second query above?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No~ The both queries are the same. What I meant was it's the behavior of this PR; this PR extends expressions, too.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I see. |
||
| override protected def coerceTypes(plan: LogicalPlan): LogicalPlan = plan resolveExpressions { | ||
| // Skip nodes who's children have not been resolved yet. | ||
| case e if !e.childrenResolved => e | ||
| case DateAdd(l, r) if r.dataType == StringType && r.foldable => | ||
| val days = try { | ||
| AnsiCast(r, IntegerType).eval().asInstanceOf[Int] | ||
| } catch { | ||
| case e: NumberFormatException => throw new AnalysisException( | ||
| "The second argument of 'date_add' function needs to be an integer.", cause = Some(e)) | ||
| } | ||
| DateAdd(l, Literal(days)) | ||
| case DateSub(l, r) if r.dataType == StringType && r.foldable => | ||
| val days = try { | ||
| AnsiCast(r, IntegerType).eval().asInstanceOf[Int] | ||
| } catch { | ||
| case e: NumberFormatException => throw new AnalysisException( | ||
| "The second argument of 'date_sub' function needs to be an integer.", cause = Some(e)) | ||
| } | ||
| DateSub(l, Literal(days)) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| trait TypeCoercionRule extends Rule[LogicalPlan] with Logging { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new approach looks good.