You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "hunter-cloud09 (via GitHub)" <gi...@apache.org> on 2023/02/14 02:40:37 UTC
[GitHub] [iceberg] hunter-cloud09 opened a new pull request, #6834: Api: Add override method
hunter-cloud09 opened a new pull request, #6834:
URL: https://github.com/apache/iceberg/pull/6834
The IcebergGenerics class only has String ... method for select, should add Collection for select will be better!
feat #6833
--
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] Fokko merged pull request #6834: Data: Add a select interface with collections for IcebergGenerics
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko merged PR #6834:
URL: https://github.com/apache/iceberg/pull/6834
--
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] hunter-cloud09 commented on pull request #6834: Data: Add a select interface with collections for IcebergGenerics
Posted by "hunter-cloud09 (via GitHub)" <gi...@apache.org>.
hunter-cloud09 commented on PR #6834:
URL: https://github.com/apache/iceberg/pull/6834#issuecomment-1431047119
@ajantha-bhat Is there anything else I need to do before merging?
--
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] Fokko commented on pull request #6834: Data: Add a select interface with collections for IcebergGenerics
Posted by "Fokko (via GitHub)" <gi...@apache.org>.
Fokko commented on PR #6834:
URL: https://github.com/apache/iceberg/pull/6834#issuecomment-1432645724
Thanks @hunter-cloud09 for adding this, and @ajantha-bhat @nastra for the 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
[GitHub] [iceberg] hunter-cloud09 commented on pull request #6834: Data: Add a select interface with collections for IcebergGenerics
Posted by "hunter-cloud09 (via GitHub)" <gi...@apache.org>.
hunter-cloud09 commented on PR #6834:
URL: https://github.com/apache/iceberg/pull/6834#issuecomment-1430853338
> LGTM.
>
> PR title can be "Data: Add a select interface with collections for IcebergGenerics"
Get It!
--
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] hunter-cloud09 commented on a diff in pull request #6834: Api: Add override method
Posted by "hunter-cloud09 (via GitHub)" <gi...@apache.org>.
hunter-cloud09 commented on code in PR #6834:
URL: https://github.com/apache/iceberg/pull/6834#discussion_r1106701320
##########
data/src/main/java/org/apache/iceberg/data/IcebergGenerics.java:
##########
@@ -66,6 +67,11 @@ public ScanBuilder select(String... selectedColumns) {
return this;
}
+ public ScanBuilder select(Collection<String> columns) {
+ this.tableScan = tableScan.select(ImmutableList.copyOf(columns));
Review Comment:
get it, i'll change there
--
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] ajantha-bhat commented on a diff in pull request #6834: Api: Add override method
Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on code in PR #6834:
URL: https://github.com/apache/iceberg/pull/6834#discussion_r1106678601
##########
data/src/main/java/org/apache/iceberg/data/IcebergGenerics.java:
##########
@@ -66,6 +67,11 @@ public ScanBuilder select(String... selectedColumns) {
return this;
}
+ public ScanBuilder select(Collection<String> columns) {
+ this.tableScan = tableScan.select(ImmutableList.copyOf(columns));
Review Comment:
Just wondering do we really need a copy of the collection again? Can't we pass the collection directly as the below interface supports it?
https://github.com/apache/iceberg/blob/master/api/src/main/java/org/apache/iceberg/Scan.java#L87
--
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] hunter-cloud09 commented on pull request #6834: Api: Add override method
Posted by "hunter-cloud09 (via GitHub)" <gi...@apache.org>.
hunter-cloud09 commented on PR #6834:
URL: https://github.com/apache/iceberg/pull/6834#issuecomment-1429377303
@nastra @Fokko thx for your review, This problem is only encountered when I use it. I will check the features of TableScan later, and then add it to IcebergGenerics.
--
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] hunter-cloud09 commented on a diff in pull request #6834: Api: Add override method
Posted by "hunter-cloud09 (via GitHub)" <gi...@apache.org>.
hunter-cloud09 commented on code in PR #6834:
URL: https://github.com/apache/iceberg/pull/6834#discussion_r1106684445
##########
data/src/main/java/org/apache/iceberg/data/IcebergGenerics.java:
##########
@@ -66,6 +67,11 @@ public ScanBuilder select(String... selectedColumns) {
return this;
}
+ public ScanBuilder select(Collection<String> columns) {
+ this.tableScan = tableScan.select(ImmutableList.copyOf(columns));
Review Comment:
In my opinion, there is no need to copy, but I am not sure why the previous author did this? Is there any special purpose?
--
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] ajantha-bhat commented on a diff in pull request #6834: Api: Add override method
Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on code in PR #6834:
URL: https://github.com/apache/iceberg/pull/6834#discussion_r1106689940
##########
data/src/main/java/org/apache/iceberg/data/IcebergGenerics.java:
##########
@@ -66,6 +67,11 @@ public ScanBuilder select(String... selectedColumns) {
return this;
}
+ public ScanBuilder select(Collection<String> columns) {
+ this.tableScan = tableScan.select(ImmutableList.copyOf(columns));
Review Comment:
they may want to convert arrays into a collection and use the collection interface. But that is also not needed now as the underlying interface has a conversion logic.
https://github.com/apache/iceberg/blob/634215fcd411dd8a6d5c0b6d7d8dbf4eb66494d8/api/src/main/java/org/apache/iceberg/Scan.java#L96-L98
--
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