Improved handling of writing NULLs in parquet #4191
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
malhotrashivam
added
parquet
malhotrashivam
requested review from
chipkent,
jmao-denver and
rcaudy
as code owners
lbooker42
reviewed
Jul 20, 2023
extensions/parquet/base/src/main/java/io/deephaven/parquet/base/AbstractBulkValuesWriter.java
Outdated
Show resolved
Hide resolved
jmao-denver
previously approved these changes
Jul 20, 2023
rcaudy
reviewed
Jul 20, 2023
extensions/parquet/base/src/main/java/io/deephaven/parquet/base/PlainIntChunkedWriter.java
Outdated
Show resolved
Hide resolved
extensions/parquet/base/src/main/java/io/deephaven/parquet/base/PlainIntChunkedWriter.java
Outdated
Show resolved
Hide resolved
extensions/parquet/table/src/main/java/io/deephaven/parquet/table/ParquetTableWriter.java
Outdated
Show resolved
Hide resolved
extensions/parquet/base/src/main/java/io/deephaven/parquet/base/PlainIntChunkedWriter.java
Outdated
Show resolved
Hide resolved
extensions/parquet/base/src/main/java/io/deephaven/parquet/base/ColumnWriterImpl.java
Outdated
Show resolved
Hide resolved
extensions/parquet/base/src/main/java/io/deephaven/parquet/base/BulkWriter.java
Outdated
Show resolved
Hide resolved
extensions/parquet/base/src/main/java/io/deephaven/parquet/base/AbstractBulkValuesWriter.java
Outdated
Show resolved
Hide resolved
extensions/parquet/base/src/main/java/io/deephaven/parquet/base/ColumnWriter.java
Outdated
Show resolved
Hide resolved
malhotrashivam
commented
Jul 22, 2023
rcaudy
reviewed
Jul 22, 2023
extensions/parquet/table/src/main/java/io/deephaven/parquet/table/ParquetTableWriter.java
Outdated
Show resolved
Hide resolved
extensions/parquet/table/src/main/java/io/deephaven/parquet/table/ParquetTableWriter.java
Outdated
Show resolved
Hide resolved
extensions/parquet/table/src/main/java/io/deephaven/parquet/table/ParquetTableWriter.java
Outdated
Show resolved
Hide resolved
extensions/parquet/table/src/main/java/io/deephaven/parquet/table/ParquetTableWriter.java
Outdated
Show resolved
Hide resolved
malhotrashivam
commented
Jul 24, 2023
extensions/parquet/table/src/main/java/io/deephaven/parquet/table/ParquetTableWriter.java
Outdated
Show resolved
Hide resolved
malhotrashivam
commented
Jul 24, 2023
extensions/parquet/table/src/main/java/io/deephaven/parquet/table/ParquetTableWriter.java
Outdated
Show resolved
Hide resolved
rcaudy
reviewed
Jul 25, 2023
malhotrashivam
commented
Jul 26, 2023
extensions/parquet/table/src/main/java/io/deephaven/parquet/table/ParquetTableWriter.java
Show resolved
Hide resolved
jmao-denver
previously approved these changes
Jul 26, 2023
rcaudy
reviewed
Jul 26, 2023
extensions/parquet/table/src/main/java/io/deephaven/parquet/table/ParquetTableWriter.java
Outdated
Show resolved
Hide resolved
extensions/parquet/table/src/main/java/io/deephaven/parquet/table/ParquetTableWriter.java
Outdated
Show resolved
Hide resolved
extensions/parquet/table/src/main/java/io/deephaven/parquet/table/ParquetTableWriter.java
Outdated
Show resolved
Hide resolved
…ble/ParquetTableWriter.java
…ble/ParquetTableWriter.java
…ble/ParquetTableWriter.java
rcaudy
approved these changes
Jul 26, 2023
malhotrashivam
changed the title
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Bug fix, closes #1548
Original State:
Before this change, nulls were handled incorrectly for primitive types as well as for arrays and vectors, such that null values were written as Deephaven null representations in the parquet file. For example, a null in an integer column would be written as QueryConstants.NULL_INT in the parquet file. Therefore, any reader other than deephaven's parquet reader would not be able to process it correctly.
Change description:
This PR fixes multiple small bugs in both java and python code for processing all nulls in a standard manner such that instead of writing different Deephaven null representations to the parquet file, we encode them by storing offsets of null values. #4186 was also discovered and fixed along with this PR for proper representation of floats in the parquet file.
Along with that, we have done refactoring in the ParquetFileWriter class to make the code more readable and add more documentation.