You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2022/09/15 23:51:44 UTC

[GitHub] [hive] saihemanth-cloudera opened a new pull request, #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

saihemanth-cloudera opened a new pull request, #3599:
URL: https://github.com/apache/hive/pull/3599

   <!--
   Thanks for sending a pull request!  Here are some tips for you:
     1. If this is your first time, please read our contributor guidelines: https://cwiki.apache.org/confluence/display/Hive/HowToContribute
     2. Ensure that you have created an issue on the Hive project JIRA: https://issues.apache.org/jira/projects/HIVE/summary
     3. Ensure you have added or run the appropriate tests for your PR: 
     4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]HIVE-XXXXX:  Your PR title ...'.
     5. Be sure to keep the PR description updated to reflect all changes.
     6. Please write your PR title to summarize what this PR proposes.
     7. If possible, provide a concise example to reproduce the issue for a faster review.
   
   -->
   
   ### What changes were proposed in this pull request?
   Deprecated  older APIs in HMS and pointed them to the newer APIs
   <!--
   Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. 
   If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
     1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
     2. If you fix some SQL features, you can provide some references of other DBMSes.
     3. If there is design documentation, please add the link.
     4. If there is a discussion in the mailing list, please add the link.
   -->
   
   
   ### Why are the changes needed?
   Going forward HMS should be using the deprecated APIs.
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you propose a new API, clarify the use case for a new API.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   
   
   ### Does this PR introduce _any_ user-facing change?
   No
   <!--
   Note that it means *any* user-facing change including all aspects such as the documentation fix.
   If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description, screenshot and/or a reproducable example to show the behavior difference if possible.
   If possible, please also clarify if this is a user-facing change compared to the released Hive versions or within the unreleased branches such as master.
   If no, write 'No'.
   -->
   
   
   ### How was this patch tested?
   Unit tests.
   <!--
   If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
   If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
   If tests were not added, please describe why they were not added and/or why it was difficult to add.
   -->
   


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1024831377


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1363,18 +1363,62 @@ private void create_database_core(RawStore ms, final Database db)
   }
 
   @Override
