-
Notifications
You must be signed in to change notification settings - Fork 29.3k
[SPARK-35085][SQL] Get columns operation should handle ANSI interval column properly #32345
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 2 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 |
|---|---|---|
|
|
@@ -131,7 +131,7 @@ private[hive] class SparkGetColumnsOperation( | |
| */ | ||
| private def getColumnSize(typ: DataType): Option[Int] = typ match { | ||
| case dt @ (BooleanType | _: NumericType | DateType | TimestampType | | ||
| CalendarIntervalType | NullType) => | ||
| CalendarIntervalType | NullType | YearMonthIntervalType | DayTimeIntervalType) => | ||
| Some(dt.defaultSize) | ||
| case CharType(n) => Some(n) | ||
| case StructType(fields) => | ||
|
|
@@ -171,8 +171,8 @@ private[hive] class SparkGetColumnsOperation( | |
| case BooleanType => java.sql.Types.BOOLEAN | ||
| case ByteType => java.sql.Types.TINYINT | ||
| case ShortType => java.sql.Types.SMALLINT | ||
| case IntegerType => java.sql.Types.INTEGER | ||
| case LongType => java.sql.Types.BIGINT | ||
| case IntegerType | YearMonthIntervalType => java.sql.Types.INTEGER | ||
| case LongType | DayTimeIntervalType => java.sql.Types.BIGINT | ||
|
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. hmm, I am not sure that we should expose ANSI intervals as raw integers/longs via JDBC. I would consider strings (preferable) or java.time.Duration/Period. @cloud-fan @srielau WDYT?
Contributor
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. Good question! I have the confusion too.
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. This is the metadata, where do we handle the data? e.g. if we want to return string or Duration/Period, where shall we instantiate string or Duration/Period values?
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. See the PR #32121. We should return strings. Since there is no appropriate type in @beliefer Could you handle
Contributor
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. OK |
||
| case FloatType => java.sql.Types.FLOAT | ||
| case DoubleType => java.sql.Types.DOUBLE | ||
| case _: DecimalType => java.sql.Types.DECIMAL | ||
|
|
||
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.
Size of what does it return?
CalendarIntervalType,YearMonthIntervalType,DayTimeIntervalTypeare returned as strings in rowSets.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.
CalendarIntervalTypereturn it'sdefaultSize(4 + 4 + 8 = 16).It seems
YearMonthIntervalTypeandDayTimeIntervalTypeshould returndefaultSizetoo.