[GitHub] [hive] kgyrtkirk commented on a change in pull request #2211: HIVE-23931: Send ValidWriteIdList and tableId to get_*_constraints HMS APIs

2021-05-14 Thread GitBox 


kgyrtkirk commented on a change in pull request #2211:
URL: https://github.com/apache/hive/pull/2211#discussion_r632474203



##
File path: 
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/cache/CachedStore.java
##
@@ -2688,13 +2734,21 @@ long getPartsFound() {
* @throws MetaException
*/
   @Override
+  @Deprecated
   public SQLAllTableConstraints getAllTableConstraints(String catName, String 
dbName, String tblName)
   throws MetaException, NoSuchObjectException {
-catName = StringUtils.normalizeIdentifier(catName);
-dbName = StringUtils.normalizeIdentifier(dbName);
-tblName = StringUtils.normalizeIdentifier(tblName);
+AllTableConstraintsRequest request = new 
AllTableConstraintsRequest(dbName,tblName,catName);
+return getAllTableConstraints(request);
+  }
+
+  @Override
+  public SQLAllTableConstraints 
getAllTableConstraints(AllTableConstraintsRequest request)
+  throws MetaException, NoSuchObjectException {
+String catName = StringUtils.normalizeIdentifier(request.getCatName());
+String dbName = StringUtils.normalizeIdentifier(request.getDbName());
+String tblName = StringUtils.normalizeIdentifier(request.getTblName());

Review comment:
   having some common table reference object could remove this copy paste 
as well

##
File path: 
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift
##
@@ -701,6 +705,8 @@ struct UniqueConstraintsRequest {
   1: required string catName,
   2: required string db_name,
   3: required string tbl_name,
+  4: optional string validWriteIdList,
+  5: optional i64 tableId=-1

Review comment:
   I meaned something which could retain that we may change the request 
further but have the table related stuff separated:
   {code}
   struct UniqueConstraintsRequest {
   1: TableReference table;
   }
   
   struct TableReference {
1: required string catName,
 2: required string db_name,
 3: required string tbl_name,
 4: optional string validWriteIdList,
 5: optional i64 tableId=-1
   }
   
   {code}
   
   this could also help clean things up a bit




-- 
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:
us...@infra.apache.org



-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org

[GitHub] [hive] kgyrtkirk commented on a change in pull request #2211: HIVE-23931: Send ValidWriteIdList and tableId to get_*_constraints HMS APIs

2021-05-14 Thread GitBox 


kgyrtkirk commented on a change in pull request #2211:
URL: https://github.com/apache/hive/pull/2211#discussion_r632473124



##
File path: 
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift
##
@@ -701,6 +705,8 @@ struct UniqueConstraintsRequest {
   1: required string catName,
   2: required string db_name,
   3: required string tbl_name,
+  4: optional string validWriteIdList,
+  5: optional i64 tableId=-1

Review comment:
   I wonder if a common object to reference a table might be handy ; these 
5 fields are copied to a bunch of requests/responses - having a common object 
might make it easier to work with them




-- 
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:
us...@infra.apache.org



-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org

[GitHub] [hive] kgyrtkirk commented on a change in pull request #2211: HIVE-23931: Send ValidWriteIdList and tableId to get_*_constraints HMS APIs

2021-05-14 Thread GitBox 


kgyrtkirk commented on a change in pull request #2211:
URL: https://github.com/apache/hive/pull/2211#discussion_r632471606



##
File path: 
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift
##
@@ -690,7 +692,9 @@ struct ForeignKeysRequest {
   2: string parent_tbl_name,
   3: string foreign_db_name,
   4: string foreign_tbl_name

Review comment:
   `,` missing?




-- 
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:
us...@infra.apache.org



-
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org