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 2019/12/26 03:32:20 UTC
[GitHub] [incubator-iceberg] waterlx opened a new pull request #719: Route
to Hadoop table more strictly in IcebergSource#getTable()
waterlx opened a new pull request #719: Route to Hadoop table more strictly in IcebergSource#getTable()
URL: https://github.com/apache/incubator-iceberg/pull/719
The PR make the routing to Hadoop table more strictly.
Originally I would like to use functions in java.net.URI (like isAbsolute, isOpaque...) to classify the path as a String, but it adds complexity (more exceptions to handle) and seems does not worth 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [incubator-iceberg] rdblue closed pull request #719: Route to
Hadoop table more strictly in IcebergSource#getTable()
Posted by GitBox <gi...@apache.org>.
rdblue closed pull request #719: Route to Hadoop table more strictly in IcebergSource#getTable()
URL: https://github.com/apache/incubator-iceberg/pull/719
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [incubator-iceberg] rdblue commented on a change in pull request
#719: Route to Hadoop table more strictly in IcebergSource#getTable()
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #719: Route to Hadoop table more strictly in IcebergSource#getTable()
URL: https://github.com/apache/incubator-iceberg/pull/719#discussion_r361719518
##########
File path: spark/src/main/java/org/apache/iceberg/spark/source/IcebergSource.java
##########
@@ -149,6 +149,12 @@ protected Table findTable(DataSourceOptions options, Configuration conf) {
}
}
+ private boolean routeToHadoopTable(final String path) {
Review comment:
Can you give examples of paths that you think are handled incorrectly by the current logic?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [incubator-iceberg] waterlx commented on a change in pull request
#719: Route to Hadoop table more strictly in IcebergSource#getTable()
Posted by GitBox <gi...@apache.org>.
waterlx commented on a change in pull request #719: Route to Hadoop table more strictly in IcebergSource#getTable()
URL: https://github.com/apache/incubator-iceberg/pull/719#discussion_r361908519
##########
File path: spark/src/main/java/org/apache/iceberg/spark/source/IcebergSource.java
##########
@@ -149,6 +149,12 @@ protected Table findTable(DataSourceOptions options, Configuration conf) {
}
}
+ private boolean routeToHadoopTable(final String path) {
Review comment:
I thought path = "db.tab/le" (table name or db name contains "/") might be better to be routed to Hive table instead of Hadoop table (as the current logic). But as I tried, Hive do not allow us to create a db or a table with "/" in its name. This PR seems not necessary.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [incubator-iceberg] rdblue commented on issue #719: Route to Hadoop
table more strictly in IcebergSource#getTable()
Posted by GitBox <gi...@apache.org>.
rdblue commented on issue #719: Route to Hadoop table more strictly in IcebergSource#getTable()
URL: https://github.com/apache/incubator-iceberg/pull/719#issuecomment-569747444
Thanks for looking into it more, @waterlx! I'm going to close this since you say it doesn't seem necessary to make these changes.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org