You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@inlong.apache.org by GitBox <gi...@apache.org> on 2022/04/29 13:38:07 UTC

[GitHub] [incubator-inlong] woofyzhao opened a new pull request, #4041: [INLONG-4040][Manager] Fix hive sink table data path

woofyzhao opened a new pull request, #4041:
URL: https://github.com/apache/incubator-inlong/pull/4041

   Fixes #4040
   Tested in dev env.


-- 
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: commits-unsubscribe@inlong.apache.org

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


[GitHub] [incubator-inlong] woofyzhao commented on a diff in pull request #4041: [INLONG-4040][Manager] Fix hive sink table data path

Posted by GitBox <gi...@apache.org>.
woofyzhao commented on code in PR #4041:
URL: https://github.com/apache/incubator-inlong/pull/4041#discussion_r862641758


##########
inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/sort/util/SinkInfoUtils.java:
##########
@@ -182,13 +183,15 @@ private static HiveSinkInfo createHiveSinkInfo(HiveSinkResponse hiveInfo, List<F
             }).collect(Collectors.toList());
         }
 
-        // dataPath = dataPath + / + tableName
-        StringBuilder dataPathBuilder = new StringBuilder();
+        // dataPath = dataPath + / + dbName + / + tableName

Review Comment:
   Fix compatibility issue for the default hive db.
   
   No matter where  you create hive table the table name must be part of the path and must also be specified here to let sort write to the correct file. The default hive location is fine. All that's left is to take the default database case into account. 



##########
inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/sort/util/SinkInfoUtils.java:
##########
@@ -182,13 +183,15 @@ private static HiveSinkInfo createHiveSinkInfo(HiveSinkResponse hiveInfo, List<F
             }).collect(Collectors.toList());
         }
 
-        // dataPath = dataPath + / + tableName
-        StringBuilder dataPathBuilder = new StringBuilder();
+        // dataPath = dataPath + / + dbName + / + tableName

Review Comment:
   Fixed compatibility issue for the default hive db.
   
   No matter where  you create hive table the table name must be part of the path and must also be specified here to let sort write to the correct file. The default hive location is fine. All that's left is to take the default database case into account. 



-- 
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: commits-unsubscribe@inlong.apache.org

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


[GitHub] [incubator-inlong] woofyzhao closed pull request #4041: [INLONG-4040][Manager] Fix hive sink table data path

Posted by GitBox <gi...@apache.org>.
woofyzhao closed pull request #4041: [INLONG-4040][Manager] Fix hive sink table data path
URL: https://github.com/apache/incubator-inlong/pull/4041


-- 
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: commits-unsubscribe@inlong.apache.org

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


[GitHub] [incubator-inlong] woofyzhao commented on a diff in pull request #4041: [INLONG-4040][Manager] Fix hive sink table data path

Posted by GitBox <gi...@apache.org>.
woofyzhao commented on code in PR #4041:
URL: https://github.com/apache/incubator-inlong/pull/4041#discussion_r862641758


##########
inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/sort/util/SinkInfoUtils.java:
##########
@@ -182,13 +183,15 @@ private static HiveSinkInfo createHiveSinkInfo(HiveSinkResponse hiveInfo, List<F
             }).collect(Collectors.toList());
         }
 
-        // dataPath = dataPath + / + tableName
-        StringBuilder dataPathBuilder = new StringBuilder();
+        // dataPath = dataPath + / + dbName + / + tableName

Review Comment:
   Fixed compatibility issue for the default hive db.
   
   No matter where  you create hive table the table name must be part of the path and must also be specified here to ensure sort writing to the correct file. The default hive location is fine. All that's left is to take the default database special case into account. 



-- 
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: commits-unsubscribe@inlong.apache.org

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


[GitHub] [incubator-inlong] woofyzhao commented on a diff in pull request #4041: [INLONG-4040][Manager] Fix hive sink table data path

Posted by GitBox <gi...@apache.org>.
woofyzhao commented on code in PR #4041:
URL: https://github.com/apache/incubator-inlong/pull/4041#discussion_r862641758


##########
inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/sort/util/SinkInfoUtils.java:
##########
@@ -182,13 +183,15 @@ private static HiveSinkInfo createHiveSinkInfo(HiveSinkResponse hiveInfo, List<F
             }).collect(Collectors.toList());
         }
 
-        // dataPath = dataPath + / + tableName
-        StringBuilder dataPathBuilder = new StringBuilder();
+        // dataPath = dataPath + / + dbName + / + tableName

Review Comment:
   Added compatibility for the default db case.
   
   No matter where  you create hive table the table name must be part of the path and must also be specified here to let sort write to the correct file. The default hive approach is fine. All that's left is to take the default database case into account. 



-- 
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: commits-unsubscribe@inlong.apache.org

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


[GitHub] [incubator-inlong] woofyzhao commented on a diff in pull request #4041: [INLONG-4040][Manager] Fix hive sink table data path

Posted by GitBox <gi...@apache.org>.
woofyzhao commented on code in PR #4041:
URL: https://github.com/apache/incubator-inlong/pull/4041#discussion_r862641758


