vihangk1 commented on a change in pull request #1664:
URL: https://github.com/apache/hive/pull/1664#discussion_r522319043



##########
File path: 
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
##########
@@ -4664,6 +4664,58 @@ public void dropStoredProcedure(StoredProcedureRequest 
request) throws MetaExcep
     return client.get_all_stored_procedures(request);
   }
 
+  /**
+   * Builder for the GetProjectionsSpec. This is a projection specification 
for partitions returned from the HMS.
+   */
+  public static class GetPartitionProjectionsSpecBuilder {
+
+    java.util.List<java.lang.String> partitionList = null;

Review comment:
       Can we mark these fields as private? Any particular reason they are 
package private?

##########
File path: 
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
##########
@@ -4664,6 +4664,58 @@ public void dropStoredProcedure(StoredProcedureRequest 
request) throws MetaExcep
     return client.get_all_stored_procedures(request);
   }
 
+  /**
+   * Builder for the GetProjectionsSpec. This is a projection specification 
for partitions returned from the HMS.
+   */
+  public static class GetPartitionProjectionsSpecBuilder {
+
+    java.util.List<java.lang.String> partitionList = null;
+    java.lang.String includePartitionPattern = null;
+    java.lang.String excludePartitionPattern = null;
+
+    public void setPartitionList(List<String> partitionList) {
+      this.partitionList = partitionList;
+    }
+
+    public void setIncludePartitionPattern(String includePartitionPattern) {
+      this.includePartitionPattern = includePartitionPattern;
+    }
+
+    public void setExcludePartitionPattern(String excludePartitionPattern) {
+      this.excludePartitionPattern = excludePartitionPattern;
+    }
+
+    public GetProjectionsSpec build() {
+      return new GetProjectionsSpec(partitionList, includePartitionPattern, 
excludePartitionPattern);
+    }
+  }
+
+  /**
+   * Builder for the GetProjectionsSpec. This is a projection specification 
for tables returned from the HMS.
+   */
+  public static class GetTableProjectionsSpecBuilder {
+
+    java.util.List<java.lang.String> columnList = null;

Review comment:
       nit, no need to use fully qualified List here. also for String. Same for 
the earlier builder class as well. Please make these fields private.

##########
File path: 
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java
##########
@@ -4664,6 +4664,58 @@ public void dropStoredProcedure(StoredProcedureRequest 
request) throws MetaExcep
     return client.get_all_stored_procedures(request);
   }
 
+  /**
+   * Builder for the GetProjectionsSpec. This is a projection specification 
for partitions returned from the HMS.
+   */
+  public static class GetPartitionProjectionsSpecBuilder {

Review comment:
       From a user perspective, I think it would be more user friendly to have 
builder class for the whole request objects (eg. GetPartitionsRequestBuilder 
which gives ability to create a projection and filter spec)

##########
File path: 
standalone-metastore/metastore-common/src/main/thrift/hive_metastore.thrift
##########
@@ -1979,7 +1979,7 @@ struct GetPartitionsRequest {
    4: optional bool withAuth,
    5: optional string user,
    6: optional list<string> groupNames,
-   7: GetPartitionsProjectionSpec projectionSpec
+   7: GetProjectionsSpec projectionSpec
    8: GetPartitionsFilterSpec filterSpec, // TODO not yet implemented. Must be 
present but ignored

Review comment:
       Not related to your change, but looks like that TODO should be removed 
since we have the filterspec implemented.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to