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


Hey Ferdinand,

Generally I think the approach is sound. That is adding a permanent storage 
handler/serdes will make this more useful to users. It's unclear of the current 
code only handlers storage handlers or SERDEs as well?

One item we should change is the API signature of the new thrift HMS calls. All 
new HMS API's should be in the request/response format. For example if we have 
an API called "Some" the method should be:

SomeResponse Some(SomeRequest)

We should also stay away from ENUM's as they are not compatbile across releases.

Thanks you!!

Brock

- Brock Noland


On Feb. 17, 2015, 3:22 a.m., cheng xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/30909/
> -----------------------------------------------------------
> 
> (Updated Feb. 17, 2015, 3:22 a.m.)
> 
> 
> Review request for hive, Brock Noland, Dong Chen, Mohit Sabharwal, and Sergio 
> Pena.
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Changes includes:
> 1.Update the DDLTask to support using statement
> 2.Serialize the resource uri into table properties
> 3.Deserialize the resource uri and add them to the session classloader
> 4.Some query test and unit tests added
> 
> 
> Diffs
> -----
> 
>   
> itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
>  130fd67 
>   itests/test-serde/pom.xml cb79072 
>   
> itests/test-serde/src/main/java/org/apache/hadoop/hive/storagehandler/TestBase64TextOutputFormat.java
>  PRE-CREATION 
>   
> itests/test-serde/src/main/java/org/apache/hadoop/hive/storagehandler/TestStorageHandler.java
>  PRE-CREATION 
>   metastore/if/hive_metastore.thrift c2a2419 
>   metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java 
> 26ca208 
>   
> metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java 
> 0ff2863 
>   metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java 
> 0aa0f51 
>   metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java 
> fcaffc7 
>   metastore/src/java/org/apache/hadoop/hive/metastore/RawStore.java 2b49eab 
>   
> metastore/src/model/org/apache/hadoop/hive/metastore/model/MStorageDescriptor.java
>  9da3071 
>   
> metastore/src/model/org/apache/hadoop/hive/metastore/model/MStorageHandler.java
>  PRE-CREATION 
>   metastore/src/model/package.jdo b41b3d8 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreControlledCommit.java
>  cf068e4 
>   
> metastore/src/test/org/apache/hadoop/hive/metastore/DummyRawStoreForJdoConnection.java
>  5f28d73 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java 089bd94 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionTask.java 569c125 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/Utilities.java 7d72783 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/mr/ExecDriver.java 3a2a6ee 
>   ql/src/java/org/apache/hadoop/hive/ql/io/HiveInputFormat.java 9d5730d 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 4aac39a 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/HiveUtils.java c4633f6 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java 69a4545 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/FunctionSemanticAnalyzer.java 
> 1ef6d1b 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g 149b788 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java bdb9204 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/StorageFormat.java 7723430 
>   ql/src/java/org/apache/hadoop/hive/ql/plan/CreateTableDesc.java 8cadb96 
>   ql/src/java/org/apache/hadoop/hive/ql/util/SemanticAnalyzerHelper.java 
> PRE-CREATION 
>   ql/src/test/org/apache/hadoop/hive/ql/exec/TestUtilities.java 69f8889 
>   ql/src/test/queries/clientpositive/storage_handler_link_external_jar.q 
> PRE-CREATION 
>   ql/src/test/results/clientpositive/storage_handler_link_external_jar.q.out 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/30909/diff/
> 
> 
> Testing
> -------
> 
> newly added UT passed locally
> 
> 
> Thanks,
> 
> cheng xu
> 
>

Reply via email to