You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@orc.apache.org by GitBox <gi...@apache.org> on 2020/09/20 04:24:03 UTC
[GitHub] [orc] dongjoon-hyun opened a new pull request #545: ORC-667: Positional mapping for nested struct types should not applied by default
dongjoon-hyun opened a new pull request #545:
URL: https://github.com/apache/orc/pull/545
# What changes were proposed in this pull request?
ORC-644 (Support positional mapping for nested types) introduced a correctness regression on nested struct types.
### Why are the changes needed?
By default, `orc.force.positional.evolution` is false. However, currently it's applied for nested struct fields.
### How was this patch tested?
Pass the newly added test case.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] omalley closed pull request #545: ORC-667: Positional mapping for nested struct types should not applied by default
Posted by GitBox <gi...@apache.org>.
omalley closed pull request #545:
URL: https://github.com/apache/orc/pull/545
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #545: ORC-667: Positional mapping for nested struct types should not applied by default
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #545:
URL: https://github.com/apache/orc/pull/545#issuecomment-695694629
cc @ArvinZheng
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #545: ORC-667: Positional mapping for nested struct types should not applied by default
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #545:
URL: https://github.com/apache/orc/pull/545#issuecomment-695748380
Thank you for review, @ArvinZheng .
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on pull request #545: ORC-667: Positional mapping for nested struct types should not applied by default
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #545:
URL: https://github.com/apache/orc/pull/545#issuecomment-697161420
Thank you, @omalley !
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a change in pull request #545: ORC-667: Positional mapping for nested struct types should not applied by default
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #545:
URL: https://github.com/apache/orc/pull/545#discussion_r491652983
##########
File path: java/core/src/java/org/apache/orc/impl/SchemaEvolution.java
##########
@@ -513,7 +513,7 @@ void buildConversion(TypeDescription fileType,
isOnlyImplicitConversion = false;
}
- if (positionalLevels == 0) {
+ if (positionalLevels <= 0) {
Review comment:
This can be negative because of ~ORC-667~, ORC-644.
##########
File path: java/core/src/java/org/apache/orc/impl/SchemaEvolution.java
##########
@@ -513,7 +513,7 @@ void buildConversion(TypeDescription fileType,
isOnlyImplicitConversion = false;
}
- if (positionalLevels == 0) {
+ if (positionalLevels <= 0) {
Review comment:
Oh. Thanks! It's fixed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] dongjoon-hyun commented on a change in pull request #545: ORC-667: Positional mapping for nested struct types should not applied by default
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #545:
URL: https://github.com/apache/orc/pull/545#discussion_r491652983
##########
File path: java/core/src/java/org/apache/orc/impl/SchemaEvolution.java
##########
@@ -513,7 +513,7 @@ void buildConversion(TypeDescription fileType,
isOnlyImplicitConversion = false;
}
- if (positionalLevels == 0) {
+ if (positionalLevels <= 0) {
Review comment:
This can be negative because of ORC-667.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] ArvinZheng commented on a change in pull request #545: ORC-667: Positional mapping for nested struct types should not applied by default
Posted by GitBox <gi...@apache.org>.
ArvinZheng commented on a change in pull request #545:
URL: https://github.com/apache/orc/pull/545#discussion_r491653773
##########
File path: java/core/src/java/org/apache/orc/impl/SchemaEvolution.java
##########
@@ -513,7 +513,7 @@ void buildConversion(TypeDescription fileType,
isOnlyImplicitConversion = false;
}
- if (positionalLevels == 0) {
+ if (positionalLevels <= 0) {
Review comment:
because of ORC-644
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] omalley closed pull request #545: ORC-667: Positional mapping for nested struct types should not applied by default
Posted by GitBox <gi...@apache.org>.
omalley closed pull request #545:
URL: https://github.com/apache/orc/pull/545
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [orc] ArvinZheng commented on pull request #545: ORC-667: Positional mapping for nested struct types should not applied by default
Posted by GitBox <gi...@apache.org>.
ArvinZheng commented on pull request #545:
URL: https://github.com/apache/orc/pull/545#issuecomment-695746547
LGTM
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org