You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@paimon.apache.org by "humengyu2012 (via GitHub)" <gi...@apache.org> on 2023/11/14 12:42:45 UTC

[PR] [Hive] Using hive table location when catalog is HiveCatalog [incubator-paimon]

humengyu2012 opened a new pull request, #2315:
URL: https://github.com/apache/incubator-paimon/pull/2315

   ### Purpose
   
   We want to query Paimon tables with Presto, but our Paimon tables are scattered across different warehouse directories(such as `/user1/warehouse`, `/user2/warehouse`).
   Therefore, we use Hive catalog to discover the addresses of different Paimon warehouses. However, in the current implementation of the Hive catalog, it does not use the Hive table's location as the location for Paimon tables.
   
   ### Tests
   
   <!-- List UT and IT cases to verify this change -->
   
   ### API and Format
   
   <!-- Does this change affect API or storage format -->
   
   ### Documentation
   
   <!-- Does this change introduce a new feature -->
   


-- 
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@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [Hive] Using hive table location when catalog is HiveCatalog [incubator-paimon]

Posted by "humengyu2012 (via GitHub)" <gi...@apache.org>.
humengyu2012 commented on code in PR #2315:
URL: https://github.com/apache/incubator-paimon/pull/2315#discussion_r1393989670


##########
paimon-hive/paimon-hive-catalog/src/main/java/org/apache/paimon/hive/HiveCatalog.java:
##########
@@ -161,6 +161,18 @@ public Optional<MetastoreClient.Factory> metastoreClientFactory(Identifier ident
         }
     }
 
+    @Override
+    public Path getDataTableLocation(Identifier identifier) {
+        try {
+            return new Path(
+                    client.getTable(identifier.getDatabaseName(), identifier.getObjectName())
+                            .getSd()
+                            .getLocation());
+        } catch (TException e) {

Review Comment:
   Regarding HiveCatalog, I have a question: should we fallback to the parent implemets, or should we adhere to the Hive specification and create tables under the database path? @JingsongLi 



-- 
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@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [Hive] Using hive table location when catalog is HiveCatalog [incubator-paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi merged PR #2315:
URL: https://github.com/apache/incubator-paimon/pull/2315


-- 
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@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [Hive] Using hive table location when catalog is HiveCatalog [incubator-paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #2315:
URL: https://github.com/apache/incubator-paimon/pull/2315#discussion_r1393526281


##########
paimon-hive/paimon-hive-catalog/src/main/java/org/apache/paimon/hive/HiveCatalog.java:
##########
@@ -161,6 +161,18 @@ public Optional<MetastoreClient.Factory> metastoreClientFactory(Identifier ident
         }
     }
 
+    @Override
+    public Path getDataTableLocation(Identifier identifier) {
+        try {
+            return new Path(
+                    client.getTable(identifier.getDatabaseName(), identifier.getObjectName())
+                            .getSd()
+                            .getLocation());
+        } catch (TException e) {

Review Comment:
   ```
   catch (NoSuchObjectException e) {
        return super.getDataTableLocation(identifier);
   }
   ```



-- 
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@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [Hive] Using hive table location when catalog is HiveCatalog [incubator-paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #2315:
URL: https://github.com/apache/incubator-paimon/pull/2315#discussion_r1392688122


##########
paimon-hive/paimon-hive-catalog/src/main/java/org/apache/paimon/hive/HiveCatalog.java:
##########
@@ -161,6 +161,18 @@ public Optional<MetastoreClient.Factory> metastoreClientFactory(Identifier ident
         }
     }
 
+    @Override
+    public Path getDataTableLocation(Identifier identifier) {
+        try {
+            return new Path(

Review Comment:
   Introduce a method to `LocationHelper`: `getTableLocation`.



-- 
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@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [Hive] Using hive table location when catalog is HiveCatalog [incubator-paimon]

Posted by "JingsongLi (via GitHub)" <gi...@apache.org>.
JingsongLi commented on code in PR #2315:
URL: https://github.com/apache/incubator-paimon/pull/2315#discussion_r1395050954


##########
paimon-hive/paimon-hive-catalog/src/main/java/org/apache/paimon/hive/HiveCatalog.java:
##########
@@ -161,6 +161,18 @@ public Optional<MetastoreClient.Factory> metastoreClientFactory(Identifier ident
         }
     }
 
+    @Override
+    public Path getDataTableLocation(Identifier identifier) {
+        try {
+            return new Path(
+                    client.getTable(identifier.getDatabaseName(), identifier.getObjectName())
+                            .getSd()
+                            .getLocation());
+        } catch (TException e) {

Review Comment:
   OK let's just throw exception.



-- 
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@paimon.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org