-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-29855][SQL] typed literals with negative sign with proper result or exception #26479
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
Changes from 1 commit
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -1860,16 +1860,19 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging | |||||
| override def visitTypeConstructor(ctx: TypeConstructorContext): Literal = withOrigin(ctx) { | ||||||
| val value = string(ctx.STRING) | ||||||
| val valueType = ctx.identifier.getText.toUpperCase(Locale.ROOT) | ||||||
| val negativeSign = Option(ctx.negativeSign).map(_.getText).getOrElse("") | ||||||
| val isNegative = if (negativeSign == "-") true else false | ||||||
|
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.
Suggested change
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. and remove
Contributor
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. do we really need to check
Member
Author
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.
|
||||||
|
|
||||||
| def toLiteral[T](f: UTF8String => Option[T], t: DataType): Literal = { | ||||||
| f(UTF8String.fromString(value)).map(Literal(_, t)).getOrElse { | ||||||
| throw new ParseException(s"Cannot parse the $valueType value: $value", ctx) | ||||||
| } | ||||||
| } | ||||||
| try { | ||||||
| valueType match { | ||||||
| case "DATE" => | ||||||
| case "DATE" if !isNegative => | ||||||
| toLiteral(stringToDate(_, getZoneId(SQLConf.get.sessionLocalTimeZone)), DateType) | ||||||
| case "TIMESTAMP" => | ||||||
| case "TIMESTAMP" if !isNegative => | ||||||
| val zoneId = getZoneId(SQLConf.get.sessionLocalTimeZone) | ||||||
| toLiteral(stringToTimestamp(_, zoneId), TimestampType) | ||||||
| case "INTERVAL" => | ||||||
|
|
@@ -1881,8 +1884,9 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging | |||||
| ex.setStackTrace(e.getStackTrace) | ||||||
| throw ex | ||||||
| } | ||||||
| Literal(applyNegativeSign(ctx.negativeSign, interval), CalendarIntervalType) | ||||||
| case "X" => | ||||||
| val signedInterval = if (isNegative) IntervalUtils.negate(interval) else interval | ||||||
| Literal(signedInterval, CalendarIntervalType) | ||||||
| case "X" if !isNegative => | ||||||
| val padding = if (value.length % 2 != 0) "0" else "" | ||||||
| Literal(DatatypeConverter.parseHexBinary(padding + value)) | ||||||
| case "INTEGER" => | ||||||
|
|
@@ -1894,9 +1898,10 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging | |||||
| ex.setStackTrace(e.getStackTrace) | ||||||
| throw ex | ||||||
| } | ||||||
| Literal(i, IntegerType) | ||||||
| Literal(if (isNegative) -i else i, IntegerType) | ||||||
| case other => | ||||||
| throw new ParseException(s"Literals of type '$other' are currently not supported.", ctx) | ||||||
| throw new ParseException(s"Literals of type '$negativeSign$other' are currently not" + | ||||||
| " supported.", ctx) | ||||||
| } | ||||||
| } catch { | ||||||
| case e: IllegalArgumentException => | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -153,3 +153,10 @@ select interval '1' year to second; | |
| select '1' year to second; | ||
| select interval 1 year '2-1' year to month; | ||
| select 1 year '2-1' year to month; | ||
| SET spark.sql.ansi.enabled=false; | ||
|
Contributor
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. not related to this PR, but it's really annoying to duplicate the test cases with ansi mode on and off. I'm wondering if we can introduce a
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. Ah, I see. That's certainly annoying. I'll try to make a pr later for that approach. |
||
|
|
||
| -- awareness for the negative sign before type | ||
| select -integer '7'; | ||
| select -date '1999-01-01'; | ||
| select -timestamp '1999-01-01'; | ||
| select -x'2379ACFe'; | ||
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.
is it simply
val isNegative = ctx.negativeSign != null?