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/12/20 13:41:23 UTC
[GitHub] [iceberg] zhongyujiang opened a new pull request #3777: [Parquet]Fix RemoveIds change parquet repeatedElement name
zhongyujiang opened a new pull request #3777:
URL: https://github.com/apache/iceberg/pull/3777
https://github.com/apache/iceberg/pull/3605#issuecomment-980645315
The RemoveIds won't keep the original custom name of repeatedElement of list/map type passed to it, instead uses a default name in new Parquet version. This PR fixed it.@rdblue can you help review this pr? Thanks!
--
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] rdblue commented on pull request #3777: [Parquet]Fix RemoveIds change parquet repeatedElement name
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #3777:
URL: https://github.com/apache/iceberg/pull/3777#issuecomment-998364711
This is only called from test code, so I think the motivation was to speculative since other classes had similar updates. I don't think that we should move forward with this since it isn't actually used.
What you could do instead is move this into src/test/java.
--
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] rdblue commented on pull request #3777: [Parquet]Fix RemoveIds change parquet repeatedElement name
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #3777:
URL: https://github.com/apache/iceberg/pull/3777#issuecomment-998142964
@zhongyujiang, what is the problem caused by 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 #3777: [Parquet]Fix RemoveIds change parquet repeatedElement name
Posted by GitBox <gi...@apache.org>.
zhongyujiang commented on pull request #3777:
URL: https://github.com/apache/iceberg/pull/3777#issuecomment-998408677
> This is only called from test code, so I think the motivation was to speculative since other classes had similar updates. I don't think that we should move forward with this since it isn't actually used.
I didn't notice that this is only used in test, and no, I am not speculating. If you read this [issue](https://github.com/apache/iceberg/issues/3604) and this [pr](https://github.com/apache/iceberg/pull/3605), you can find that I found the problem of ApplyNameMapping change custom parquet schema before another merged pr, I checked other classes extend ParquetTypeVisitor and found RemoveIds has the same problem. I fixed both in this [pr](https://github.com/apache/iceberg/pull/3605), but no one reviewed. Yesterday I found the issue of ApplyNameMapping is solved in another pr and already merged, so I closed my last pr and extracted the part of RemoveIds here. That's my motivation.
--
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] kbendick commented on pull request #3777: [Parquet]Fix RemoveIds change parquet repeatedElement name
Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #3777:
URL: https://github.com/apache/iceberg/pull/3777#issuecomment-998350293
@zhongyujiang could you update the PR description to further clarify what problem this is solving? It's hard to to tell when it's linked via a comment if we weren't involved in the initial comment discussion.
--
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] rdblue closed pull request #3777: [Parquet]Fix RemoveIds change parquet repeatedElement name
Posted by GitBox <gi...@apache.org>.
rdblue closed pull request #3777:
URL: https://github.com/apache/iceberg/pull/3777
--
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