Skip to content
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

[Kernel] Add support for reading tables with timestamp_ntz type columns #2855

Merged
merged 2 commits into from
Apr 6, 2024

Conversation

vkorukanti
Copy link
Collaborator

@vkorukanti vkorukanti commented Apr 5, 2024

  1. Add read support (schema, parquet reader and type)
  2. Add partition pruning on ts_ntz type columns
  3. Add expression evaluator support in default handler for ts_ntz type columns.

Close #2279

@vkorukanti vkorukanti changed the title [WIP][Kernel] Add support for reading tables with timestamp_ntz type columns [Kernel] Add support for reading tables with timestamp_ntz type columns Apr 5, 2024
@@ -86,6 +86,9 @@ public long getLong(int rowId) {
} else if (partitionType.equivalent(TimestampType.TIMESTAMP)) {
return InternalUtils.microsSinceEpoch(
Timestamp.valueOf(input.getString(rowId)));
} else if (partitionType.equivalent(TimestampNTZType.TIMESTAMP_NTZ)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor nit: might be safer to do

else if (partitionType.equivalent(TimestampType.TIMESTAMP) ||
        partitionType.equivalent(TimestampNTZType.TIMESTAMP_NTZ))) {
    return InternalUtils.microsSinceEpoch(
        Timestamp.valueOf(input.getString(rowId)));  
}

then one implementation won't accidentally diverge from the other in the future

@@ -77,7 +79,9 @@ public static Converter createConverter(
return DecimalColumnReader.createDecimalConverter(
initialBatchSize, (DecimalType) typeFromClient, typeFromFile);
} else if (typeFromClient instanceof TimestampType) {
return TimestampConverters.createTimestampConverter(initialBatchSize, typeFromFile);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto, but up to you

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated the other one. This one has different method calls depending upon the type.

@vkorukanti vkorukanti merged commit 64c3207 into delta-io:master Apr 6, 2024
7 checks passed
@vkorukanti vkorukanti deleted the tsNtz branch May 9, 2024 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request][Kernel] Add support for reading tables with timestamp_ntz type columns
2 participants