+  @Deprecated
   public void create_database(final Database db)
       throws AlreadyExistsException, InvalidObjectException, MetaException {
-    startFunction("create_database", ": " + db.toString());
+    CreateDatabaseRequest req = new CreateDatabaseRequest(db.getName());
+    if (db.isSetDescription()) {

Review Comment:
   A CreateDatabaseRequest.Builder here to wrap the attributes is better.



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] sonarcloud[bot] commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1254567175

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3599)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [2 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [96 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=coverage&view=list) No Coverage information  
   [![No Duplication information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo-16px.png 'No Duplication information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=duplicated_lines_density&view=list) No Duplication information
   
   


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1199532586


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1360,19 +1384,49 @@ public void create_database(final Database db)
         }
         Deadline.checkTimeout();
       }
+      Database db = new Database(createDatabaseRequest.getDatabaseName(), createDatabaseRequest.getDescription(),
+              createDatabaseRequest.getLocationUri() ,createDatabaseRequest.getParameters());
+      if (createDatabaseRequest.isSetPrivileges()) {
+        db.setPrivileges(createDatabaseRequest.getPrivileges());
+      }
+      if (createDatabaseRequest.isSetOwnerName()) {
+        db.setOwnerName(createDatabaseRequest.getOwnerName());
+      }
+      if (createDatabaseRequest.isSetOwnerType()) {
+        db.setOwnerType(createDatabaseRequest.getOwnerType());
+      }
+      db.setCatalogName(createDatabaseRequest.getCatalogName());
+      if (createDatabaseRequest.isSetCreateTime()) {

Review Comment:
   If the create time is not given, I think we can just create a new one



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1211237025


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1335,18 +1335,42 @@ private void create_database_core(RawStore ms, final Database db)
   }
 
   @Override
+  @Deprecated
   public void create_database(final Database db)
       throws AlreadyExistsException, InvalidObjectException, MetaException {
-    startFunction("create_database", ": " + db.toString());
+    CreateDatabaseRequest req = new CreateDatabaseRequest(db.getName());
+    req.setDescription(db.getDescription());
+    req.setLocationUri(db.getLocationUri());
+    req.setParameters(db.getParameters());
+    req.setPrivileges(db.getPrivileges());
+    req.setOwnerName(db.getOwnerName());
+    req.setOwnerType(db.getOwnerType());
+    req.setCatalogName(db.getCatalogName());
+    req.setCreateTime(db.getCreateTime());
+    req.setManagedLocationUri(db.getManagedLocationUri());
+    req.setType(db.getType());
+    req.setDataConnectorName(db.getConnector_name());
+    req.setRemote_dbname(db.getRemote_dbname());
+
+    create_database_req(req);
+    //location and manged location might be set/changed.
+    db.setLocationUri(req.getLocationUri());

Review Comment:
   The method is declared as void, the old remote client does not expect this method to return anything, it couldn't reach out the the modified `db` unless using a get database request.
   I saw from the old `create_database_core` method:
   https://github.com/apache/hive/blob/master/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java#L1151
   
   it doesn't reset the location after flushing `db`. 



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] sonarcloud[bot] commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1249935378

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3599)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [3 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [87 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=coverage&view=list) No Coverage information  
   [![No Duplication information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo-16px.png 'No Duplication information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=duplicated_lines_density&view=list) No Duplication information
   
   


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1029969026


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2602,21 +2665,29 @@ service ThriftHiveMetastore extends fb303.FacebookService
   Partition append_partition_with_environment_context(1:string db_name, 2:string tbl_name,
       3:list<string> part_vals, 4:EnvironmentContext environment_context)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
+  Partition append_partition_req(1:AppendPartitionsRequest appendPartitionsReq)
+                       throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   Partition append_partition_by_name(1:string db_name, 2:string tbl_name, 3:string part_name)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   Partition append_partition_by_name_with_environment_context(1:string db_name, 2:string tbl_name,
       3:string part_name, 4:EnvironmentContext environment_context)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
+  Partition append_partition_by_name_req(1:AppendPartitionRequest appendPartitionRequest)
+        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   bool drop_partition(1:string db_name, 2:string tbl_name, 3:list<string> part_vals, 4:bool deleteData)
                        throws(1:NoSuchObjectException o1, 2:MetaException o2)
   bool drop_partition_with_environment_context(1:string db_name, 2:string tbl_name,
       3:list<string> part_vals, 4:bool deleteData, 5:EnvironmentContext environment_context)
                        throws(1:NoSuchObjectException o1, 2:MetaException o2)
+  bool drop_partition_req(1:DropPartitionRequest dropPartitionReq)
+                       throws(1:NoSuchObjectException o1, 2:MetaException o2)
   bool drop_partition_by_name(1:string db_name, 2:string tbl_name, 3:string part_name, 4:bool deleteData)
                        throws(1:NoSuchObjectException o1, 2:MetaException o2)
   bool drop_partition_by_name_with_environment_context(1:string db_name, 2:string tbl_name,
       3:string part_name, 4:bool deleteData, 5:EnvironmentContext environment_context)
                        throws(1:NoSuchObjectException o1, 2:MetaException o2)
+  bool drop_partition_by_name_req(1:DropPartitionRequest dropPartitionReq)

Review Comment:
   Yeah but both the APIs does different things. we cannot club both the APIs.



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1234985105


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1912,10 +1979,10 @@ public void create_dataconnector(final DataConnector connector)
     } catch (Exception e) {
       ex = e;
       throw handleException(e)
-          .throwIfInstance(MetaException.class, InvalidObjectException.class, AlreadyExistsException.class)
-          .defaultMetaException();
+              .throwIfInstance(MetaException.class, InvalidObjectException.class, AlreadyExistsException.class)

Review Comment:
   nit: unnecessary ident 



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1037637777


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -895,13 +895,14 @@ struct AddPartitionsResult {
 
 // Request type for add_partitions_req
 struct AddPartitionsRequest {
-  1: required string dbName,
-  2: required string tblName,
-  3: required list<Partition> parts,
+  1: optional string dbName,

Review Comment:
   Ack



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] sonarcloud[bot] commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1569237322

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3599)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [100 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=coverage&view=list) No Coverage information  
   [![No Duplication information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo-16px.png 'No Duplication information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=duplicated_lines_density&view=list) No Duplication information
   
   


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1154389917


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -939,6 +940,16 @@ struct DropPartitionsRequest {
   9: optional string catName
 }
 
+struct DropPartitionRequest {
+  1: optional string dbName,
+  2: optional string tblName,
+  3: optional string partName,
+  4: optional list<string> partVals,
+  5: optional bool deleteData,
+  6: optional EnvironmentContext environmentContext,
+  7: optional string catName

Review Comment:
   Ack



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] sonarcloud[bot] commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1498253468

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3599)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [92 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=coverage&view=list) No Coverage information  
   [![No Duplication information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo-16px.png 'No Duplication information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=duplicated_lines_density&view=list) No Duplication information
   
   


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1598419596

   Leave a minor comment, you can fix them as you want.


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


Re: [PR] HIVE-26537: Deprecate older APIs in the HMS thrift interface. [hive]

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1865234420

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3599) **Quality Gate passed**  
   The SonarCloud Quality Gate passed, but some issues were introduced.
   
   [92 New issues](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&inNewCodePeriod=true)  
   [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&inNewCodePeriod=true)  
   No data about Coverage  
   No data about Duplication  
     
   [See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3599)
   
   


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1161975796


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2679,7 +2742,8 @@ PartitionsResponse get_partitions_req(1:PartitionsRequest req)
 
   list<string> get_partition_names(1:string db_name, 2:string tbl_name, 3:i16 max_parts=-1)

Review Comment:
   We'll deprecate the older APIs in the next release.



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1199531087


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2451,19 +2497,24 @@ service ThriftHiveMetastore extends fb303.FacebookService
   void drop_catalog(1: DropCatalogRequest catName) throws (1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
 
   void create_database(1:Database database) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
+  void create_database_req(1:CreateDatabaseRequest createDatabaseRequest) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
   Database get_database(1:string name) throws(1:NoSuchObjectException o1, 2:MetaException o2)
   Database get_database_req(1:GetDatabaseRequest request) throws(1:NoSuchObjectException o1, 2:MetaException o2)
   void drop_database(1:string name, 2:bool deleteData, 3:bool cascade) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
   void drop_database_req(1:DropDatabaseRequest req) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
   list<string> get_databases(1:string pattern) throws(1:MetaException o1)
   list<string> get_all_databases() throws(1:MetaException o1)
   void alter_database(1:string dbname, 2:Database db) throws(1:MetaException o1, 2:NoSuchObjectException o2)
+  void alter_database_req(1:AlterDatabaseRequest alterDbReq) throws(1:MetaException o1, 2:NoSuchObjectException o2)
 
   void create_dataconnector(1:DataConnector connector) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
+  void create_dataconnector_req(1:CreateDataConnectorRequest connectorReq) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
   DataConnector get_dataconnector_req(1:GetDataConnectorRequest request) throws(1:NoSuchObjectException o1, 2:MetaException o2)
-  void drop_dataconnector(1:string name, bool ifNotExists, bool checkReferences) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
+  void drop_dataconnector(1:string name, 2:bool ifNotExists, 3:bool checkReferences) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)

Review Comment:
   The `drop_dataconnector` is introduced after `4.0.0-alpha-1`, if we do not use it heavily, then I think we can just remove 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.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r972558575


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2602,21 +2665,29 @@ service ThriftHiveMetastore extends fb303.FacebookService
   Partition append_partition_with_environment_context(1:string db_name, 2:string tbl_name,
       3:list<string> part_vals, 4:EnvironmentContext environment_context)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
+  Partition append_partition_req(1:AppendPartitionsRequest appendPartitionsReq)
+                       throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   Partition append_partition_by_name(1:string db_name, 2:string tbl_name, 3:string part_name)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   Partition append_partition_by_name_with_environment_context(1:string db_name, 2:string tbl_name,
       3:string part_name, 4:EnvironmentContext environment_context)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
+  Partition append_partition_by_name_req(1:AppendPartitionRequest appendPartitionRequest)
+        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   bool drop_partition(1:string db_name, 2:string tbl_name, 3:list<string> part_vals, 4:bool deleteData)
                        throws(1:NoSuchObjectException o1, 2:MetaException o2)
   bool drop_partition_with_environment_context(1:string db_name, 2:string tbl_name,
       3:list<string> part_vals, 4:bool deleteData, 5:EnvironmentContext environment_context)
                        throws(1:NoSuchObjectException o1, 2:MetaException o2)
+  bool drop_partition_req(1:DropPartitionRequest dropPartitionReq)
+                       throws(1:NoSuchObjectException o1, 2:MetaException o2)
   bool drop_partition_by_name(1:string db_name, 2:string tbl_name, 3:string part_name, 4:bool deleteData)
                        throws(1:NoSuchObjectException o1, 2:MetaException o2)
   bool drop_partition_by_name_with_environment_context(1:string db_name, 2:string tbl_name,
       3:string part_name, 4:bool deleteData, 5:EnvironmentContext environment_context)
                        throws(1:NoSuchObjectException o1, 2:MetaException o2)
+  bool drop_partition_by_name_req(1:DropPartitionRequest dropPartitionReq)

Review Comment:
   the method takes the same parameter with drop_partition_req



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1024839579


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1363,18 +1363,66 @@ private void create_database_core(RawStore ms, final Database db)
   }
 
   @Override
+  @Deprecated
   public void create_database(final Database db)
       throws AlreadyExistsException, InvalidObjectException, MetaException {
-    startFunction("create_database", ": " + db.toString());
+    CreateDatabaseRequest req = new CreateDatabaseRequest();
+    req.setDatabaseName(db.getName());
+    if (db.isSetDescription()) {
+      req.setDescription(db.getDescription());
+    }
+    if (db.isSetLocationUri()) {
+      req.setLocationUri(db.getLocationUri());
+    }
+    if (db.isSetParameters()) {
+      req.setParameters(db.getParameters());
+    }
+    if (db.isSetPrivileges()) {
+      req.setPrivileges(db.getPrivileges());
+    }
+    if (db.isSetOwnerName()) {
+      req.setOwnerName(db.getOwnerName());
+    }
+    if (db.isSetOwnerType()) {
+      req.setOwnerType(db.getOwnerType());
+    }
+    if (db.isSetCatalogName()) {
+      req.setCatalogName(db.getCatalogName());
+    }
+    if (db.isSetCreateTime()) {
+      req.setCreateTime(db.getCreateTime());
+    }
+    if (db.isSetManagedLocationUri()) {
+      req.setManagedLocationUri(db.getManagedLocationUri());
+    }
+    if (db.isSetType()) {
+      req.setType(db.getType());
+    }
+    if (db.isSetConnector_name()) {
+      req.setDataConnectorName(db.getConnector_name());
+    }
+    if (db.isSetRemote_dbname()) {
+      req.setRemote_dbname(db.getRemote_dbname());
+    }
+    create_database_req(req);

Review Comment:
   Maybe it better to construct the database from `CreateDatabaseRequest` then call `create_database(Database)`. From my point of view, in `create_database_req(req)`, we must retrieve the attributes again from `req`, it's kinds of duplicate, and `CreateDatabaseRequest` is meant to convey messages over wire, if we have already reached the HMS side, there is no need to create it again.
   Another benefit is that we can make the new changes less intervened with the elder implements and better for review, what do you think?
   
   



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


Re: [PR] HIVE-26537: Deprecate older APIs in the HMS thrift interface. [hive]

Posted by "aturoczy (via GitHub)" <gi...@apache.org>.
aturoczy commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1869681782

   As I see there is a Javadoc generation issue and a timeout issue. Cloud you pls check again?


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] sonarcloud[bot] commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1251814646

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3599)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [3 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [95 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=coverage&view=list) No Coverage information  
   [![No Duplication information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo-16px.png 'No Duplication information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=duplicated_lines_density&view=list) No Duplication information
   
   


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] sonarcloud[bot] commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1334593650

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3599)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [92 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=coverage&view=list) No Coverage information  
   [![No Duplication information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo-16px.png 'No Duplication information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=duplicated_lines_density&view=list) No Duplication information
   
   


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] nrg4878 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "nrg4878 (via GitHub)" <gi...@apache.org>.
nrg4878 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1095313330


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -939,6 +940,16 @@ struct DropPartitionsRequest {
   9: optional string catName
 }
 
+struct DropPartitionRequest {
+  1: optional string dbName,
+  2: optional string tblName,
+  3: optional string partName,
+  4: optional list<string> partVals,
+  5: optional bool deleteData,
+  6: optional EnvironmentContext environmentContext,
+  7: optional string catName

Review Comment:
   should the catName be the first in this struct? just looks cleaner. I know it was added much later than the others for some of the other structs. But we are adding a new one.



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2070,18 +2096,30 @@ struct CreateDatabaseRequest {
   8: optional string catalogName,
   9: optional i32 createTime,
   10: optional string managedLocationUri,
-  11: optional string type,
-  12: optional string dataConnectorName
+  11: optional DatabaseType type,
+  12: optional string dataConnectorName,
+  13: optional string remote_dbname

Review Comment:
   why do we need the remote_dbname in this? this will be provided in the parameters along with the createDB request. 
   Also, I am a bit concerned about changing type from string to DatabaseType. its cleaner but is there another benefit?



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -1696,13 +1707,28 @@ struct ExtendedTableInfo {
  4: optional list<string> requiredWriteCapabilities // capabilities required for write access
 }
 
+struct DropTableRequest {
+ 1: required string dbName,
+ 2: required string tableName,
+ 3: optional string catalogName,

Review Comment:
   same here. It could make sense for catalogName to be the first.



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2070,18 +2096,30 @@ struct CreateDatabaseRequest {
   8: optional string catalogName,
   9: optional i32 createTime,
   10: optional string managedLocationUri,
-  11: optional string type,
-  12: optional string dataConnectorName
+  11: optional DatabaseType type,
+  12: optional string dataConnectorName,
+  13: optional string remote_dbname
 }
 
 struct CreateDataConnectorRequest {
-  1: DataConnector connector
+  1: required DataConnector connector
 }
 
 struct GetDataConnectorRequest {
   1: required string connectorName
 }
 
+struct AlterDataConnectorRequest {
+  1: required string connectorName,
+  2: required DataConnector newConnector
+}
+
+struct DropDataConnectorRequest {
+  1: required string connectorName,
+  2: optional bool ifNotExists,
+  3: optional bool checkReferences

Review Comment:
   Do we need this? This causes HMS to look at all the DBs that reference this connector name right? instead should we just define what should happen? We could allow the connector be dropped even with references (under the assumption that they want to re-create it differently, eg different properties like password, maybe secure the credentials etc) or do not allow dropping of the connector if there are references.
   
   If we have this, I think clients can define the behavior or keep changing it on the long run, but I am worried about having to scan all DBs.



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2236,9 +2282,9 @@ struct GetSchemaResponse {
 
 struct GetPartitionRequest {
    1: optional string catName,
-   2: required string dbName,
-   3: required string tblName,
-   4: required list<string> partVals,
+   2: optional string dbName,

Review Comment:
   why are these fields changed to optional? Shouldn't this request always be scoped to a table?



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2249,8 +2295,8 @@ struct GetPartitionResponse {
 
 struct PartitionsRequest { // Not using Get prefix as that name is already used for a different method
    1: optional string catName,
-   2: required string dbName,
-   3: required string tblName,
+   2: optional string dbName,

Review Comment:
   same comment as above



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2262,8 +2308,8 @@ struct PartitionsResponse { // Not using Get prefix as that name is already used
 
 struct GetPartitionNamesPsRequest {
    1: optional string catName,
-   2: required string dbName,
-   3: required string tblName,
+   2: optional string dbName,

Review Comment:
   ditto



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2451,19 +2497,24 @@ service ThriftHiveMetastore extends fb303.FacebookService
   void drop_catalog(1: DropCatalogRequest catName) throws (1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
 
   void create_database(1:Database database) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)

Review Comment:
   shouldn't this be deleted?



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2451,19 +2497,24 @@ service ThriftHiveMetastore extends fb303.FacebookService
   void drop_catalog(1: DropCatalogRequest catName) throws (1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
 
   void create_database(1:Database database) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
+  void create_database_req(1:CreateDatabaseRequest createDatabaseRequest) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
   Database get_database(1:string name) throws(1:NoSuchObjectException o1, 2:MetaException o2)
   Database get_database_req(1:GetDatabaseRequest request) throws(1:NoSuchObjectException o1, 2:MetaException o2)
   void drop_database(1:string name, 2:bool deleteData, 3:bool cascade) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
   void drop_database_req(1:DropDatabaseRequest req) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
   list<string> get_databases(1:string pattern) throws(1:MetaException o1)
   list<string> get_all_databases() throws(1:MetaException o1)
   void alter_database(1:string dbname, 2:Database db) throws(1:MetaException o1, 2:NoSuchObjectException o2)
+  void alter_database_req(1:AlterDatabaseRequest alterDbReq) throws(1:MetaException o1, 2:NoSuchObjectException o2)
 
   void create_dataconnector(1:DataConnector connector) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)

Review Comment:
   shouldn't this be deleted?



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2451,19 +2497,24 @@ service ThriftHiveMetastore extends fb303.FacebookService
   void drop_catalog(1: DropCatalogRequest catName) throws (1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
 
   void create_database(1:Database database) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
+  void create_database_req(1:CreateDatabaseRequest createDatabaseRequest) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
   Database get_database(1:string name) throws(1:NoSuchObjectException o1, 2:MetaException o2)
   Database get_database_req(1:GetDatabaseRequest request) throws(1:NoSuchObjectException o1, 2:MetaException o2)
   void drop_database(1:string name, 2:bool deleteData, 3:bool cascade) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
   void drop_database_req(1:DropDatabaseRequest req) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
   list<string> get_databases(1:string pattern) throws(1:MetaException o1)
   list<string> get_all_databases() throws(1:MetaException o1)
   void alter_database(1:string dbname, 2:Database db) throws(1:MetaException o1, 2:NoSuchObjectException o2)

Review Comment:
   shouldn't this be deleted?



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2451,19 +2497,24 @@ service ThriftHiveMetastore extends fb303.FacebookService
   void drop_catalog(1: DropCatalogRequest catName) throws (1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
 
   void create_database(1:Database database) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
+  void create_database_req(1:CreateDatabaseRequest createDatabaseRequest) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
   Database get_database(1:string name) throws(1:NoSuchObjectException o1, 2:MetaException o2)
   Database get_database_req(1:GetDatabaseRequest request) throws(1:NoSuchObjectException o1, 2:MetaException o2)
   void drop_database(1:string name, 2:bool deleteData, 3:bool cascade) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
   void drop_database_req(1:DropDatabaseRequest req) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
   list<string> get_databases(1:string pattern) throws(1:MetaException o1)
   list<string> get_all_databases() throws(1:MetaException o1)
   void alter_database(1:string dbname, 2:Database db) throws(1:MetaException o1, 2:NoSuchObjectException o2)
+  void alter_database_req(1:AlterDatabaseRequest alterDbReq) throws(1:MetaException o1, 2:NoSuchObjectException o2)
 
   void create_dataconnector(1:DataConnector connector) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
+  void create_dataconnector_req(1:CreateDataConnectorRequest connectorReq) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
   DataConnector get_dataconnector_req(1:GetDataConnectorRequest request) throws(1:NoSuchObjectException o1, 2:MetaException o2)
-  void drop_dataconnector(1:string name, bool ifNotExists, bool checkReferences) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
+  void drop_dataconnector(1:string name, 2:bool ifNotExists, 3:bool checkReferences) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
+  void drop_dataconnector_req(1:DropDataConnectorRequest dropDcReq) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
   list<string> get_dataconnectors() throws(1:MetaException o1)
   void alter_dataconnector(1:string name, 2:DataConnector connector) throws(1:MetaException o1, 2:NoSuchObjectException o2)

Review Comment:
   shouldnt this be deleted?



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2451,19 +2497,24 @@ service ThriftHiveMetastore extends fb303.FacebookService
   void drop_catalog(1: DropCatalogRequest catName) throws (1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
 
   void create_database(1:Database database) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
+  void create_database_req(1:CreateDatabaseRequest createDatabaseRequest) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
   Database get_database(1:string name) throws(1:NoSuchObjectException o1, 2:MetaException o2)
   Database get_database_req(1:GetDatabaseRequest request) throws(1:NoSuchObjectException o1, 2:MetaException o2)
   void drop_database(1:string name, 2:bool deleteData, 3:bool cascade) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
   void drop_database_req(1:DropDatabaseRequest req) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
   list<string> get_databases(1:string pattern) throws(1:MetaException o1)
   list<string> get_all_databases() throws(1:MetaException o1)
   void alter_database(1:string dbname, 2:Database db) throws(1:MetaException o1, 2:NoSuchObjectException o2)
+  void alter_database_req(1:AlterDatabaseRequest alterDbReq) throws(1:MetaException o1, 2:NoSuchObjectException o2)
 
   void create_dataconnector(1:DataConnector connector) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
+  void create_dataconnector_req(1:CreateDataConnectorRequest connectorReq) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
   DataConnector get_dataconnector_req(1:GetDataConnectorRequest request) throws(1:NoSuchObjectException o1, 2:MetaException o2)
-  void drop_dataconnector(1:string name, bool ifNotExists, bool checkReferences) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
+  void drop_dataconnector(1:string name, 2:bool ifNotExists, 3:bool checkReferences) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)

Review Comment:
   shouldnt this be deleted?



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2533,6 +2584,8 @@ service ThriftHiveMetastore extends fb303.FacebookService
   void drop_table_with_environment_context(1:string dbname, 2:string name, 3:bool deleteData,

Review Comment:
   shouldnt this be deleted?



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2533,6 +2584,8 @@ service ThriftHiveMetastore extends fb303.FacebookService
   void drop_table_with_environment_context(1:string dbname, 2:string name, 3:bool deleteData,
       4:EnvironmentContext environment_context)
                        throws(1:NoSuchObjectException o1, 2:MetaException o3)
+  void drop_table_req(1:DropTableRequest dropTableReq)
+        throws(1:NoSuchObjectException o1, 2:MetaException o3)
   void truncate_table(1:string dbName, 2:string tableName, 3:list<string> partNames)

Review Comment:
   should we convert this to a request based as well?



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2627,21 +2682,29 @@ service ThriftHiveMetastore extends fb303.FacebookService
   Partition append_partition_with_environment_context(1:string db_name, 2:string tbl_name,
       3:list<string> part_vals, 4:EnvironmentContext environment_context)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
+  Partition append_partition_req(1:AppendPartitionsRequest appendPartitionsReq)
+                       throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   Partition append_partition_by_name(1:string db_name, 2:string tbl_name, 3:string part_name)

Review Comment:
   shouldnt this be deleted?



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2627,21 +2682,29 @@ service ThriftHiveMetastore extends fb303.FacebookService
   Partition append_partition_with_environment_context(1:string db_name, 2:string tbl_name,
       3:list<string> part_vals, 4:EnvironmentContext environment_context)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
+  Partition append_partition_req(1:AppendPartitionsRequest appendPartitionsReq)
+                       throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   Partition append_partition_by_name(1:string db_name, 2:string tbl_name, 3:string part_name)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   Partition append_partition_by_name_with_environment_context(1:string db_name, 2:string tbl_name,

Review Comment:
   shouldnt this be deleted?



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2627,21 +2682,29 @@ service ThriftHiveMetastore extends fb303.FacebookService
   Partition append_partition_with_environment_context(1:string db_name, 2:string tbl_name,
       3:list<string> part_vals, 4:EnvironmentContext environment_context)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
+  Partition append_partition_req(1:AppendPartitionsRequest appendPartitionsReq)
+                       throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   Partition append_partition_by_name(1:string db_name, 2:string tbl_name, 3:string part_name)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   Partition append_partition_by_name_with_environment_context(1:string db_name, 2:string tbl_name,
       3:string part_name, 4:EnvironmentContext environment_context)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
+  Partition append_partition_by_name_req(1:AppendPartitionsRequest appendPartitionRequest)
+        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   bool drop_partition(1:string db_name, 2:string tbl_name, 3:list<string> part_vals, 4:bool deleteData)
                        throws(1:NoSuchObjectException o1, 2:MetaException o2)
   bool drop_partition_with_environment_context(1:string db_name, 2:string tbl_name,

Review Comment:
   shouldnt this be deleted?



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2627,21 +2682,29 @@ service ThriftHiveMetastore extends fb303.FacebookService
   Partition append_partition_with_environment_context(1:string db_name, 2:string tbl_name,
       3:list<string> part_vals, 4:EnvironmentContext environment_context)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
+  Partition append_partition_req(1:AppendPartitionsRequest appendPartitionsReq)
+                       throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   Partition append_partition_by_name(1:string db_name, 2:string tbl_name, 3:string part_name)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   Partition append_partition_by_name_with_environment_context(1:string db_name, 2:string tbl_name,
       3:string part_name, 4:EnvironmentContext environment_context)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
+  Partition append_partition_by_name_req(1:AppendPartitionsRequest appendPartitionRequest)
+        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   bool drop_partition(1:string db_name, 2:string tbl_name, 3:list<string> part_vals, 4:bool deleteData)
                        throws(1:NoSuchObjectException o1, 2:MetaException o2)
   bool drop_partition_with_environment_context(1:string db_name, 2:string tbl_name,
       3:list<string> part_vals, 4:bool deleteData, 5:EnvironmentContext environment_context)
                        throws(1:NoSuchObjectException o1, 2:MetaException o2)
+  bool drop_partition_req(1:DropPartitionRequest dropPartitionReq)
+                       throws(1:NoSuchObjectException o1, 2:MetaException o2)
   bool drop_partition_by_name(1:string db_name, 2:string tbl_name, 3:string part_name, 4:bool deleteData)

Review Comment:
   shouldnt this be deleted?



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2679,7 +2742,8 @@ PartitionsResponse get_partitions_req(1:PartitionsRequest req)
 
   list<string> get_partition_names(1:string db_name, 2:string tbl_name, 3:i16 max_parts=-1)

Review Comment:
   shouldnt this be deleted?



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2276,8 +2322,8 @@ struct GetPartitionNamesPsResponse {
 
 struct GetPartitionsPsWithAuthRequest {
    1: optional string catName,
-   2: required string dbName,
-   3: required string tblName,
+   2: optional string dbName,

Review Comment:
   ditto



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2060,7 +2086,7 @@ struct CreateTableRequest {
 }
 
 struct CreateDatabaseRequest {
-  1: required string databaseName,
+  1: optional string databaseName,

Review Comment:
   not sure I understand this. Should the dbName for a createDB API be required?



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2627,21 +2682,29 @@ service ThriftHiveMetastore extends fb303.FacebookService
   Partition append_partition_with_environment_context(1:string db_name, 2:string tbl_name,
       3:list<string> part_vals, 4:EnvironmentContext environment_context)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
+  Partition append_partition_req(1:AppendPartitionsRequest appendPartitionsReq)
+                       throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   Partition append_partition_by_name(1:string db_name, 2:string tbl_name, 3:string part_name)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   Partition append_partition_by_name_with_environment_context(1:string db_name, 2:string tbl_name,
       3:string part_name, 4:EnvironmentContext environment_context)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
+  Partition append_partition_by_name_req(1:AppendPartitionsRequest appendPartitionRequest)
+        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   bool drop_partition(1:string db_name, 2:string tbl_name, 3:list<string> part_vals, 4:bool deleteData)

Review Comment:
   shouldnt this be deleted?



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2627,21 +2682,29 @@ service ThriftHiveMetastore extends fb303.FacebookService
   Partition append_partition_with_environment_context(1:string db_name, 2:string tbl_name,
       3:list<string> part_vals, 4:EnvironmentContext environment_context)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
+  Partition append_partition_req(1:AppendPartitionsRequest appendPartitionsReq)
+                       throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   Partition append_partition_by_name(1:string db_name, 2:string tbl_name, 3:string part_name)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   Partition append_partition_by_name_with_environment_context(1:string db_name, 2:string tbl_name,
       3:string part_name, 4:EnvironmentContext environment_context)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
+  Partition append_partition_by_name_req(1:AppendPartitionsRequest appendPartitionRequest)
+        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   bool drop_partition(1:string db_name, 2:string tbl_name, 3:list<string> part_vals, 4:bool deleteData)
                        throws(1:NoSuchObjectException o1, 2:MetaException o2)
   bool drop_partition_with_environment_context(1:string db_name, 2:string tbl_name,
       3:list<string> part_vals, 4:bool deleteData, 5:EnvironmentContext environment_context)
                        throws(1:NoSuchObjectException o1, 2:MetaException o2)
+  bool drop_partition_req(1:DropPartitionRequest dropPartitionReq)
+                       throws(1:NoSuchObjectException o1, 2:MetaException o2)
   bool drop_partition_by_name(1:string db_name, 2:string tbl_name, 3:string part_name, 4:bool deleteData)
                        throws(1:NoSuchObjectException o1, 2:MetaException o2)
   bool drop_partition_by_name_with_environment_context(1:string db_name, 2:string tbl_name,

Review Comment:
   shouldnt this be deleted?



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2616,6 +2669,8 @@ service ThriftHiveMetastore extends fb303.FacebookService
       2:EnvironmentContext environment_context)
       throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2,
       3:MetaException o3)
+  Partition add_partition_req(1:AddPartitionsRequest addPartitionsReq)
+        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   i32 add_partitions(1:list<Partition> new_parts)

Review Comment:
   shouldnt this be deleted?



##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2627,21 +2682,29 @@ service ThriftHiveMetastore extends fb303.FacebookService
   Partition append_partition_with_environment_context(1:string db_name, 2:string tbl_name,

Review Comment:
   shouldnt this be deleted?



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1154460254


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2451,19 +2497,24 @@ service ThriftHiveMetastore extends fb303.FacebookService
   void drop_catalog(1: DropCatalogRequest catName) throws (1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
 
   void create_database(1:Database database) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)

Review Comment:
   I thought we are planning to keep the backward compatibility with the old client and a new server for at least one release. So that's the reason why I'm keeping the deprecated APIs. Should I just delete 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.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1154462271


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2533,6 +2584,8 @@ service ThriftHiveMetastore extends fb303.FacebookService
   void drop_table_with_environment_context(1:string dbname, 2:string name, 3:bool deleteData,
       4:EnvironmentContext environment_context)
                        throws(1:NoSuchObjectException o1, 2:MetaException o3)
+  void drop_table_req(1:DropTableRequest dropTableReq)
+        throws(1:NoSuchObjectException o1, 2:MetaException o3)
   void truncate_table(1:string dbName, 2:string tableName, 3:list<string> partNames)

Review Comment:
   Request-based API for this already exists. See L2591.



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] sonarcloud[bot] commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1496903970

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3599)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [92 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=coverage&view=list) No Coverage information  
   [![No Duplication information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo-16px.png 'No Duplication information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=duplicated_lines_density&view=list) No Duplication information
   
   


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


Re: [PR] HIVE-26537: Deprecate older APIs in the HMS thrift interface. [hive]

Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1439253202


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -5586,12 +5807,39 @@ public PartitionsResponse get_partitions_req(PartitionsRequest req)
   public List<Partition> get_partitions_with_auth(final String dbName,
       final String tblName, final short maxParts, final String userName,
       final List<String> groupNames) throws TException {
-    return get_partitions_ps_with_auth(dbName, tblName,

Review Comment:
   We don't need to restore `get_partitions_with_auth_optional_schema`, the `GetPartitionsArgs` has contains all the context for fetching the partition



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] sonarcloud[bot] commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1251960372

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3599)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [3 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [95 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=coverage&view=list) No Coverage information  
   [![No Duplication information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo-16px.png 'No Duplication information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=duplicated_lines_density&view=list) No Duplication information
   
   


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r972560425


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1363,18 +1363,62 @@ private void create_database_core(RawStore ms, final Database db)
   }
 
   @Override
+  @Deprecated
   public void create_database(final Database db)
       throws AlreadyExistsException, InvalidObjectException, MetaException {
-    startFunction("create_database", ": " + db.toString());
+    CreateDatabaseRequest req = new CreateDatabaseRequest(db.getName());
+    if (db.isSetDescription()) {
+      req.setDescription(db.getDescription());
+    }
+    if (db.isSetLocationUri()) {
+      req.setLocationUri(db.getLocationUri());
+    }
+    if (db.isSetParameters()) {
+      req.setParameters(db.getParameters());
+    }
+    if (db.isSetPrivileges()) {
+      req.setPrivileges(db.getPrivileges());
+    }
+    if (db.isSetOwnerName()) {
+      req.setOwnerName(db.getOwnerName());
+    }
+    if (db.isSetOwnerType()) {
+      req.setOwnerType(db.getOwnerType());
+    }
+    if (db.isSetCatalogName()) {
+      req.setCatalogName(db.getCatalogName());
+    }
+    if (db.isSetCreateTime()) {
+      req.setCreateTime(db.getCreateTime());
+    }
+    if (db.isSetManagedLocationUri()) {
+      req.setManagedLocationUri(db.getManagedLocationUri());
+    }
+    if (db.isSetType()) {
+      req.setType(db.getType());
+    }
+    if (db.isSetConnector_name()) {
+      req.setDataConnectorName(db.getConnector_name());
+    }
+    if (db.isSetRemote_dbname()) {
+      req.setRemote_dbname(db.getRemote_dbname());
+    }
+    create_database_req(req);
+  }
+
+  @Override
+  public void create_database_req(final CreateDatabaseRequest createDatabaseRequest)

Review Comment:
   for create_database_req, I would create the `Database` from `createDatabaseRequest`, then invokes `create_database(final Database db)`



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1037482722


##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -1274,7 +1308,46 @@ public void createDatabase(Database db)
     if (!db.isSetCatalogName()) {
       db.setCatalogName(getDefaultCatalog(conf));
     }
-    client.create_database(db);
+    CreateDatabaseRequest req = new CreateDatabaseRequest();
+    req.setDatabaseName(db.getName());
+    if (db.isSetDescription()) {
+      req.setDescription(db.getDescription());
+    }
+    if (db.isSetLocationUri()) {
+      req.setLocationUri(db.getLocationUri());
+    }
+    if (db.isSetParameters()) {
+      req.setParameters(db.getParameters());
+    }
+    if (db.isSetPrivileges()) {
+      req.setPrivileges(db.getPrivileges());
+    }
+    if (db.isSetOwnerName()) {
+      req.setOwnerName(db.getOwnerName());
+    }
+    if (db.isSetOwnerType()) {

Review Comment:
   Thrift objects don't come with Builder methods. So I don't think there is a way to implement your suggestion here.



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] sonarcloud[bot] commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1549028978

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3599)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [99 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=coverage&view=list) No Coverage information  
   [![No Duplication information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo-16px.png 'No Duplication information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=duplicated_lines_density&view=list) No Duplication information
   
   


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1154425672


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2070,18 +2096,30 @@ struct CreateDatabaseRequest {
   8: optional string catalogName,
   9: optional i32 createTime,
   10: optional string managedLocationUri,
-  11: optional string type,
-  12: optional string dataConnectorName
+  11: optional DatabaseType type,
+  12: optional string dataConnectorName,
+  13: optional string remote_dbname

Review Comment:
   I'm trying to point to create_database_req(req) instead of create_database(database), by converting the database object to a request object. As a result, I had to introduce whatever is in the database object to the CreateDatabaseRequest object. If this is not a good change, what do you suggest I should be doing?



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1211238449


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2486,19 +2532,24 @@ service ThriftHiveMetastore extends fb303.FacebookService
   void drop_catalog(1: DropCatalogRequest catName) throws (1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
 
   void create_database(1:Database database) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
+  void create_database_req(1:CreateDatabaseRequest createDatabaseRequest) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
   Database get_database(1:string name) throws(1:NoSuchObjectException o1, 2:MetaException o2)
   Database get_database_req(1:GetDatabaseRequest request) throws(1:NoSuchObjectException o1, 2:MetaException o2)
   void drop_database(1:string name, 2:bool deleteData, 3:bool cascade) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
   void drop_database_req(1:DropDatabaseRequest req) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
   list<string> get_databases(1:string pattern) throws(1:MetaException o1)
   list<string> get_all_databases() throws(1:MetaException o1)
   void alter_database(1:string dbname, 2:Database db) throws(1:MetaException o1, 2:NoSuchObjectException o2)
+  void alter_database_req(1:AlterDatabaseRequest alterDbReq) throws(1:MetaException o1, 2:NoSuchObjectException o2)
 
   void create_dataconnector(1:DataConnector connector) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
+  void create_dataconnector_req(1:CreateDataConnectorRequest connectorReq) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
   DataConnector get_dataconnector_req(1:GetDataConnectorRequest request) throws(1:NoSuchObjectException o1, 2:MetaException o2)
   void drop_dataconnector(1:string name, 2:bool ifNotExists, 3:bool checkReferences) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)

Review Comment:
   The `drop_dataconnector` is introduced after 4.0.0-alpha-1, if we do not use it heavily, then I think we can just remove 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.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1212372017


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2486,19 +2532,24 @@ service ThriftHiveMetastore extends fb303.FacebookService
   void drop_catalog(1: DropCatalogRequest catName) throws (1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
 
   void create_database(1:Database database) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
+  void create_database_req(1:CreateDatabaseRequest createDatabaseRequest) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
   Database get_database(1:string name) throws(1:NoSuchObjectException o1, 2:MetaException o2)
   Database get_database_req(1:GetDatabaseRequest request) throws(1:NoSuchObjectException o1, 2:MetaException o2)
   void drop_database(1:string name, 2:bool deleteData, 3:bool cascade) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
   void drop_database_req(1:DropDatabaseRequest req) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
   list<string> get_databases(1:string pattern) throws(1:MetaException o1)
   list<string> get_all_databases() throws(1:MetaException o1)
   void alter_database(1:string dbname, 2:Database db) throws(1:MetaException o1, 2:NoSuchObjectException o2)
+  void alter_database_req(1:AlterDatabaseRequest alterDbReq) throws(1:MetaException o1, 2:NoSuchObjectException o2)
 
   void create_dataconnector(1:DataConnector connector) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
+  void create_dataconnector_req(1:CreateDataConnectorRequest connectorReq) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)
   DataConnector get_dataconnector_req(1:GetDataConnectorRequest request) throws(1:NoSuchObjectException o1, 2:MetaException o2)
   void drop_dataconnector(1:string name, 2:bool ifNotExists, 3:bool checkReferences) throws(1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)

Review Comment:
   I'm going to bring up dev community for feedback on whether we should remove the deprecated APIs or keep them for the 4.0 release. If the community is inclined towards the latter then I'll just remove data connector APIs alone.



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


Re: [PR] HIVE-26537: Deprecate older APIs in the HMS thrift interface. [hive]

Posted by "nrg4878 (via GitHub)" <gi...@apache.org>.
nrg4878 commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1785276310

   Re-opening the PR


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


Re: [PR] HIVE-26537: Deprecate older APIs in the HMS thrift interface. [hive]

Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1439240262


##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -1345,7 +1383,48 @@ public void createDatabase(Database db)
     if (!db.isSetCatalogName()) {
       db.setCatalogName(getDefaultCatalog(conf));
     }
-    client.create_database(db);
+    if (db.getName() == null) {
+      throw new MetaException("DbName cannot be null");
+    }
+    CreateDatabaseRequest req = new CreateDatabaseRequest(db.getName());
+    if (db.isSetDescription()) {
+      req.setDescription(db.getDescription());
+    }
+    if (db.isSetLocationUri()) {
+      req.setLocationUri(db.getLocationUri());
+    }
+    if (db.isSetParameters()) {
+      req.setParameters(db.getParameters());
+    }
+    if (db.isSetPrivileges()) {
+      req.setPrivileges(db.getPrivileges());
+    }
+    if (db.isSetOwnerName()) {
+      req.setOwnerName(db.getOwnerName());
+    }
+    if (db.isSetOwnerType()) {
+      req.setOwnerType(db.getOwnerType());
+    }
+    req.setCatalogName(db.getCatalogName());
+    if (db.isSetCreateTime()) {
+      req.setCreateTime(db.getCreateTime());
+    }
+    if (db.isSetManagedLocationUri()) {
+      req.setManagedLocationUri(db.getManagedLocationUri());
+    }
+    if (db.isSetType()) {
+      req.setType(db.getType());
+    }
+    if (db.isSetConnector_name()) {
+      req.setDataConnectorName(db.getConnector_name());
+    }
+    if (db.isSetRemote_dbname()) {
+      req.setRemote_dbname(db.getRemote_dbname());
+    }
+    client.create_database_req(req);
+    //location and manged location might be set/changed.
+    db.setLocationUri(req.getLocationUri());

Review Comment:
   why do we need to restore the location from req, is it for testing?



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] sonarcloud[bot] commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1249660389

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3599)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [3 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [87 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=coverage&view=list) No Coverage information  
   [![No Duplication information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo-16px.png 'No Duplication information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=duplicated_lines_density&view=list) No Duplication information
   
   


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] sonarcloud[bot] commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1249993691

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3599)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [3 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [92 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=coverage&view=list) No Coverage information  
   [![No Duplication information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo-16px.png 'No Duplication information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=duplicated_lines_density&view=list) No Duplication information
   
   


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r972556764


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2121,6 +2159,22 @@ struct AlterPartitionsRequest {
   7: optional string validWriteIdList
 }
 
+struct AppendPartitionRequest {
+  1: optional string catalogName,
+  2: required string dbName,
+  3: required string tableName,
+  4: required string partName,
+  5: optional EnvironmentContext environmentContext
+}
+
+struct AppendPartitionsRequest {

Review Comment:
   can this struct be merged with AppendPartitionRequest? most of fields are the same



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1037474456


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2602,21 +2665,29 @@ service ThriftHiveMetastore extends fb303.FacebookService
   Partition append_partition_with_environment_context(1:string db_name, 2:string tbl_name,
       3:list<string> part_vals, 4:EnvironmentContext environment_context)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
+  Partition append_partition_req(1:AppendPartitionsRequest appendPartitionsReq)
+                       throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   Partition append_partition_by_name(1:string db_name, 2:string tbl_name, 3:string part_name)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   Partition append_partition_by_name_with_environment_context(1:string db_name, 2:string tbl_name,
       3:string part_name, 4:EnvironmentContext environment_context)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
+  Partition append_partition_by_name_req(1:AppendPartitionRequest appendPartitionRequest)

Review Comment:
   Arguments are same but if you look closely, these two methods do different functionalities calling different methods in 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.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1154441856


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2236,9 +2282,9 @@ struct GetSchemaResponse {
 
 struct GetPartitionRequest {
    1: optional string catName,
-   2: required string dbName,
-   3: required string tblName,
-   4: required list<string> partVals,
+   2: optional string dbName,

Review Comment:
   2 reasons. 1) ease of use in HMSClient 2) Tests are sending null values in these required values and as a result, the request won't even land on HMS.
   But both reasons are defeating the whole objective as these 3 values are always required. I'll handle the null values in the HMS client and keep these values as required.



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] sonarcloud[bot] commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1495035163

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3599)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [91 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=coverage&view=list) No Coverage information  
   [![No Duplication information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo-16px.png 'No Duplication information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=duplicated_lines_density&view=list) No Duplication information
   
   


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


Re: [PR] HIVE-26537: Deprecate older APIs in the HMS thrift interface. [hive]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.
URL: https://github.com/apache/hive/pull/3599


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


Re: [PR] HIVE-26537: Deprecate older APIs in the HMS thrift interface. [hive]

Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1439231267


##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -1100,10 +1100,17 @@ public Partition add_partition(Partition new_part) throws TException {
 
   public Partition add_partition(Partition new_part, EnvironmentContext envContext)
       throws TException {
-    if (new_part != null && !new_part.isSetCatName()) {
+    if (new_part == null || new_part.getDbName() == null) {
+      throw new MetaException("Partition/DB name cannot be null.");
+    }
+    if (!new_part.isSetCatName()) {
       new_part.setCatName(getDefaultCatalog(conf));
     }
-    Partition p = client.add_partition_with_environment_context(new_part, envContext);
+    AddPartitionsRequest addPartitionsReq = new AddPartitionsRequest(new_part.getDbName(), new_part.getTableName(),
+            new ArrayList<>(Arrays.asList(new_part)), false);
+    addPartitionsReq.setCatName(new_part.getCatName());
+    addPartitionsReq.setEnvironmentContext(envContext);
+    Partition p = client.add_partition_req(addPartitionsReq);

Review Comment:
   Can we just call `client.add_partitions_req` instead of introducing a new similar method?



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


Re: [PR] HIVE-26537: Deprecate older APIs in the HMS thrift interface. [hive]

Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1439246896


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -3214,15 +3290,51 @@ private List<Path> dropPartitionsAndGetLocations(RawStore ms, String catName, St
   }
 
   @Override
+  @Deprecated
   public void drop_table(final String dbname, final String name, final boolean deleteData)
       throws NoSuchObjectException, MetaException {
-    drop_table_with_environment_context(dbname, name, deleteData, null);
+    startFunction("drop_table", ": " + dbname + ":" + name);
+    boolean success = false;
+    Exception ex = null;
+    try {
+    String[] parsedDbName = parseDbName(dbname, conf);

Review Comment:
   can we just call `drop_table_core` in these old deprecated methods or just leave as it is?



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1199529336


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2451,19 +2497,24 @@ service ThriftHiveMetastore extends fb303.FacebookService
   void drop_catalog(1: DropCatalogRequest catName) throws (1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
 
   void create_database(1:Database database) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)

Review Comment:
   Will we keep only one `create_database_req` method for creating `database` stuff in the future release?



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1154436457


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2070,18 +2096,30 @@ struct CreateDatabaseRequest {
   8: optional string catalogName,
   9: optional i32 createTime,
   10: optional string managedLocationUri,
-  11: optional string type,
-  12: optional string dataConnectorName
+  11: optional DatabaseType type,
+  12: optional string dataConnectorName,
+  13: optional string remote_dbname
 }
 
 struct CreateDataConnectorRequest {
-  1: DataConnector connector
+  1: required DataConnector connector
 }
 
 struct GetDataConnectorRequest {
   1: required string connectorName
 }
 
+struct AlterDataConnectorRequest {
+  1: required string connectorName,
+  2: required DataConnector newConnector
+}
+
+struct DropDataConnectorRequest {
+  1: required string connectorName,
+  2: optional bool ifNotExists,
+  3: optional bool checkReferences

Review Comment:
   Here the change is about creating a request-based API for the already existing API (which takes in all the above 3 parameters).
   Currently, we don't use this flag. Yeah, your concern is genuine in the future when we implement this flag. 



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] sonarcloud[bot] commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1497477892

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3599)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [92 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=coverage&view=list) No Coverage information  
   [![No Duplication information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo-16px.png 'No Duplication information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=duplicated_lines_density&view=list) No Duplication information
   
   


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1229356933


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2632,9 +2629,6 @@ service ThriftHiveMetastore extends fb303.FacebookService
                        throws (1: MetaException o1)
   list<string> get_all_tables(1: string db_name) throws (1: MetaException o1)
 
-  Table get_table(1:string dbname, 2:string tbl_name)

Review Comment:
   The `get_table` and `get_table_objects_by_name` is marked as deprecated in HIVE-15062.



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r972559262


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1363,18 +1363,62 @@ private void create_database_core(RawStore ms, final Database db)
   }
 
   @Override
+  @Deprecated
   public void create_database(final Database db)
       throws AlreadyExistsException, InvalidObjectException, MetaException {
-    startFunction("create_database", ": " + db.toString());
+    CreateDatabaseRequest req = new CreateDatabaseRequest(db.getName());
+    if (db.isSetDescription()) {

Review Comment:
   if the description is null, it's safe to call req.setDescription(null), we need not an extra db.isSetDescription()?



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


Re: [PR] HIVE-26537: Deprecate older APIs in the HMS thrift interface. [hive]

Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1439246896


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -3214,15 +3290,51 @@ private List<Path> dropPartitionsAndGetLocations(RawStore ms, String catName, St
   }
 
   @Override
+  @Deprecated
   public void drop_table(final String dbname, final String name, final boolean deleteData)
       throws NoSuchObjectException, MetaException {
-    drop_table_with_environment_context(dbname, name, deleteData, null);
+    startFunction("drop_table", ": " + dbname + ":" + name);
+    boolean success = false;
+    Exception ex = null;
+    try {
+    String[] parsedDbName = parseDbName(dbname, conf);

Review Comment:
   can we just call `drop_table_core` in these old deprecated methods or just leave as it is?
   Similar to append/drop/get partitions



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


Re: [PR] HIVE-26537: Deprecate older APIs in the HMS thrift interface. [hive]

Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1873759488

   The failed test `TestDbNotificationListener.createDatabase`seems to be related to the change, cloud you please check that?


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


Re: [PR] HIVE-26537: Deprecate older APIs in the HMS thrift interface. [hive]

Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1439239756


##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -1822,8 +1907,13 @@ public boolean dropPartition(String dbName, String tableName, String partName, b
   @Override
   public boolean dropPartition(String catName, String db_name, String tbl_name, String name,
                                boolean deleteData) throws TException {
-    return client.drop_partition_by_name_with_environment_context(prependCatalogToDbName(
-        catName, db_name, conf), tbl_name, name, deleteData, null);
+    DropPartitionRequest dropPartitionReq = new DropPartitionRequest();
+    dropPartitionReq.setDbName(db_name);
+    dropPartitionReq.setTblName(tbl_name);
+    dropPartitionReq.setCatName(catName);
+    dropPartitionReq.setPartName(name);
+    dropPartitionReq.setDeleteData(deleteData);
+    return client.drop_partition_by_name_req(dropPartitionReq);

Review Comment:
   for the `drop_partition_by_name_req` and `drop_partition_req`, I would prefer to add only `drop_partition_req` for both purposes, Let HMSHandler check if the client sets the partition name or partition vals, the same to `append_partition_by_name_req` and `append_partition_req`



##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -1822,8 +1907,13 @@ public boolean dropPartition(String dbName, String tableName, String partName, b
   @Override
   public boolean dropPartition(String catName, String db_name, String tbl_name, String name,
                                boolean deleteData) throws TException {
-    return client.drop_partition_by_name_with_environment_context(prependCatalogToDbName(
-        catName, db_name, conf), tbl_name, name, deleteData, null);
+    DropPartitionRequest dropPartitionReq = new DropPartitionRequest();
+    dropPartitionReq.setDbName(db_name);
+    dropPartitionReq.setTblName(tbl_name);
+    dropPartitionReq.setCatName(catName);
+    dropPartitionReq.setPartName(name);
+    dropPartitionReq.setDeleteData(deleteData);
+    return client.drop_partition_by_name_req(dropPartitionReq);

Review Comment:
   for the `drop_partition_by_name_req` and `drop_partition_req`, I would prefer to add only `drop_partition_req` for both purposes, let HMSHandler check if the client sets the partition name or partition vals, the same to `append_partition_by_name_req` and `append_partition_req`



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


Re: [PR] HIVE-26537: Deprecate older APIs in the HMS thrift interface. [hive]

Posted by "deniskuzZ (via GitHub)" <gi...@apache.org>.
deniskuzZ commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1858807021

   @nrg4878, @saihemanth-cloudera  should this be part of 4.0 release?


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


Re: [PR] HIVE-26537: Deprecate older APIs in the HMS thrift interface. [hive]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1974952552

   This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
   Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1024830177


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2602,21 +2665,29 @@ service ThriftHiveMetastore extends fb303.FacebookService
   Partition append_partition_with_environment_context(1:string db_name, 2:string tbl_name,
       3:list<string> part_vals, 4:EnvironmentContext environment_context)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
+  Partition append_partition_req(1:AppendPartitionsRequest appendPartitionsReq)
+                       throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   Partition append_partition_by_name(1:string db_name, 2:string tbl_name, 3:string part_name)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
   Partition append_partition_by_name_with_environment_context(1:string db_name, 2:string tbl_name,
       3:string part_name, 4:EnvironmentContext environment_context)
                        throws (1:InvalidObjectException o1, 2:AlreadyExistsException o2, 3:MetaException o3)
+  Partition append_partition_by_name_req(1:AppendPartitionRequest appendPartitionRequest)

Review Comment:
   this method takes the same parameter and throws the same exceptions as the `append_partition_req` , so I think we can just leave only one of 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.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1037533650


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1363,18 +1363,66 @@ private void create_database_core(RawStore ms, final Database db)
   }
 
   @Override
+  @Deprecated
   public void create_database(final Database db)
       throws AlreadyExistsException, InvalidObjectException, MetaException {
-    startFunction("create_database", ": " + db.toString());
+    CreateDatabaseRequest req = new CreateDatabaseRequest();
+    req.setDatabaseName(db.getName());
+    if (db.isSetDescription()) {
+      req.setDescription(db.getDescription());
+    }
+    if (db.isSetLocationUri()) {
+      req.setLocationUri(db.getLocationUri());
+    }
+    if (db.isSetParameters()) {
+      req.setParameters(db.getParameters());
+    }
+    if (db.isSetPrivileges()) {
+      req.setPrivileges(db.getPrivileges());
+    }
+    if (db.isSetOwnerName()) {
+      req.setOwnerName(db.getOwnerName());
+    }
+    if (db.isSetOwnerType()) {
+      req.setOwnerType(db.getOwnerType());
+    }
+    if (db.isSetCatalogName()) {
+      req.setCatalogName(db.getCatalogName());
+    }
+    if (db.isSetCreateTime()) {
+      req.setCreateTime(db.getCreateTime());
+    }
+    if (db.isSetManagedLocationUri()) {
+      req.setManagedLocationUri(db.getManagedLocationUri());
+    }
+    if (db.isSetType()) {
+      req.setType(db.getType());
+    }
+    if (db.isSetConnector_name()) {
+      req.setDataConnectorName(db.getConnector_name());
+    }
+    if (db.isSetRemote_dbname()) {
+      req.setRemote_dbname(db.getRemote_dbname());
+    }
+    create_database_req(req);

Review Comment:
   Again, `create_database(final Database db)` api is a deprecated method, we don't want to do anything in this api other than re-routing it to `create_database_req(req)` api. In the next release, we would like to remove all the deprecated APIs in the thrift class. Even though it breaks the incompatibility, moving forward we'll have a  clean code base.



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1035187132


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1363,18 +1363,62 @@ private void create_database_core(RawStore ms, final Database db)
   }
 
   @Override
+  @Deprecated
   public void create_database(final Database db)
       throws AlreadyExistsException, InvalidObjectException, MetaException {
-    startFunction("create_database", ": " + db.toString());
+    CreateDatabaseRequest req = new CreateDatabaseRequest(db.getName());
+    if (db.isSetDescription()) {
+      req.setDescription(db.getDescription());
+    }
+    if (db.isSetLocationUri()) {
+      req.setLocationUri(db.getLocationUri());
+    }
+    if (db.isSetParameters()) {
+      req.setParameters(db.getParameters());
+    }
+    if (db.isSetPrivileges()) {
+      req.setPrivileges(db.getPrivileges());
+    }
+    if (db.isSetOwnerName()) {
+      req.setOwnerName(db.getOwnerName());
+    }
+    if (db.isSetOwnerType()) {
+      req.setOwnerType(db.getOwnerType());
+    }
+    if (db.isSetCatalogName()) {
+      req.setCatalogName(db.getCatalogName());
+    }
+    if (db.isSetCreateTime()) {
+      req.setCreateTime(db.getCreateTime());
+    }
+    if (db.isSetManagedLocationUri()) {
+      req.setManagedLocationUri(db.getManagedLocationUri());
+    }
+    if (db.isSetType()) {
+      req.setType(db.getType());
+    }
+    if (db.isSetConnector_name()) {
+      req.setDataConnectorName(db.getConnector_name());
+    }
+    if (db.isSetRemote_dbname()) {
+      req.setRemote_dbname(db.getRemote_dbname());
+    }
+    create_database_req(req);
+  }
+
+  @Override
+  public void create_database_req(final CreateDatabaseRequest createDatabaseRequest)

Review Comment:
   create_database(final Database db) is a deprecated class, we don't want to use it going forward. Hence I'm creating the req APIs. 



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] sonarcloud[bot] commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1248787308

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3599)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [2 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [85 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=coverage&view=list) No Coverage information  
   [![No Duplication information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo-16px.png 'No Duplication information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=duplicated_lines_density&view=list) No Duplication information
   
   


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] sonarcloud[bot] commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1253057859

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3599)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [3 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [101 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=coverage&view=list) No Coverage information  
   [![No Duplication information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo-16px.png 'No Duplication information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=duplicated_lines_density&view=list) No Duplication information
   
   


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1154391179


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -1696,13 +1707,28 @@ struct ExtendedTableInfo {
  4: optional list<string> requiredWriteCapabilities // capabilities required for write access
 }
 
+struct DropTableRequest {
+ 1: required string dbName,
+ 2: required string tableName,
+ 3: optional string catalogName,

Review Comment:
   Ack



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] sonarcloud[bot] commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1550377543

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3599)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [1 Bug](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [99 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=coverage&view=list) No Coverage information  
   [![No Duplication information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo-16px.png 'No Duplication information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=duplicated_lines_density&view=list) No Duplication information
   
   


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1210841291


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1335,18 +1335,42 @@ private void create_database_core(RawStore ms, final Database db)
   }
 
   @Override
+  @Deprecated
   public void create_database(final Database db)
       throws AlreadyExistsException, InvalidObjectException, MetaException {
-    startFunction("create_database", ": " + db.toString());
+    CreateDatabaseRequest req = new CreateDatabaseRequest(db.getName());
+    req.setDescription(db.getDescription());
+    req.setLocationUri(db.getLocationUri());
+    req.setParameters(db.getParameters());
+    req.setPrivileges(db.getPrivileges());
+    req.setOwnerName(db.getOwnerName());
+    req.setOwnerType(db.getOwnerType());
+    req.setCatalogName(db.getCatalogName());
+    req.setCreateTime(db.getCreateTime());
+    req.setManagedLocationUri(db.getManagedLocationUri());
+    req.setType(db.getType());
+    req.setDataConnectorName(db.getConnector_name());
+    req.setRemote_dbname(db.getRemote_dbname());
+
+    create_database_req(req);
+    //location and manged location might be set/changed.
+    db.setLocationUri(req.getLocationUri());

Review Comment:
   I actually thought that tests are using the `db` to assert on the location but looks like none of the tests are doing it. My only concern is, the old client, may be expecting a modified location that will no more be compatible with this change. So, if we are not removing the old/deprecated APIs in this version then it is safe to keep this or else we can remove it otherwise.



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1199532157


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1335,18 +1335,42 @@ private void create_database_core(RawStore ms, final Database db)
   }
 
   @Override
+  @Deprecated
   public void create_database(final Database db)
       throws AlreadyExistsException, InvalidObjectException, MetaException {
-    startFunction("create_database", ": " + db.toString());
+    CreateDatabaseRequest req = new CreateDatabaseRequest(db.getName());
+    req.setDescription(db.getDescription());
+    req.setLocationUri(db.getLocationUri());
+    req.setParameters(db.getParameters());
+    req.setPrivileges(db.getPrivileges());
+    req.setOwnerName(db.getOwnerName());
+    req.setOwnerType(db.getOwnerType());
+    req.setCatalogName(db.getCatalogName());
+    req.setCreateTime(db.getCreateTime());
+    req.setManagedLocationUri(db.getManagedLocationUri());
+    req.setType(db.getType());
+    req.setDataConnectorName(db.getConnector_name());
+    req.setRemote_dbname(db.getRemote_dbname());
+
+    create_database_req(req);
+    //location and manged location might be set/changed.
+    db.setLocationUri(req.getLocationUri());

Review Comment:
   Why do we change the `db` after we flush the changes, is there somewhere we must use the `db` afterwards?



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1199529336


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2451,19 +2497,24 @@ service ThriftHiveMetastore extends fb303.FacebookService
   void drop_catalog(1: DropCatalogRequest catName) throws (1:NoSuchObjectException o1, 2:InvalidOperationException o2, 3:MetaException o3)
 
   void create_database(1:Database database) throws(1:AlreadyExistsException o1, 2:InvalidObjectException o2, 3:MetaException o3)

Review Comment:
   Is there only one `create_database_req` method for creating `database` stuff in the future release?



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] github-actions[bot] commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1685138258

   This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
   Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


Re: [PR] HIVE-26537: Deprecate older APIs in the HMS thrift interface. [hive]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1773953445

   This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
   Feel free to reach out on the dev@hive.apache.org list if the patch is in need of reviews.


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] sonarcloud[bot] commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
sonarcloud[bot] commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1251403532

   Kudos, SonarCloud Quality Gate passed!&nbsp; &nbsp; [![Quality Gate passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/passed-16px.png 'Quality Gate passed')](https://sonarcloud.io/dashboard?id=apache_hive&pullRequest=3599)
   
   [![Bug](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/bug-16px.png 'Bug')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [![C](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/C-16px.png 'C')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG) [3 Bugs](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=BUG)  
   [![Vulnerability](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/vulnerability-16px.png 'Vulnerability')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY) [0 Vulnerabilities](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=VULNERABILITY)  
   [![Security Hotspot](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/security_hotspot-16px.png 'Security Hotspot')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT) [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_hive&pullRequest=3599&resolved=false&types=SECURITY_HOTSPOT)  
   [![Code Smell](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/code_smell-16px.png 'Code Smell')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [![A](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/RatingBadge/A-16px.png 'A')](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL) [95 Code Smells](https://sonarcloud.io/project/issues?id=apache_hive&pullRequest=3599&resolved=false&types=CODE_SMELL)
   
   [![No Coverage information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/CoverageChart/NoCoverageInfo-16px.png 'No Coverage information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=coverage&view=list) No Coverage information  
   [![No Duplication information](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/Duplications/NoDuplicationInfo-16px.png 'No Duplication information')](https://sonarcloud.io/component_measures?id=apache_hive&pullRequest=3599&metric=duplicated_lines_density&view=list) No Duplication information
   
   


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1154395035


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2060,7 +2086,7 @@ struct CreateTableRequest {
 }
 
 struct CreateDatabaseRequest {
-  1: required string databaseName,
+  1: optional string databaseName,

Review Comment:
   I changed it to optional because it was easy for me to use it that way in HMSClient and HiveMetaStore class. Let me revert to the original way.



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1029958439


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -2121,6 +2159,22 @@ struct AlterPartitionsRequest {
   7: optional string validWriteIdList
 }
 
+struct AppendPartitionRequest {
+  1: optional string catalogName,
+  2: required string dbName,
+  3: required string tableName,
+  4: required string partName,
+  5: optional EnvironmentContext environmentContext
+}
+
+struct AppendPartitionsRequest {

Review Comment:
   Ack



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1030965278


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1363,18 +1363,62 @@ private void create_database_core(RawStore ms, final Database db)
   }
 
   @Override
+  @Deprecated
   public void create_database(final Database db)
       throws AlreadyExistsException, InvalidObjectException, MetaException {
-    startFunction("create_database", ": " + db.toString());
+    CreateDatabaseRequest req = new CreateDatabaseRequest(db.getName());
+    if (db.isSetDescription()) {

Review Comment:
   There is no builder class in the CreateDatabaseRequest thrift object. 



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1024824772


##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java:
##########
@@ -1274,7 +1308,46 @@ public void createDatabase(Database db)
     if (!db.isSetCatalogName()) {
       db.setCatalogName(getDefaultCatalog(conf));
     }
-    client.create_database(db);
+    CreateDatabaseRequest req = new CreateDatabaseRequest();
+    req.setDatabaseName(db.getName());
+    if (db.isSetDescription()) {
+      req.setDescription(db.getDescription());
+    }
+    if (db.isSetLocationUri()) {
+      req.setLocationUri(db.getLocationUri());
+    }
+    if (db.isSetParameters()) {
+      req.setParameters(db.getParameters());
+    }
+    if (db.isSetPrivileges()) {
+      req.setPrivileges(db.getPrivileges());
+    }
+    if (db.isSetOwnerName()) {
+      req.setOwnerName(db.getOwnerName());
+    }
+    if (db.isSetOwnerType()) {

Review Comment:
   we can set property directly without null check, perhaps a database builder is helpful, for example:
   `
   new CreateDatabaseRequest.Builder(catName/**required args*/, dbName/**required args*/).createTime(time).location(uri).managedLocation(location).build()
   `
   This works for other request as well, such as `AppendPartitionsRequest`, `AddPartitionsRequest`, etc.



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by GitBox <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1024827172


##########
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift:
##########
@@ -895,13 +895,14 @@ struct AddPartitionsResult {
 
 // Request type for add_partitions_req
 struct AddPartitionsRequest {
-  1: required string dbName,
-  2: required string tblName,
-  3: required list<Partition> parts,
+  1: optional string dbName,

Review Comment:
   Is there any particular reason to change the required parameters to optional?



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] dengzhhu653 commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "dengzhhu653 (via GitHub)" <gi...@apache.org>.
dengzhhu653 commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1199533052


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1360,19 +1384,49 @@ public void create_database(final Database db)
         }
         Deadline.checkTimeout();
       }
+      Database db = new Database(createDatabaseRequest.getDatabaseName(), createDatabaseRequest.getDescription(),
+              createDatabaseRequest.getLocationUri() ,createDatabaseRequest.getParameters());
+      if (createDatabaseRequest.isSetPrivileges()) {
+        db.setPrivileges(createDatabaseRequest.getPrivileges());
+      }
+      if (createDatabaseRequest.isSetOwnerName()) {
+        db.setOwnerName(createDatabaseRequest.getOwnerName());
+      }
+      if (createDatabaseRequest.isSetOwnerType()) {
+        db.setOwnerType(createDatabaseRequest.getOwnerType());
+      }
+      db.setCatalogName(createDatabaseRequest.getCatalogName());
+      if (createDatabaseRequest.isSetCreateTime()) {
+        db.setCreateTime(createDatabaseRequest.getCreateTime());
+      }
+      if (createDatabaseRequest.isSetManagedLocationUri()) {
+        db.setManagedLocationUri(createDatabaseRequest.getManagedLocationUri());
+      }
+      if (createDatabaseRequest.isSetType()) {
+        db.setType(createDatabaseRequest.getType());
+      }
+      if (createDatabaseRequest.isSetDataConnectorName()) {
+        db.setConnector_name(createDatabaseRequest.getDataConnectorName());
+      }
+      if (createDatabaseRequest.isSetRemote_dbname()) {
+        db.setRemote_dbname(createDatabaseRequest.getRemote_dbname());
+      }
       create_database_core(getMS(), db);
+      createDatabaseRequest.setLocationUri(db.getLocationUri());

Review Comment:
   Why do we change the `createDatabaseRequest` after `create_database_core`



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] aturoczy commented on pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "aturoczy (via GitHub)" <gi...@apache.org>.
aturoczy commented on PR #3599:
URL: https://github.com/apache/hive/pull/3599#issuecomment-1687710582

   Is this ticket still active? Do we want to put this into Hive4? @nrg4878 @saihemanth-cloudera @ayushtkn @deniskuzZ 
   Pretty big change.


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


[GitHub] [hive] saihemanth-cloudera commented on a diff in pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.

Posted by "saihemanth-cloudera (via GitHub)" <gi...@apache.org>.
saihemanth-cloudera commented on code in PR #3599:
URL: https://github.com/apache/hive/pull/3599#discussion_r1210852419


##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/HMSHandler.java:
##########
@@ -1360,19 +1384,49 @@ public void create_database(final Database db)
         }
         Deadline.checkTimeout();
       }
+      Database db = new Database(createDatabaseRequest.getDatabaseName(), createDatabaseRequest.getDescription(),
+              createDatabaseRequest.getLocationUri() ,createDatabaseRequest.getParameters());
+      if (createDatabaseRequest.isSetPrivileges()) {
+        db.setPrivileges(createDatabaseRequest.getPrivileges());
+      }
+      if (createDatabaseRequest.isSetOwnerName()) {
+        db.setOwnerName(createDatabaseRequest.getOwnerName());
+      }
+      if (createDatabaseRequest.isSetOwnerType()) {
+        db.setOwnerType(createDatabaseRequest.getOwnerType());
+      }
+      db.setCatalogName(createDatabaseRequest.getCatalogName());
+      if (createDatabaseRequest.isSetCreateTime()) {
+        db.setCreateTime(createDatabaseRequest.getCreateTime());
+      }
+      if (createDatabaseRequest.isSetManagedLocationUri()) {
+        db.setManagedLocationUri(createDatabaseRequest.getManagedLocationUri());
+      }
+      if (createDatabaseRequest.isSetType()) {
+        db.setType(createDatabaseRequest.getType());
+      }
+      if (createDatabaseRequest.isSetDataConnectorName()) {
+        db.setConnector_name(createDatabaseRequest.getDataConnectorName());
+      }
+      if (createDatabaseRequest.isSetRemote_dbname()) {
+        db.setRemote_dbname(createDatabaseRequest.getRemote_dbname());
+      }
       create_database_core(getMS(), db);
+      createDatabaseRequest.setLocationUri(db.getLocationUri());

Review Comment:
   The same reasoning I gave for modification of `db` object after create_database_req()



-- 
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: gitbox-unsubscribe@hive.apache.org

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


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


Re: [PR] HIVE-26537: Deprecate older APIs in the HMS thrift interface. [hive]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #3599: HIVE-26537: Deprecate older APIs in the HMS thrift interface.
URL: https://github.com/apache/hive/pull/3599


-- 
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: gitbox-unsubscribe@hive.apache.org

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


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