[GitHub] [hive] sunchao commented on a change in pull request #2878: HIVE-24893: Add UploadData and DownloadData to TCLIService.thrift

[GitBox <gi...@apache.org>]

sunchao commented on a change in pull request #2878:
URL: https://github.com/apache/hive/pull/2878#discussion_r784369235



##########
File path: service-rpc/if/TCLIService.thrift
##########
@@ -751,6 +751,52 @@ struct TGetTypeInfoResp {
   2: optional TOperationHandle operationHandle
 }
 
+// UploadData()
+//
+// UploadData data to table/path.
+struct TUploadDataReq {
+  // The session to execute the statement against
+  1: required TSessionHandle sessionHandle
+
+  // The table to be stored
+  2: optional string tableName
+
+  // The path to be stored
+  3: optional string path
+
+  // The data to be transferred
+  4: required binary values
+}
+
+struct TUploadDataResp {
+  1: required TStatus status
+  2: required TOperationHandle operationHandle
+}
+
+// DownloadData()
+//
+// Download data to JDBC client.
+struct TDownloadDataReq {
+  // The session to download data
+  1: required TSessionHandle sessionHandle
+
+  // The download table name
+  2: optional TPatternOrIdentifier tableName
+
+  // The download query
+  3: optional string query
+
+  // The download file format
+  4: optional string format

Review comment:
       Curious how is the file format being used in downloading data?

##########
File path: service-rpc/if/TCLIService.thrift
##########
@@ -751,6 +751,52 @@ struct TGetTypeInfoResp {
   2: optional TOperationHandle operationHandle
 }
 
+// UploadData()
+//
+// UploadData data to table/path.
+struct TUploadDataReq {
+  // The session to execute the statement against
+  1: required TSessionHandle sessionHandle
+
+  // The table to be stored
+  2: optional string tableName

Review comment:
       Should we make it more explicit and say that one of `tableName` or `path` must be set?

##########
File path: service-rpc/if/TCLIService.thrift
##########
@@ -751,6 +751,52 @@ struct TGetTypeInfoResp {
   2: optional TOperationHandle operationHandle
 }
 
+// UploadData()
+//
+// UploadData data to table/path.
+struct TUploadDataReq {
+  // The session to execute the statement against
+  1: required TSessionHandle sessionHandle
+
+  // The table to be stored
+  2: optional string tableName
+
+  // The path to be stored
+  3: optional string path
+
+  // The data to be transferred
+  4: required binary values
+}
+
+struct TUploadDataResp {
+  1: required TStatus status
+  2: required TOperationHandle operationHandle
+}
+
+// DownloadData()
+//
+// Download data to JDBC client.
+struct TDownloadDataReq {
+  // The session to download data
+  1: required TSessionHandle sessionHandle
+
+  // The download table name
+  2: optional TPatternOrIdentifier tableName
+
+  // The download query
+  3: optional string query

Review comment:
       What is this for?

##########
File path: service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIServiceClient.java
##########
@@ -36,56 +37,7 @@
 import org.apache.hive.service.cli.RowSetFactory;
 import org.apache.hive.service.cli.SessionHandle;
 import org.apache.hive.service.cli.TableSchema;
-import org.apache.hive.service.rpc.thrift.TCLIService;
-import org.apache.hive.service.rpc.thrift.TCancelDelegationTokenReq;
-import org.apache.hive.service.rpc.thrift.TCancelDelegationTokenResp;
-import org.apache.hive.service.rpc.thrift.TCancelOperationReq;
-import org.apache.hive.service.rpc.thrift.TCancelOperationResp;
-import org.apache.hive.service.rpc.thrift.TCloseOperationReq;
-import org.apache.hive.service.rpc.thrift.TCloseOperationResp;
-import org.apache.hive.service.rpc.thrift.TCloseSessionReq;
-import org.apache.hive.service.rpc.thrift.TCloseSessionResp;
-import org.apache.hive.service.rpc.thrift.TExecuteStatementReq;
-import org.apache.hive.service.rpc.thrift.TExecuteStatementResp;
-import org.apache.hive.service.rpc.thrift.TFetchResultsReq;
-import org.apache.hive.service.rpc.thrift.TFetchResultsResp;
-import org.apache.hive.service.rpc.thrift.TGetCatalogsReq;
-import org.apache.hive.service.rpc.thrift.TGetCatalogsResp;
-import org.apache.hive.service.rpc.thrift.TGetColumnsReq;
-import org.apache.hive.service.rpc.thrift.TGetColumnsResp;
-import org.apache.hive.service.rpc.thrift.TGetCrossReferenceReq;
-import org.apache.hive.service.rpc.thrift.TGetCrossReferenceResp;
-import org.apache.hive.service.rpc.thrift.TGetDelegationTokenReq;
-import org.apache.hive.service.rpc.thrift.TGetDelegationTokenResp;
-import org.apache.hive.service.rpc.thrift.TGetFunctionsReq;
-import org.apache.hive.service.rpc.thrift.TGetFunctionsResp;
-import org.apache.hive.service.rpc.thrift.TGetInfoReq;
-import org.apache.hive.service.rpc.thrift.TGetInfoResp;
-import org.apache.hive.service.rpc.thrift.TGetOperationStatusReq;
-import org.apache.hive.service.rpc.thrift.TGetOperationStatusResp;
-import org.apache.hive.service.rpc.thrift.TGetPrimaryKeysReq;
-import org.apache.hive.service.rpc.thrift.TGetPrimaryKeysResp;
-import org.apache.hive.service.rpc.thrift.TGetQueryIdReq;
-import org.apache.hive.service.rpc.thrift.TGetResultSetMetadataReq;
-import org.apache.hive.service.rpc.thrift.TGetResultSetMetadataResp;
-import org.apache.hive.service.rpc.thrift.TGetSchemasReq;
-import org.apache.hive.service.rpc.thrift.TGetSchemasResp;
-import org.apache.hive.service.rpc.thrift.TGetTableTypesReq;
-import org.apache.hive.service.rpc.thrift.TGetTableTypesResp;
-import org.apache.hive.service.rpc.thrift.TGetTablesReq;
-import org.apache.hive.service.rpc.thrift.TGetTablesResp;
-import org.apache.hive.service.rpc.thrift.TGetTypeInfoReq;
-import org.apache.hive.service.rpc.thrift.TGetTypeInfoResp;
-import org.apache.hive.service.rpc.thrift.TOpenSessionReq;
-import org.apache.hive.service.rpc.thrift.TOpenSessionResp;
-import org.apache.hive.service.rpc.thrift.TOperationHandle;
-import org.apache.hive.service.rpc.thrift.TProtocolVersion;
-import org.apache.hive.service.rpc.thrift.TRenewDelegationTokenReq;
-import org.apache.hive.service.rpc.thrift.TRenewDelegationTokenResp;
-import org.apache.hive.service.rpc.thrift.TSetClientInfoReq;
-import org.apache.hive.service.rpc.thrift.TSetClientInfoResp;
-import org.apache.hive.service.rpc.thrift.TStatus;
-import org.apache.hive.service.rpc.thrift.TStatusCode;
+import org.apache.hive.service.rpc.thrift.*;

Review comment:
       nit: don't use star import.

##########
File path: service-rpc/if/TCLIService.thrift
##########
@@ -751,6 +751,52 @@ struct TGetTypeInfoResp {
   2: optional TOperationHandle operationHandle
 }
 
+// UploadData()
+//
+// UploadData data to table/path.
+struct TUploadDataReq {
+  // The session to execute the statement against
+  1: required TSessionHandle sessionHandle
+
+  // The table to be stored
+  2: optional string tableName
+
+  // The path to be stored
+  3: optional string path
+
+  // The data to be transferred
+  4: required binary values
+}
+
+struct TUploadDataResp {
+  1: required TStatus status
+  2: required TOperationHandle operationHandle
+}
+
+// DownloadData()
+//
+// Download data to JDBC client.
+struct TDownloadDataReq {
+  // The session to download data
+  1: required TSessionHandle sessionHandle
+
+  // The download table name
+  2: optional TPatternOrIdentifier tableName

Review comment:
       Should we also support download from a specific path?




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