##########
inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/sort/util/SinkInfoUtils.java:
##########
@@ -182,13 +183,15 @@ private static HiveSinkInfo createHiveSinkInfo(HiveSinkResponse hiveInfo, List<F
             }).collect(Collectors.toList());
         }
 
-        // dataPath = dataPath + / + tableName
-        StringBuilder dataPathBuilder = new StringBuilder();
+        // dataPath = dataPath + / + dbName + / + tableName

Review Comment:
   Fixed compatibility issue for the default hive db.
   
   No matter where  you create hive table the table name must be part of the path and must also be specified here to sure sort writing to the correct file. The default hive location is fine. All that's left is to take the default database special case into account. 



-- 
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: commits-unsubscribe@inlong.apache.org

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


[GitHub] [incubator-inlong] woofyzhao commented on a diff in pull request #4041: [INLONG-4040][Manager] Fix hive sink table data path

Posted by GitBox <gi...@apache.org>.
woofyzhao commented on code in PR #4041:
URL: https://github.com/apache/incubator-inlong/pull/4041#discussion_r862641758


##########
inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/sort/util/SinkInfoUtils.java:
##########
@@ -182,13 +183,15 @@ private static HiveSinkInfo createHiveSinkInfo(HiveSinkResponse hiveInfo, List<F
             }).collect(Collectors.toList());
         }
 
-        // dataPath = dataPath + / + tableName
-        StringBuilder dataPathBuilder = new StringBuilder();
+        // dataPath = dataPath + / + dbName + / + tableName

Review Comment:
   Fixed compatibility issue for the default hive db.
   
   No matter where  you create hive table the table name must be part of the path and must also be specified here to ensure sort writing to the correct file. The default hive location is fine. All that's left is to take care of the default database special case.



-- 
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: commits-unsubscribe@inlong.apache.org

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


[GitHub] [incubator-inlong] woofyzhao commented on a diff in pull request #4041: [INLONG-4040][Manager] Fix hive sink table data path

Posted by GitBox <gi...@apache.org>.
woofyzhao commented on code in PR #4041:
URL: https://github.com/apache/incubator-inlong/pull/4041#discussion_r862641758


##########
inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/sort/util/SinkInfoUtils.java:
##########
@@ -182,13 +183,15 @@ private static HiveSinkInfo createHiveSinkInfo(HiveSinkResponse hiveInfo, List<F
             }).collect(Collectors.toList());
         }
 
-        // dataPath = dataPath + / + tableName
-        StringBuilder dataPathBuilder = new StringBuilder();
+        // dataPath = dataPath + / + dbName + / + tableName

Review Comment:
   Added compatibility for the default db case.
   
   No matter where  you create hive table the table name must be part of the path and must also be specified here to let sort write to the correct file. The default hive location is fine. All that's left is to take the default database case into account. 



-- 
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: commits-unsubscribe@inlong.apache.org

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


[GitHub] [incubator-inlong] woofyzhao commented on pull request #4041: [INLONG-4040][Manager] Fix hive sink table data path

Posted by GitBox <gi...@apache.org>.
woofyzhao commented on PR #4041:
URL: https://github.com/apache/incubator-inlong/pull/4041#issuecomment-1118103382

   It turns out that the user should specify full path rather than just warehouse prefix.


-- 
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: commits-unsubscribe@inlong.apache.org

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


[GitHub] [incubator-inlong] healchow commented on a diff in pull request #4041: [INLONG-4040][Manager] Fix hive sink table data path

Posted by GitBox <gi...@apache.org>.
healchow commented on code in PR #4041:
URL: https://github.com/apache/incubator-inlong/pull/4041#discussion_r861889400


##########
inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/sort/util/SinkInfoUtils.java:
##########
@@ -182,13 +183,15 @@ private static HiveSinkInfo createHiveSinkInfo(HiveSinkResponse hiveInfo, List<F
             }).collect(Collectors.toList());
         }
 
-        // dataPath = dataPath + / + tableName
-        StringBuilder dataPathBuilder = new StringBuilder();
+        // dataPath = dataPath + / + dbName + / + tableName

Review Comment:
   We use the data path and table name as the real path to writing files.
   If changed, maybe occurred an exception in other cases.



-- 
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: commits-unsubscribe@inlong.apache.org

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


[GitHub] [incubator-inlong] healchow commented on a diff in pull request #4041: [INLONG-4040][Manager] Fix hive sink table data path

Posted by GitBox <gi...@apache.org>.
healchow commented on code in PR #4041:
URL: https://github.com/apache/incubator-inlong/pull/4041#discussion_r862262857


##########
inlong-manager/manager-service/src/main/java/org/apache/inlong/manager/service/sort/util/SinkInfoUtils.java:
##########
@@ -182,13 +183,15 @@ private static HiveSinkInfo createHiveSinkInfo(HiveSinkResponse hiveInfo, List<F
             }).collect(Collectors.toList());
         }
 
-        // dataPath = dataPath + / + tableName
-        StringBuilder dataPathBuilder = new StringBuilder();
+        // dataPath = dataPath + / + dbName + / + tableName

Review Comment:
   Maybe you can change the usage of the dataPath when creating Hive tables.



-- 
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: commits-unsubscribe@inlong.apache.org

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