-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-54683][SQL] Unify geo and time types blocking #53438
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
base: master
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -345,6 +345,8 @@ object DeserializerBuildHelper { | |
| createDeserializerForInstant(path) | ||
| case LocalDateTimeEncoder => | ||
| createDeserializerForLocalDateTime(path) | ||
| case LocalTimeEncoder if !SQLConf.get.isTimeTypeEnabled => | ||
| throw org.apache.spark.sql.errors.QueryCompilationErrors.unsupportedTimeTypeError() | ||
|
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. Shall we import
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. geo type blocking does not import It's temporary and will be removed after we complete time type development. I think it's fine. |
||
| case LocalTimeEncoder => | ||
| createDeserializerForLocalTime(path) | ||
| case UDTEncoder(udt, udtClass) => | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -367,6 +367,8 @@ object SerializerBuildHelper { | |
| case TimestampEncoder(false) => createSerializerForSqlTimestamp(input) | ||
| case InstantEncoder(false) => createSerializerForJavaInstant(input) | ||
| case LocalDateTimeEncoder => createSerializerForLocalDateTime(input) | ||
| case LocalTimeEncoder if !SQLConf.get.isTimeTypeEnabled => | ||
|
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. ditto. |
||
| throw org.apache.spark.sql.errors.QueryCompilationErrors.unsupportedTimeTypeError() | ||
|
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. ditto. import |
||
| case LocalTimeEncoder => createSerializerForLocalTime(input) | ||
| case UDTEncoder(udt, udtClass) => createSerializerForUserDefinedType(input, udt, udtClass) | ||
| case OptionEncoder(valueEnc) => | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -712,6 +712,7 @@ trait CheckAnalysis extends LookupCatalog with QueryErrorsBase with PlanToString | |
| } | ||
|
|
||
| create.tableSchema.foreach(f => TypeUtils.failWithIntervalType(f.dataType)) | ||
| TypeUtils.failUnsupportedDataType(create.tableSchema, SQLConf.get) | ||
|
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. This is a bit off topic, but just follow how we block interval type for v2 catalogs, which we don't have any builtin impl in Spark yet.
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. Could you use a new JIRA ID instead of a follow-up, @cloud-fan ? |
||
| SchemaUtils.checkIndeterminateCollationInSchema(create.tableSchema) | ||
|
|
||
| case write: V2WriteCommand if write.resolved => | ||
|
|
||
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 this missed before? Or a side-effect of unification (which becomes to miss in other layer)?
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.
It's missed before and I found it when trying to unify the geo type blocking code.