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




ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSurrogateKey.java
Lines 118 (patched)
<https://reviews.apache.org/r/68744/#comment292883>

    taskId won't change during a lifecycle of this object in tasks. So, you can 
move this line in configure(), save it in a field and use it in evaluate().



ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSurrogateKey.java
Lines 119 (patched)
<https://reviews.apache.org/r/68744/#comment292884>

    Don't see a reason to have lastTaskId field. Perhaps, you can get rid of it.



ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSurrogateKey.java
Lines 124-125 (patched)
<https://reviews.apache.org/r/68744/#comment292885>

    This can be moved to configure()



ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFSurrogateKey.java
Lines 55 (patched)
<https://reviews.apache.org/r/68744/#comment292886>

    do udf.setWriteId(3) and call runAndVerifyConst() again to get coverage on 
writeId too.



ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFSurrogateKey.java
Lines 82 (patched)
<https://reviews.apache.org/r/68744/#comment292887>

    Also add -ve tests to catch exception when we go over limits.


- Ashutosh Chauhan


On Sept. 18, 2018, 9:35 p.m., Miklos Gergely wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68744/
> -----------------------------------------------------------
> 
> (Updated Sept. 18, 2018, 9:35 p.m.)
> 
> 
> Review request for hive and Ashutosh Chauhan.
> 
> 
> Bugs: HIVE-20536
>     https://issues.apache.org/jira/browse/HIVE-20536
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> Add new function that allows the generation of a surrogate key composed of 
> the write id, the task id, and an incremental row id.
> 
> 
> Diffs
> -----
> 
>   ql/src/java/org/apache/hadoop/hive/ql/exec/FunctionRegistry.java 3f538b3 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java 
> 3309b9b 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java 6d7e63e 
>   
> ql/src/java/org/apache/hadoop/hive/ql/udf/generic/GenericUDFSurrogateKey.java 
> PRE-CREATION 
>   
> ql/src/test/org/apache/hadoop/hive/ql/udf/generic/TestGenericUDFSurrogateKey.java
>  PRE-CREATION 
>   ql/src/test/results/clientpositive/show_functions.q.out 8d41e78 
> 
> 
> Diff: https://reviews.apache.org/r/68744/diff/2/
> 
> 
> Testing
> -------
> 
> Added a new junit test for the function.
> Tested it in beeline by adding one row, adding multiple rows, adding mutliple 
> rows to multiple tables via multuple insert (all having their own 
> surrogate_key column)
> 
> 
> Thanks,
> 
> Miklos Gergely
> 
>

Reply via email to