[GitHub] [incubator-paimon] JingsongLi commented on a diff in pull request #886: [hive]Setting the location of paimon table in the tbproperties of hive table when creating the hive extern table

["JingsongLi (via GitHub)" <gi...@apache.org>]

JingsongLi commented on code in PR #886:
URL: https://github.com/apache/incubator-paimon/pull/886#discussion_r1185713629


##########
paimon-hive/paimon-hive-common/src/main/java/org/apache/paimon/hive/LocationKeyExtractor.java:
##########
@@ -18,11 +18,20 @@
 
 package org.apache.paimon.hive;
 
+import java.util.Properties;
+
 /**
  * declaring the name of the key in the parameters of the Hive metastore table, which indicates
  * where the Paimon table is stored.
  */
 public class LocationKeyExtractor {
     // special at the tbproperties with the name paimon_meta_location.
     public static final String TBPROPERTIES_LOCATION_KEY = "paimon_meta_location";
+    public static final String STORAGE_LOCATION_KEY = "location";
+
+    public static String getLocation(Properties properties) {
+        String storageLocation = properties.getProperty(STORAGE_LOCATION_KEY);

Review Comment:
   Can we just use `hive_metastoreConstants.META_TABLE_LOCATION`?



##########
docs/content/engines/hive.md:
##########
@@ -211,6 +211,18 @@ OK
 3	Paimon
 */
 
+```
+## Setting of location Location 
+In addition to the way setting location above, you can also place the location setting in TBProperties 
+to avoid Hive accessing Paimon's location through its own file system when creating tables.
+This method is effective in scenarios using Object storage,such as s3.
+
+```sql
+CREATE EXTERNAL TABLE external_test_table
+STORED BY 'org.apache.paimon.hive.PaimonStorageHandler'
+TBLPROPERTIES (
+ 'paimon_meta_location' ='s3://xxxxx/path/to/table/store/warehouse/default.db/test_table'

Review Comment:
   After looking at the api again, I feel we can adjust the key.
   Maybe `paimon_location`?



##########
paimon-hive/paimon-hive-connector-common/src/main/java/org/apache/paimon/hive/PaimonJobConf.java:
##########
@@ -91,4 +91,13 @@ public static Options extractCatalogConfig(Configuration hiveConf) {
         }
         return Options.fromMap(configMap);
     }
+
+    public static void setLocationFromProperties(

Review Comment:
   Why not using `configureOutputJobProperties` or `configureInputJobProperties`?



##########
paimon-hive/paimon-hive-connector-common/src/main/java/org/apache/paimon/hive/mapred/PaimonInputFormat.java:
##########
@@ -48,8 +49,18 @@ public InputSplit[] getSplits(JobConf jobConf, int numSplits) {
         FileStoreTable table = createFileStoreTable(jobConf);
         InnerTableScan scan = table.newScan();
         createPredicate(table.schema(), jobConf).ifPresent(scan::withFilter);
+
+        final String location;
+
+        String propertiesLocation = jobConf.get(LocationKeyExtractor.STORAGE_LOCATION_KEY);

Review Comment:
   Why do this? `table.location()` should be correct.



-- 
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