Skip to content

Commit

Permalink
[CARBONDATA-3134] fixed null values when cachelevel is set as blocklet
Browse files Browse the repository at this point in the history
Problem:
For each blocklet an object of SegmentPropertiesAndSchemaHolder is created to store the schema used for query. This object is created only if no other blocklet has the same schema. To check the schema we are comparing List<ColumnSchema>, as the equals method in ColumnSchema does not check for columnUniqueId therefore this check is failing and the new restructured blocklet is using the schema of the old blocklet. Due to this the newly added column is being ignored as the old blocklet schema specifies that the column is delete(alter drop).

Solution:
Instead of checking the equality through equals and hashcode, write a new implementation for both and check based on columnUniqueId.
  • Loading branch information
kunal642 committed Nov 27, 2018
1 parent dbb7ca1 commit 5468f98
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -332,13 +334,42 @@ public void clear() {
}
SegmentPropertiesAndSchemaHolder.SegmentPropertiesWrapper other =
(SegmentPropertiesAndSchemaHolder.SegmentPropertiesWrapper) obj;
return tableIdentifier.equals(other.tableIdentifier) && columnsInTable
.equals(other.columnsInTable) && Arrays
return tableIdentifier.equals(other.tableIdentifier) && checkColumnSchemaEquality(
columnsInTable, other.columnsInTable) && Arrays
.equals(columnCardinality, other.columnCardinality);
}

private boolean checkColumnSchemaEquality(List<ColumnSchema> obj1, List<ColumnSchema> obj2) {
List<ColumnSchema> clonedObj1 = new ArrayList<>();
List<ColumnSchema> clonedObj2 = new ArrayList<>();
clonedObj1.addAll(obj1);
clonedObj2.addAll(obj2);
Collections.sort(clonedObj1, new Comparator<ColumnSchema>() {
@Override public int compare(ColumnSchema o1, ColumnSchema o2) {
return o1.getColumnUniqueId().compareTo(o2.getColumnUniqueId());
}
});
Collections.sort(clonedObj2, new Comparator<ColumnSchema>() {
@Override public int compare(ColumnSchema o1, ColumnSchema o2) {
return o1.getColumnUniqueId().compareTo(o2.getColumnUniqueId());
}
});
boolean exists = true;
for (int i = 0; i < obj1.size(); i++) {
if (!clonedObj1.get(i).equalsWithColumnId(clonedObj2.get(i))) {
exists = false;
break;
}
}
return exists;
}

@Override public int hashCode() {
return tableIdentifier.hashCode() + columnsInTable.hashCode() + Arrays
int hashCode = 0;
for (ColumnSchema columnSchema: columnsInTable) {
hashCode += columnSchema.hashCodeWithColumnId();
}
return tableIdentifier.hashCode() + hashCode + Arrays
.hashCode(columnCardinality);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,10 @@ public void setParentColumnTableRelations(
return result;
}

public int hashCodeWithColumnId() {
return hashCode() + ((columnUniqueId == null) ? 0 : columnUniqueId.hashCode());
}

/**
* Overridden equals method for columnSchema
*/
Expand Down Expand Up @@ -334,25 +338,14 @@ public void setParentColumnTableRelations(
* @param obj
* @return
*/
public boolean equalsWithStrictCheck(Object obj) {
public boolean equalsWithColumnId(Object obj) {
if (!this.equals(obj)) {
return false;
}
ColumnSchema other = (ColumnSchema) obj;
if (!columnUniqueId.equals(other.columnUniqueId) ||
(isDimensionColumn != other.isDimensionColumn) ||
(isSortColumn != other.isSortColumn)) {
return false;
}
if (encodingList.size() != other.encodingList.size()) {
if (!columnUniqueId.equals(other.columnUniqueId)) {
return false;
}
for (int i = 0; i < encodingList.size(); i++) {
if (encodingList.get(i).compareTo(other.encodingList.get(i)) != 0) {
return false;
}
}

return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ test("Creation of partition table should fail if the colname in table schema and
test("validate data in partition table after dropping and adding a column") {
sql("drop table if exists par")
sql("create table par(name string) partitioned by (age double) stored by " +
"'carbondata'")
"'carbondata' TBLPROPERTIES('cache_level'='blocklet')")
sql(s"load data local inpath '$resourcesPath/uniqwithoutheader.csv' into table par options" +
s"('header'='false')")
sql("alter table par drop columns(name)")
Expand Down

0 comments on commit 5468f98

Please sign in to comment.