Rebase on upstream (2022-11-15) #1
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Serialize time.Time values as Parquet timestamps. The default unit is NANOS and can be changed using the timestamp() struct tag.
type timeColumn struct {
t1 time.Time
t2 time.Time `parquet:",timestamp(millisecond)"`
}
Reading a parquet file with a decimal column isn't loaded with logical type information. This behavior was not implemented. `decimalType` is more complex from the other types because a parquet decimal can be backed by multiple different physical types. This PR loads logical type information for `DECIMAL` fields. Closes segmentio#365
parquet-cli prints the string format of decimals as `DECIMAL(precision,scale)`. Update parquet-go's string format to match.
nathanwiegand
approved these changes
Nov 15, 2022
tatatodd
approved these changes
Nov 15, 2022
| tagName = "segmentio" | ||
| keyTagName = "segmentio-key" | ||
| valueTagName = "segmentio-value" | ||
| ) |
There was a problem hiding this comment.
Minor: something looks fishy here; the neeva branch should already have these changes.
Did you pull the latest neeva branch, to create your PR?
| @@ -381,8 +381,27 @@ func schemaElementTypeOf(s *format.SchemaElement) Type { | |||
| case lt.Enum != nil: | |||
| return (*enumType)(lt.Enum) | |||
| case lt.Decimal != nil: | |||
| // TODO: | |||
| // return (*decimalType)(lt.Decimal) | |||
| // A parquet decimal can be one of several different physical types. | |||
There was a problem hiding this comment.
Can you add a better title and description for the PR?
Ideally we'd upstream any changes we make, and it'll be helpful to have a bit more context.
E.g. I see a bunch of changes in this PR to handle decimal and timestamps, but the current PR description is about handling empty files.
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The motivation for this is to pull in a fix that I added to upstream to make it possible to read empty files.