-
Notifications
You must be signed in to change notification settings - Fork 28.9k
[SPARK-46466][SQL] Vectorized parquet reader should never do rebase for timestamp ntz #44428
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 all commits
c2547d6
e3743d7
57c0c0b
286d72a
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 |
|---|---|---|
|
|
@@ -109,24 +109,32 @@ public ParquetVectorUpdater getUpdater(ColumnDescriptor descriptor, DataType spa | |
| // For unsigned int64, it stores as plain signed int64 in Parquet when dictionary | ||
| // fallbacks. We read them as decimal values. | ||
| return new UnsignedLongUpdater(); | ||
| } else if (isTimestamp(sparkType) && | ||
| } else if (sparkType == DataTypes.TimestampType && | ||
| isTimestampTypeMatched(LogicalTypeAnnotation.TimeUnit.MICROS)) { | ||
| validateTimestampType(sparkType); | ||
| if ("CORRECTED".equals(datetimeRebaseMode)) { | ||
| return new LongUpdater(); | ||
| } else { | ||
| boolean failIfRebase = "EXCEPTION".equals(datetimeRebaseMode); | ||
| return new LongWithRebaseUpdater(failIfRebase, datetimeRebaseTz); | ||
| } | ||
| } else if (isTimestamp(sparkType) && | ||
| } else if (sparkType == DataTypes.TimestampType && | ||
| isTimestampTypeMatched(LogicalTypeAnnotation.TimeUnit.MILLIS)) { | ||
| validateTimestampType(sparkType); | ||
| if ("CORRECTED".equals(datetimeRebaseMode)) { | ||
| return new LongAsMicrosUpdater(); | ||
| } else { | ||
| final boolean failIfRebase = "EXCEPTION".equals(datetimeRebaseMode); | ||
| return new LongAsMicrosRebaseUpdater(failIfRebase, datetimeRebaseTz); | ||
| } | ||
| } else if (sparkType == DataTypes.TimestampNTZType && | ||
| isTimestampTypeMatched(LogicalTypeAnnotation.TimeUnit.MICROS)) { | ||
| validateTimestampNTZType(); | ||
| // TIMESTAMP_NTZ is a new data type and has no legacy files that need to do rebase. | ||
|
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. Just a question.
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. legacy here means parquet files written before the calendar switch.
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. Got it now. You means we never rebase the time zone for |
||
| return new LongUpdater(); | ||
| } else if (sparkType == DataTypes.TimestampNTZType && | ||
| isTimestampTypeMatched(LogicalTypeAnnotation.TimeUnit.MILLIS)) { | ||
| validateTimestampNTZType(); | ||
| // TIMESTAMP_NTZ is a new data type and has no legacy files that need to do rebase. | ||
| return new LongAsMicrosUpdater(); | ||
| } else if (sparkType instanceof DayTimeIntervalType) { | ||
| return new LongUpdater(); | ||
| } | ||
|
|
@@ -195,12 +203,11 @@ boolean isTimestampTypeMatched(LogicalTypeAnnotation.TimeUnit unit) { | |
| annotation.getUnit() == unit; | ||
| } | ||
|
|
||
| void validateTimestampType(DataType sparkType) { | ||
| private void validateTimestampNTZType() { | ||
| assert(logicalTypeAnnotation instanceof TimestampLogicalTypeAnnotation); | ||
| // Throw an exception if the Parquet type is TimestampLTZ and the Catalyst type is TimestampNTZ. | ||
| // Throw an exception if the Parquet type is TimestampLTZ as the Catalyst type is TimestampNTZ. | ||
| // This is to avoid mistakes in reading the timestamp values. | ||
| if (((TimestampLogicalTypeAnnotation) logicalTypeAnnotation).isAdjustedToUTC() && | ||
| sparkType == DataTypes.TimestampNTZType) { | ||
| if (((TimestampLogicalTypeAnnotation) logicalTypeAnnotation).isAdjustedToUTC()) { | ||
| convertErrorForTimestampNTZ("int64 time(" + logicalTypeAnnotation + ")"); | ||
| } | ||
| } | ||
|
|
@@ -1149,10 +1156,6 @@ private static boolean isLongDecimal(DataType dt) { | |
| return false; | ||
| } | ||
|
|
||
| private static boolean isTimestamp(DataType dt) { | ||
| return dt == DataTypes.TimestampType || dt == DataTypes.TimestampNTZType; | ||
| } | ||
|
|
||
| private static boolean isDecimalTypeMatched(ColumnDescriptor descriptor, DataType dt) { | ||
| DecimalType d = (DecimalType) dt; | ||
| LogicalTypeAnnotation typeAnnotation = descriptor.getPrimitiveType().getLogicalTypeAnnotation(); | ||
|
|
||
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.
Shall we remove
isTimestampis not used any more?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.
done