PARQUET-385 PARQUET-379: Fixes strict schema merging #315
Conversation
This file contains hidden or 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
Contributor
Author
|
Not quite sure whether we should also deprecate the |
Contributor
There was a problem hiding this comment.
Since originalType is an Enum, seems we can just do like:
if (getOriginalType() != toMerge.getOriginalType())
reportSchemaMergeError(toMerge);
which covers:
- if both are null, the condition evals to false
- if one is null and the other is not null, the condition evals to true
- if both are not null, then the condition evals to false if they take different values, and evals to true otherwise.
@liancheng what do you think?
Contributor
Author
There was a problem hiding this comment.
Thanks! Being in the Scala world for so long a time, I really need to re-learn Java again :)
Contributor
|
Having examined all existing 20 OriginalTypes, and 2 OriginalTypes to be added, I think this PR works well. So: |
Member
|
+1 |
Member
|
@liancheng can you remove the square brackets in your pr title? That trips up the merge script. |
liancheng
changed the title
Contributor
Author
|
@julienledem Done. |
piyushnarang
pushed a commit
to piyushnarang/parquet-mr
that referenced
this pull request
Jun 15, 2016
This PR fixes strict mode schema merging. To merge two `PrimitiveType` `t1` and `t2`, they must satisfy the following conditions: 1. `t1` and `t2` have the same primitive type name 1. `t1` and `t2` either - don't have original type, or - have the same original type 1. If `t1` and `t2` are both `FIXED_LEN_BYTE_ARRAY`, they should have the same length Also, merged schema now preserves original name if there's any. Author: Cheng Lian <lian@databricks.com> Closes apache#315 from liancheng/fix-strict-schema-merge and squashes the following commits: a29138c [Cheng Lian] Addresses PR comment 1ac804e [Cheng Lian] Fixes strict schema merging
rdblue
pushed a commit
to rdblue/parquet-mr
that referenced
this pull request
Jan 6, 2017
This PR fixes strict mode schema merging. To merge two `PrimitiveType` `t1` and `t2`, they must satisfy the following conditions: 1. `t1` and `t2` have the same primitive type name 1. `t1` and `t2` either - don't have original type, or - have the same original type 1. If `t1` and `t2` are both `FIXED_LEN_BYTE_ARRAY`, they should have the same length Also, merged schema now preserves original name if there's any. Author: Cheng Lian <lian@databricks.com> Closes apache#315 from liancheng/fix-strict-schema-merge and squashes the following commits: a29138c [Cheng Lian] Addresses PR comment 1ac804e [Cheng Lian] Fixes strict schema merging
This was referenced Jun 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This PR fixes strict mode schema merging. To merge two
PrimitiveTypet1andt2, they must satisfy the following conditions:t1andt2have the same primitive type namet1andt2eithert1andt2are bothFIXED_LEN_BYTE_ARRAY, they should have the same lengthAlso, merged schema now preserves original name if there's any.