ORC: Add name mapping support #1208
Conversation
|
@rdblue @rdsr @shardulm94 PTAL when you have a chance. Thanks! |
edgarRd
force-pushed
the
orc-name-mapping
branch
from
a7c5068 to
106795d
Compare
edgarRd
force-pushed
the
orc-name-mapping
branch
from
106795d to
2b365e8
Compare
|
@rdsr @shardulm94 @rdblue I've updated this branch to latest master, PTAL when you have a chance. Thanks! |
| assertTrue("TypeDescription schemas should be equal, not comparing Attributes", | ||
| typeDescriptionWithIds.equals(typeDescriptionWithIdsFromNameMapping, false /* checkAttributes */)); |
There was a problem hiding this comment.
If we are not comparing the attributes here, is there any value to adding this assert? Seems like name mapping's effect (assigning ids) is not actually being tested in this case.
There was a problem hiding this comment.
I've added a check for the IDs as well.
|
I'll try to get to this over the weekend. Sorry for not getting back to you sooner, I'm a bit behind with reviews. |
| this.schema = schema; | ||
| this.readerFunction = readerFunction; | ||
| this.file = file; | ||
| this.nameMapping = Optional.ofNullable(nameMapping).orElse(MappingUtil.create(schema)); |
There was a problem hiding this comment.
Should we also check if the file schema does not have ids before initializing nameMapping?
orc/src/test/java/org/apache/iceberg/orc/TestExpressionToSearchArgument.java
Outdated
Show resolved
Hide resolved
- Projection on complex types: maps, lists, structs
|
@rdsr @shardulm94 I think this is ready for another review. PTAL when you have a chance. |
shardulm94
left a comment
shardulm94
left a comment
There was a problem hiding this comment.
Looks good to me! @rdsr Any further comments on this one?
orc/src/test/java/org/apache/iceberg/orc/TestExpressionToSearchArgument.java
Outdated
Show resolved
Hide resolved
|
@rdblue do you have any additional comment? Thanks! |
|
Thanks @edgarRd . I just merged this! |
|
Thanks @rdsr and @shardulm94 for the reviews and merging! |
4 participants
This PR fixes #1204 implementing name mapping for ORC.