hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Sushanth Sowmyan <khorg...@gmail.com>
Subject Re: Review Request 55392: HIVE-15469: Fix REPL DUMP/LOAD DROP_PTN so it works on non-string-ptn-key tables
Date Wed, 11 Jan 2017 19:59:54 GMT

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




itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
(line 550)
<https://reviews.apache.org/r/55392/#comment232522>

    This has minor clashes with issues.apache.org/jira/browse/HIVE-15365 , and easier to fix
here after that goes in rather than there.
    
    Instead of this code segment, we can use the following:
    
    ```java
    DropPartitionMessage dropPtnMsg = md.getDropPartitionMessage(event.getMessage());
    Table tableObj = dropPtnMsg.getTableObj();
    // .. and the asserts can remain as-is.
    ```
    
    Note that the first line is likely spurious as well if HIVE-15365 goes in, since it will
create the dropPtnMsg here, so the only line needing changing is the line instantiating tableObj.
    
    I can regenerate this patch post-HIVE-15365, not a problem.



itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestReplicationScenarios.java (line
345)
<https://reviews.apache.org/r/55392/#comment232523>

    One more post-HIVE-15365 comment. :)
    
    run(..) followed by verifyResults(..) is being replaced by two methods:
    
    verifyRun(.. , ..) or
    verifySetup(.. , ..)
    
    verifySetup is called in cases where you're still setting up the test, and verifying that
your setup happened correctly. In this case, for instance, the run followed by verifyResults
would be replaced by verifySetup instead.
    
    verifyRun is called when running some command that we're interested in testing where the
results showcase the functionality we're testing.
    
    The idea is that in steady state, after we finish our initial development, we flip a switch,
and all verifySetups don't do the additional verification step, whereas verifyRun still would.



itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestReplicationScenarios.java (line
372)
<https://reviews.apache.org/r/55392/#comment232524>

    still verifySetup case, as per prior comment.



itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestReplicationScenarios.java (line
385)
<https://reviews.apache.org/r/55392/#comment232525>

    still verifySetup, since we're testing that the source dropped the data correctly.



itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestReplicationScenarios.java (line
415)
<https://reviews.apache.org/r/55392/#comment232526>

    This is now a verifyRun, finally. :)


- Sushanth Sowmyan


On Jan. 10, 2017, 9:29 p.m., Vaibhav Gumashta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55392/
> -----------------------------------------------------------
> 
> (Updated Jan. 10, 2017, 9:29 p.m.)
> 
> 
> Review request for hive, Daniel Dai, Sushanth Sowmyan, and Thejas Nair.
> 
> 
> Bugs: HIVE-15469
>     https://issues.apache.org/jira/browse/HIVE-15469
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> https://issues.apache.org/jira/browse/HIVE-15469
> 
> 
> Diffs
> -----
> 
>   itests/hcatalog-unit/src/test/java/org/apache/hive/hcatalog/listener/TestDbNotificationListener.java
4eabb24 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/TestReplicationScenarios.java
6b86080 
>   metastore/src/java/org/apache/hadoop/hive/metastore/messaging/DropPartitionMessage.java
26aecb3 
>   metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONDropPartitionMessage.java
b8ea224 
>   metastore/src/java/org/apache/hadoop/hive/metastore/messaging/json/JSONMessageFactory.java
2749371 
>   ql/src/java/org/apache/hadoop/hive/ql/parse/ReplicationSemanticAnalyzer.java 85f8c64

> 
> Diff: https://reviews.apache.org/r/55392/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Vaibhav Gumashta
> 
>


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