You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2021/11/25 03:28:38 UTC
[GitHub] [iceberg] zhongyujiang opened a new pull request #3605: make ApplyNameMapping keep field's name after name mapping.
zhongyujiang opened a new pull request #3605:
URL: https://github.com/apache/iceberg/pull/3605
This PR makes ApplyNameMapping keep field's name after name mapping, which is for compatibility while reading the migrated Iceberg table with legacy parquet schema mentioned in this issue #3604.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] uncleGen commented on pull request #3605: make ApplyNameMapping keep field's name after name mapping.
Posted by GitBox <gi...@apache.org>.
uncleGen commented on pull request #3605:
URL: https://github.com/apache/iceberg/pull/3605#issuecomment-997677874
Is there same issue in ORC ff?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] uncleGen edited a comment on pull request #3605: make ApplyNameMapping keep field's name after name mapping.
Posted by GitBox <gi...@apache.org>.
uncleGen edited a comment on pull request #3605:
URL: https://github.com/apache/iceberg/pull/3605#issuecomment-997677874
In general, this pr makes sense to me, and is there same issue in ORC ff?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] uncleGen edited a comment on pull request #3605: make ApplyNameMapping keep field's name after name mapping.
Posted by GitBox <gi...@apache.org>.
uncleGen edited a comment on pull request #3605:
URL: https://github.com/apache/iceberg/pull/3605#issuecomment-997677874
In general, this pr makes sense to me. And is there same issue in ORC ff?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] zhongyujiang closed pull request #3605: make ApplyNameMapping keep field's name after name mapping.
Posted by GitBox <gi...@apache.org>.
zhongyujiang closed pull request #3605:
URL: https://github.com/apache/iceberg/pull/3605
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] chenjunjiedada commented on pull request #3605: make ApplyNameMapping keep field's name after name mapping.
Posted by GitBox <gi...@apache.org>.
chenjunjiedada commented on pull request #3605:
URL: https://github.com/apache/iceberg/pull/3605#issuecomment-978918750
Looks correct, could you please add a unit test for this？
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] zhongyujiang commented on pull request #3605: make ApplyNameMapping keep field's name after name mapping.
Posted by GitBox <gi...@apache.org>.
zhongyujiang commented on pull request #3605:
URL: https://github.com/apache/iceberg/pull/3605#issuecomment-980645315
Hi @chenjunjiedada, I found that [RemoveIds](https://github.com/apache/iceberg/blob/master/parquet/src/main/java/org/apache/iceberg/parquet/RemoveIds.java#L50) haven't keep the original name of repeatedElement of list/map type too:
```
@Override
public Type list(GroupType array, Type item) {
return Types.list(array.getRepetition())
.element(item)
.named(array.getName());
}
```
I think this also need a fix.
And speaking of unit test, I found that [TestPruneColumns](https://github.com/apache/iceberg/blob/master/parquet/src/test/java/org/apache/iceberg/parquet/TestPruneColumns.java) already did a similar one for PruneColumns, maybe I can reuse the fileSchema in it, so can I just rename TestPruneColumns to TestParquetTypeVisitor and put the unit tests for RemoveIds and ApplyNameMapping in it? What do you think?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] zhongyujiang commented on pull request #3605: make ApplyNameMapping keep field's name after name mapping.
Posted by GitBox <gi...@apache.org>.
zhongyujiang commented on pull request #3605:
URL: https://github.com/apache/iceberg/pull/3605#issuecomment-997899516
> In general, this pr makes sense to me, and is there same issue in ORC ff?
Sorry, I don't know much of ORC, and I think this issue is already solved by this [pr](https://github.com/apache/iceberg/pull/3723), it's already merged. I'll close this one.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] zhongyujiang commented on pull request #3605: make ApplyNameMapping keep field's name after name mapping.
Posted by GitBox <gi...@apache.org>.
zhongyujiang commented on pull request #3605:
URL: https://github.com/apache/iceberg/pull/3605#issuecomment-980637193
> Looks correct, could you please add a unit test for this？
Of course!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] uncleGen commented on pull request #3605: make ApplyNameMapping keep field's name after name mapping.
Posted by GitBox <gi...@apache.org>.
uncleGen commented on pull request #3605:
URL: https://github.com/apache/iceberg/pull/3605#issuecomment-997710093
Besides, could you please add an end-to-end unit test?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] zhongyujiang commented on pull request #3605: make ApplyNameMapping keep field's name after name mapping.
Posted by GitBox <gi...@apache.org>.
zhongyujiang commented on pull request #3605:
URL: https://github.com/apache/iceberg/pull/3605#issuecomment-982582243
Hey @chenjunjiedada , I make RemoveIds keeps field's original name too and add unit tests for both RemoveIds and ApplyNameMapping in the latest commit, could you please give it a review?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org