hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Carl Steinbach <...@apache.org>
Subject Re: Review Request 44031: HIVE-13130: HS2 changes: API calls for retrieving primary keys and foreign keys information
Date Thu, 07 Apr 2016 07:06:49 GMT


> On April 6, 2016, 8:53 p.m., Ashutosh Chauhan wrote:
> > service-rpc/if/TCLIService.thrift, line 963
> > <https://reviews.apache.org/r/44031/diff/2/?file=1317342#file1317342line963>
> >
> >     Do we really need catalogName? In Hive, there is no concept of Catalog at the
moment.

I disagree with Ashutosh. Consider the following points:
1) Whenever possible the HS2 Thrift API tries to maintain a 1:1 mapping with the corresponding
JDBC API methods.
2) The getSchemas(), getTables(), getColumns() and getFunctions() methods in the JDBC API
all take a catalogName field as input. As a result the getSchemas(), getTables(), getColumns()
and getFunctions() methods in the HS2 JDBC driver also take a catalogName field as input.
So far this hasn't caused any problems.
3) The GetSchemas(), GetTables(), GetColumns() and GetFunctions() RPCs in the HS2 Thrift API
have catalogName input parameters. Since this hasn't caused any problems I don't know why
we would want to break this convention now.

Please do not remove catalogName from any of these RPCs.


> On April 6, 2016, 8:53 p.m., Ashutosh Chauhan wrote:
> > service-rpc/if/TCLIService.thrift, line 966
> > <https://reviews.apache.org/r/44031/diff/2/?file=1317342#file1317342line966>
> >
> >     Its better to call this dbName to be consistent with Hive terminology.

No, it's better to use schemaName to be consistent with JDBC terminology, as well as to maintain
consistency with the other RPCs that are already part of this Thrift API.


> On April 6, 2016, 8:53 p.m., Ashutosh Chauhan wrote:
> > service-rpc/if/TCLIService.thrift, line 974
> > <https://reviews.apache.org/r/44031/diff/2/?file=1317342#file1317342line974>
> >
> >     Why is this optional?

There won't be an operationHandle to return if the status indicates an error. Consider what
happens when getPrimaryKeys() throws a SQLException instead of returning a ResultSet.


> On April 6, 2016, 8:53 p.m., Ashutosh Chauhan wrote:
> > service-rpc/if/TCLIService.thrift, line 985
> > <https://reviews.apache.org/r/44031/diff/2/?file=1317342#file1317342line985>
> >
> >     Database

No.


> On April 6, 2016, 8:53 p.m., Ashutosh Chauhan wrote:
> > service-rpc/if/TCLIService.thrift, line 982
> > <https://reviews.apache.org/r/44031/diff/2/?file=1317342#file1317342line982>
> >
> >     There is no catalog in Hive.

Ignore.


- Carl


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/44031/#review127434
-----------------------------------------------------------


On March 29, 2016, 12:11 a.m., Hari Sankar Sivarama Subramaniyan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44031/
> -----------------------------------------------------------
> 
> (Updated March 29, 2016, 12:11 a.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> HS2 changes: API calls for retrieving primary keys and foreign keys information. Changes
without the generated thrift files.
> 
> 
> Diffs
> -----
> 
>   jdbc/src/java/org/apache/hive/jdbc/HiveDatabaseMetaData.java 7e54d1f 
>   service-rpc/if/TCLIService.thrift aa28b6e 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService.h 3407564 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService.cpp fc82b88 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_server.skeleton.cpp 66ed6a7 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.h 7f1d9dd 
>   service-rpc/src/gen/thrift/gen-cpp/TCLIService_types.cpp 3a27a60 
>   service-rpc/src/gen/thrift/gen-javabean/org/apache/hive/service/rpc/thrift/TCLIService.java
c684f89 
>   service-rpc/src/gen/thrift/gen-php/TCLIService.php eba62f1 
>   service-rpc/src/gen/thrift/gen-php/Types.php b7df50a 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/TCLIService-remote 56f5c5d 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/TCLIService.py ad2d71d 
>   service-rpc/src/gen/thrift/gen-py/TCLIService/ttypes.py c691781 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service.rb 7d7f7a7 
>   service-rpc/src/gen/thrift/gen-rb/t_c_l_i_service_types.rb 07ed97c 
>   service/src/java/org/apache/hive/service/cli/CLIService.java ab30ae2 
>   service/src/java/org/apache/hive/service/cli/EmbeddedCLIServiceClient.java 9cad5be

>   service/src/java/org/apache/hive/service/cli/ICLIService.java 0a54bdd 
>   service/src/java/org/apache/hive/service/cli/operation/GetCrossReferenceOperation.java
PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/operation/GetPrimaryKeysOperation.java
PRE-CREATION 
>   service/src/java/org/apache/hive/service/cli/operation/OperationManager.java 56a9c18

>   service/src/java/org/apache/hive/service/cli/session/HiveSession.java 4f4e92d 
>   service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java 80a1844 
>   service/src/java/org/apache/hive/service/cli/thrift/RetryingThriftCLIServiceClient.java
14191e5 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 62fcde5 
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIServiceClient.java ccce6dc

> 
> Diff: https://reviews.apache.org/r/44031/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Hari Sankar Sivarama Subramaniyan
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message