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/19 18:32:56 UTC
[GitHub] [iceberg] rdblue opened a new pull request #3773: Parquet: Simplify changes from #3723
rdblue opened a new pull request #3773:
URL: https://github.com/apache/iceberg/pull/3773
This makes a couple of modifications to #3723 to make it more consistent and simpler.
* Instead of creating a fake element type, add "element" to field names directly
* To support direct control of `fieldNames`, move name handling into `ApplyNameMapping`
* Remove unnecessary method, `makeListType`
* Apply similar changes to maps to avoid name mismatch bugs in the future
--
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 merged pull request #3773: Parquet: Simplify changes from #3723
Posted by GitBox <gi...@apache.org>.
rdblue merged pull request #3773:
URL: https://github.com/apache/iceberg/pull/3773
--
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 #3773: Parquet: Simplify changes from #3723
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #3773:
URL: https://github.com/apache/iceberg/pull/3773#issuecomment-997439714
@SinghAsDev FYI
--
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 #3773: Parquet: Simplify changes from #3723
Posted by GitBox <gi...@apache.org>.
uncleGen commented on pull request #3773:
URL: https://github.com/apache/iceberg/pull/3773#issuecomment-998428937
#3605 may be fixed by this pr.
--
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 removed a comment on pull request #3773: Parquet: Simplify changes from #3723
Posted by GitBox <gi...@apache.org>.
uncleGen removed a comment on pull request #3773:
URL: https://github.com/apache/iceberg/pull/3773#issuecomment-998428937
#3605 may be fixed by this pr.
--
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 a change in pull request #3773: Parquet: Simplify changes from #3723
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #3773:
URL: https://github.com/apache/iceberg/pull/3773#discussion_r773326578
##########
File path: parquet/src/main/java/org/apache/iceberg/parquet/ApplyNameMapping.java
##########
@@ -97,29 +96,30 @@ public Type primitive(PrimitiveType primitive) {
return field == null ? primitive : primitive.withId(field.id());
}
+ public void beforeField(Type type) {
+ fieldNames.push(type.getName());
+ }
+
+ public void afterField(Type type) {
+ fieldNames.pop();
+ }
+
@Override
public void beforeElementField(Type element) {
- super.beforeElementField(makeElement(element));
+ // normalize the name to "element" so that the mapping will match structures with alternative names
Review comment:
I don't think so. The main point is that we don't want to care what the other name was.
--
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] SinghAsDev commented on a change in pull request #3773: Parquet: Simplify changes from #3723
Posted by GitBox <gi...@apache.org>.
SinghAsDev commented on a change in pull request #3773:
URL: https://github.com/apache/iceberg/pull/3773#discussion_r771989421
##########
File path: parquet/src/main/java/org/apache/iceberg/parquet/ApplyNameMapping.java
##########
@@ -97,29 +96,30 @@ public Type primitive(PrimitiveType primitive) {
return field == null ? primitive : primitive.withId(field.id());
}
+ public void beforeField(Type type) {
+ fieldNames.push(type.getName());
+ }
+
+ public void afterField(Type type) {
+ fieldNames.pop();
+ }
+
@Override
public void beforeElementField(Type element) {
- super.beforeElementField(makeElement(element));
+ // normalize the name to "element" so that the mapping will match structures with alternative names
Review comment:
@rdblue would it help if we add some example of this alternate naming source here? Same for the map's key and value fields.
--
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] SinghAsDev commented on pull request #3773: Parquet: Simplify changes from #3723
Posted by GitBox <gi...@apache.org>.
SinghAsDev commented on pull request #3773:
URL: https://github.com/apache/iceberg/pull/3773#issuecomment-997444310
Thanks @rdblue , this makes sense. I like that we don't have to modify/ construct parquet types with this change.
I have a couple more changes along this, and would like to get your reviews on them. Will likely post them today.
--
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 #3773: Parquet: Simplify changes from #3723
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #3773:
URL: https://github.com/apache/iceberg/pull/3773#issuecomment-997439670
@RussellSpitzer can you take a look at this follow-up to #3723?
--
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