[GitHub] carbondata pull request #2319: [CARBONDATA-2493] DataType.equals() failes fo...

[qiuchenjian-2]

GitHub user ajantha-bhat opened a pull request:

    https://github.com/apache/carbondata/pull/2319

     [CARBONDATA-2493] DataType.equals() failes for complex types

    This PR is dependent on #2313
   
    problem :
    [CARBONDATA-2493] DataType.equals() failes for complex types
   
    root cause : Complex types equals method was not implemented. Just the object's equals was used
   
    Solution: Add a equals method for all the complex type
   
    Be sure to do all of the following checklist to help us incorporate
    your contribution quickly and easily:
   
     - [ ] Any interfaces changed?NO
     
     - [ ] Any backward compatibility impacted?NA
     
     - [ ] Document update required?NA
   
     - [ ] Testing done
    Same test cases with complex type are ran
         
     - [ ] For large changes, please consider breaking it into sub-tasks under an umbrella JIRA.
   


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ajantha-bhat/carbondata coverity_fortify_fixes

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/carbondata/pull/2319.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2319
   
----
commit 89c0af941afb651c7d5879432e8f236d44eec400
Author: Raghunandan S <carbondatacontributions@...>
Date:   2017-08-27T18:07:05Z

    coverity scan fixes
    https://scan4.coverity.com/reports.htm#v29367/p11911

commit c9207d601b0631780261f3cf84989f778ff0b0a6
Author: ajantha-bhat <ajanthabhat@...>
Date:   2018-05-18T07:51:33Z

    [CARBONDATA-2493] DataType.equals() failes for complex types

----


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2319
 
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4804/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2319
 
    SDV Build Fail , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4988/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2319
 
    Build Failed  with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5962/



---

[qiuchenjian-2]

Github user ajantha-bhat commented on the issue:

    https://github.com/apache/carbondata/pull/2319
 
    retest this please


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2319
 
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4808/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2319
 
    Build Failed with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4810/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2319
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5966/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2319
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/4992/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2319
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/5969/



---

[qiuchenjian-2]

Github user gvramana commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2319#discussion_r189549116
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/datatype/StructField.java ---
    @@ -54,4 +54,41 @@ public String getFieldName() {
       public List<StructField> getChildren() {
         return children;
       }
    +
    +  @Override
    +  public int hashCode() {
    +    final int prime = 31;
    +    int result = 1;
    +    result = prime * result +
    +        fieldName.hashCode() + dataType.hashCode() + ((children == null) ? 0 : children.hashCode());
    +    return result;
    +  }
    +
    +  @Override
    +  public boolean equals(Object obj) {
    +    if (this == obj) {
    +      return true;
    +    }
    +    if (obj == null) {
    +      return false;
    +    }
    +    if (getClass() != obj.getClass()) {
    +      return false;
    +    }
    +    StructField other = (StructField) obj;
    +    if (!this.fieldName.equalsIgnoreCase(other.fieldName)) {
    +      return false;
    +    }
    +    if (!this.dataType.equals(other.dataType)) {
    +      return false;
    +    }
    +    if (children == null) {
    +      if (other.children != null) {
    +        return false;
    +      }
    +    } else if (!children.equals(other.children)) {
    --- End diff --
   
    children != null && other.children == null will through exception


---

[qiuchenjian-2]

Github user gvramana commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2319#discussion_r189547319
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/datatype/ArrayType.java ---
    @@ -52,11 +56,12 @@ public boolean equals(Object obj) {
       public int hashCode() {
         final int prime = 31;
         int result = 1;
    -    result = prime * result + getName().hashCode();
    +    result = prime * result + getName().hashCode() + getElementType().hashCode();
    --- End diff --
   
    Hash code function should be like this, keep multiplying and adding subfield
   
    final int prime = 31;
    int result = 1;
    result = prime * result + getName().hashCode();
    result = prime * result + keyType.hashCode();
    result = prime *  result + valueTyoe.hashCode();
    return result;
   
    Change accrodingly all hash functions


---

[qiuchenjian-2]

Github user ajantha-bhat commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2319#discussion_r189557906
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/datatype/StructField.java ---
    @@ -54,4 +54,41 @@ public String getFieldName() {
       public List<StructField> getChildren() {
         return children;
       }
    +
    +  @Override
    +  public int hashCode() {
    +    final int prime = 31;
    +    int result = 1;
    +    result = prime * result +
    +        fieldName.hashCode() + dataType.hashCode() + ((children == null) ? 0 : children.hashCode());
    +    return result;
    +  }
    +
    +  @Override
    +  public boolean equals(Object obj) {
    +    if (this == obj) {
    +      return true;
    +    }
    +    if (obj == null) {
    +      return false;
    +    }
    +    if (getClass() != obj.getClass()) {
    +      return false;
    +    }
    +    StructField other = (StructField) obj;
    +    if (!this.fieldName.equalsIgnoreCase(other.fieldName)) {
    +      return false;
    +    }
    +    if (!this.dataType.equals(other.dataType)) {
    +      return false;
    +    }
    +    if (children == null) {
    +      if (other.children != null) {
    +        return false;
    +      }
    +    } else if (!children.equals(other.children)) {
    --- End diff --
   
    This will not throw an exception as in all the equals method, we check if the object is null return false.
   
    But Modified code, as it is safe code.


---

[qiuchenjian-2]

Github user ajantha-bhat commented on a diff in the pull request:

    https://github.com/apache/carbondata/pull/2319#discussion_r189558009
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/metadata/datatype/ArrayType.java ---
    @@ -52,11 +56,12 @@ public boolean equals(Object obj) {
       public int hashCode() {
         final int prime = 31;
         int result = 1;
    -    result = prime * result + getName().hashCode();
    +    result = prime * result + getName().hashCode() + getElementType().hashCode();
    --- End diff --
   
    ok. Handled in all the places.


---

[qiuchenjian-2]

Github user ajantha-bhat commented on the issue:

    https://github.com/apache/carbondata/pull/2319
 
    retest this please


---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2319
 
    Build Success with Spark 2.1.0, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder1/6012/



---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/carbondata/pull/2319
 
    Build Success with Spark 2.2.1, Please check CI http://88.99.58.216:8080/job/ApacheCarbonPRBuilder/4854/



---

[qiuchenjian-2]

Github user ravipesala commented on the issue:

    https://github.com/apache/carbondata/pull/2319
 
    SDV Build Success , Please check CI http://144.76.159.231:8080/job/ApacheSDVTests/5024/



---

[qiuchenjian-2]

Github user asfgit closed the pull request at:

    https://github.com/apache/carbondata/pull/2319


---

[qiuchenjian-2]

Github user gvramana commented on the issue:

    https://github.com/apache/carbondata/pull/2319
 
    LGTM


---