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.

Sign up for GitHub

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

Jump to bottom

timestamps #72

Merged
merged 3 commits into from
Dec 1, 2016
Merged

timestamps #72

merged 3 commits into from
Dec 1, 2016

Conversation

pwoody
Copy link

@pwoody pwoody commented Dec 1, 2016

No description provided.

@pwoody
Copy link
Author

pwoody commented Dec 1, 2016

@robert3005 fixed the weird rebase. this should be good to go

@pwoody
Copy link
Author

pwoody commented Dec 1, 2016

This works on the legacy int96 format as well because spark.sql.parquet.int96AsTimestamp defaults to true and disables filter pushdown. If you set that setting to false you will see this error when attempting to read the old format:

Caused by: java.lang.IllegalArgumentException: FilterPredicate column: value's declared type (java.lang.Long) does not match the schema found in file metadata. Column value is of type: INT96 Valid types for this column are: [class org.apache.parquet.io.api.Binary]

@pwoody
Copy link
Author

pwoody commented Dec 1, 2016

@ash211 @lynchmattj @schlosna

@pwoody pwoody mentioned this pull request Dec 1, 2016
Closed
robert3005
robert3005 reviewed Dec 1, 2016
View reviewed changes
...re/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetFileFormat.scala
.reduceOption(FilterApi.and)
} else {
None
}

log.debug(s"Pushing converted filters: $pushed")
Copy link

Choose a reason for hiding this comment

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

maybe info? FilterCompat.get logs at that level

@robert3005
Copy link

👍 only nit about logging since I was chasing about that value multiple times already. Otherwise looks good to me.

@pwoody
Copy link
Author

pwoody commented Dec 1, 2016

Cool done, merging.

@pwoody pwoody merged commit 784072a into master Dec 1, 2016
@pwoody pwoody deleted the pw/timestamps branch December 1, 2016 21:27
schlosna
schlosna reviewed Dec 1, 2016
View reviewed changes
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/parquet/ParquetFilters.scala
val isOptionalField = f.metadata.contains(StructType.metadataKeyForOptionalField) &&
f.metadata.getBoolean(StructType.metadataKeyForOptionalField)

(isTs && int96AsTimestamp) || isOptionalField
Copy link

Choose a reason for hiding this comment

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

nit: could be lazy with the boolean evaluations:

fields.filterNot { f =>
    (int96AsTimestamp && DataTypes.TimestampType.acceptsType(f.dataType))
    || (f.metadata.contains(StructType.metadataKeyForOptionalField)
                    && f.metadata.getBoolean(StructType.metadataKeyForOptionalField))
}.map(f => f.name -> f.dataType).toMap

laflechejonathan pushed a commit that referenced this pull request Sep 27, 2021
@jdcasale
Use dataSchema.nonEmpty instead of partitionColumnNames.nonEmpty (#72)
1b4f123
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@schlosna schlosna schlosna left review comments

@robert3005 robert3005 robert3005 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pwoody @robert3005 @schlosna