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




core/src/main/java/org/apache/oozie/command/SchemaCheckXCommand.java
Lines 256 (patched)
<https://reviews.apache.org/r/68467/#comment291238>

    Extract to local variable for better readability



core/src/main/java/org/apache/oozie/util/db/CompositeIndex.java
Lines 22-36 (patched)
<https://reviews.apache.org/r/68467/#comment291240>

    Should be computed like:
    ```
    String.format("CREATE INDEX I_%s_%s_%s ON %s (%s, %s", 
tableName.toUpperCase(), left.toUpperCase(), right.toUpperCase(), 
tableName.toUpperCase(), left.toUpperCase(), right.toUpperCase())
    ```
    
    given the attributes `tableName`, `left`, and `right`.



core/src/main/java/org/apache/oozie/util/db/CompositeIndex.java
Lines 38 (patched)
<https://reviews.apache.org/r/68467/#comment291241>

    Should be computed like discussed above.



core/src/main/java/org/apache/oozie/util/db/CompositeIndex.java
Lines 44-51 (patched)
<https://reviews.apache.org/r/68467/#comment291242>

    Why not use `values()` instead?



core/src/main/java/org/apache/oozie/util/db/CompositeIndex.java
Lines 53 (patched)
<https://reviews.apache.org/r/68467/#comment291243>

    Renamte to `find()`



core/src/test/java/org/apache/oozie/util/db/TestCompositeIndex.java
Lines 29 (patched)
<https://reviews.apache.org/r/68467/#comment291245>

    Would assert with `String` constants instead of existing `enum` values.



core/src/test/java/org/apache/oozie/util/db/TestCompositeIndex.java
Lines 35 (patched)
<https://reviews.apache.org/r/68467/#comment291246>

    Would assert with `String` constants instead of existing `enum` values.



tools/src/main/java/org/apache/oozie/tools/OozieDBCLI.java
Line 762 (original), 753 (patched)
<https://reviews.apache.org/r/68467/#comment291247>

    Nice :)


- András Piros


On Aug. 22, 2018, 6:48 a.m., Kinga Marton wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68467/
> -----------------------------------------------------------
> 
> (Updated Aug. 22, 2018, 6:48 a.m.)
> 
> 
> Review request for oozie, András Piros and Andras Salamon.
> 
> 
> Bugs: OOZIE-2684
>     https://issues.apache.org/jira/browse/OOZIE-2684
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> In SchemaCheckerService, Oozie compares expected and found indexed columns 
> and generates the below error message which could be confusing to the users.
> 
> `2016-09-16 12:39:26,564  INFO SchemaCheckXCommand:520 - 
> SERVER[c6402.ambari.apache.org] USER[-] GROUP[-] TOKEN[-] APP[-] JOB[-] 
> ACTION[-] About to check database schema`
> `2016-09-16 12:39:26,703 ERROR SchemaCheckXCommand:517 - 
> SERVER[c6402.ambari.apache.org] USER[-] GROUP[-] TOKEN[-] APP[-] JOB[-] 
> ACTION[-] Found [1] extra indexes for columns in table [WF_ACTIONS]: [wf_id]`
> `2016-09-16 12:39:26,723 ERROR SchemaCheckXCommand:517 - 
> SERVER[c6402.ambari.apache.org] USER[-] GROUP[-] TOKEN[-] APP[-] JOB[-] 
> ACTION[-] Database schema is BAD! Check previous error log messages for 
> details`
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/oozie/command/SchemaCheckXCommand.java 
> 521408ee 
>   core/src/main/java/org/apache/oozie/util/db/CompositeIndex.java 
> PRE-CREATION 
>   core/src/test/java/org/apache/oozie/util/db/TestCompositeIndex.java 
> PRE-CREATION 
>   tools/src/main/java/org/apache/oozie/tools/OozieDBCLI.java eeaaa60c 
> 
> 
> Diff: https://reviews.apache.org/r/68467/diff/1/
> 
> 
> Testing
> -------
> 
> Tested manually with MySql and PostgreSql.
> 
> I will open another Jira for making this part of code more testable, since 
> now is quite difficult to write some tests for it.
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>

Reply via email to