From df856d6007fdb51dc9e0f6eccb3116c3048655a7 Mon Sep 17 00:00:00 2001 From: Dongjoon Hyun Date: Fri, 3 Apr 2015 15:00:52 +0900 Subject: [PATCH] TAJO-1512: Fix various Expr cloning bugs --- .../org/apache/tajo/algebra/Aggregation.java | 16 ++++--- .../org/apache/tajo/algebra/AlterTable.java | 8 +++- .../apache/tajo/algebra/BetweenPredicate.java | 2 +- .../org/apache/tajo/algebra/CreateTable.java | 46 ++++++++++++------- .../java/org/apache/tajo/algebra/Insert.java | 4 +- .../apache/tajo/algebra/UnaryOperator.java | 4 +- .../apache/tajo/algebra/ValueListExpr.java | 2 +- .../org/apache/tajo/algebra/TestExpr.java | 25 ++++++++++ 8 files changed, 79 insertions(+), 28 deletions(-) diff --git a/tajo-algebra/src/main/java/org/apache/tajo/algebra/Aggregation.java b/tajo-algebra/src/main/java/org/apache/tajo/algebra/Aggregation.java index edb523b552..8bcd7e2998 100644 --- a/tajo-algebra/src/main/java/org/apache/tajo/algebra/Aggregation.java +++ b/tajo-algebra/src/main/java/org/apache/tajo/algebra/Aggregation.java @@ -71,14 +71,18 @@ public boolean equalsTo(Expr expr) { public Object clone() throws CloneNotSupportedException { Aggregation aggregation = (Aggregation) super.clone(); - aggregation.namedExprs = new NamedExpr[namedExprs.length]; - for (int i = 0; i < namedExprs.length; i++) { - aggregation.namedExprs[i] = (NamedExpr) namedExprs[i].clone(); + if (namedExprs != null) { + aggregation.namedExprs = new NamedExpr[namedExprs.length]; + for (int i = 0; i < namedExprs.length; i++) { + aggregation.namedExprs[i] = (NamedExpr) namedExprs[i].clone(); + } } - aggregation.groups = new GroupElement[groups.length]; - for (int i = 0; i < groups.length; i++) { - aggregation.groups[i] = (GroupElement) groups[i].clone(); + if (groups != null) { + aggregation.groups = new GroupElement[groups.length]; + for (int i = 0; i < groups.length; i++) { + aggregation.groups[i] = (GroupElement) groups[i].clone(); + } } return aggregation; } diff --git a/tajo-algebra/src/main/java/org/apache/tajo/algebra/AlterTable.java b/tajo-algebra/src/main/java/org/apache/tajo/algebra/AlterTable.java index 3ca9a7bbd5..9440257976 100644 --- a/tajo-algebra/src/main/java/org/apache/tajo/algebra/AlterTable.java +++ b/tajo-algebra/src/main/java/org/apache/tajo/algebra/AlterTable.java @@ -168,12 +168,16 @@ public Object clone() throws CloneNotSupportedException { alter.newTableName = newTableName; alter.columnName = columnName; alter.newColumnName = newColumnName; - alter.addNewColumn = (ColumnDefinition) addNewColumn.clone(); + if (addNewColumn != null) { + alter.addNewColumn = (ColumnDefinition) addNewColumn.clone(); + } alter.alterTableOpType = alterTableOpType; alter.columns = columns; alter.values = values; alter.location = location; - alter.params = new HashMap(params); + if (params != null) { + alter.params = new HashMap(params); + } return alter; } } diff --git a/tajo-algebra/src/main/java/org/apache/tajo/algebra/BetweenPredicate.java b/tajo-algebra/src/main/java/org/apache/tajo/algebra/BetweenPredicate.java index 4b17bdb204..e3680617b1 100644 --- a/tajo-algebra/src/main/java/org/apache/tajo/algebra/BetweenPredicate.java +++ b/tajo-algebra/src/main/java/org/apache/tajo/algebra/BetweenPredicate.java @@ -81,7 +81,7 @@ public Object clone() throws CloneNotSupportedException { between.not = not; between.symmetric = symmetric; between.predicand = (Expr) predicand.clone(); - between.begin = (Expr) between.clone(); + between.begin = (Expr) begin.clone(); between.end = (Expr) end.clone(); return between; } diff --git a/tajo-algebra/src/main/java/org/apache/tajo/algebra/CreateTable.java b/tajo-algebra/src/main/java/org/apache/tajo/algebra/CreateTable.java index 4056ed385e..2d4a24108a 100644 --- a/tajo-algebra/src/main/java/org/apache/tajo/algebra/CreateTable.java +++ b/tajo-algebra/src/main/java/org/apache/tajo/algebra/CreateTable.java @@ -196,8 +196,12 @@ public Object clone() throws CloneNotSupportedException { createTable.storageType = storageType; createTable.location = location; createTable.subquery = subquery; - createTable.params = new HashMap(params); - createTable.partition = (PartitionMethodDescExpr) partition.clone(); + if (params != null) { + createTable.params = new HashMap(params); + } + if (partition != null) { + createTable.partition = (PartitionMethodDescExpr) partition.clone(); + } createTable.ifNotExists = ifNotExists; return createTable; } @@ -305,11 +309,13 @@ public boolean equals(Object object) { @Override public Object clone() throws CloneNotSupportedException { RangePartition range = (RangePartition) super.clone(); - range.columns = new ColumnReferenceExpr[columns.length]; - for (int i = 0; i < columns.length; i++) { - range.columns[i] = (ColumnReferenceExpr) columns[i].clone(); + if (columns != null) { + range.columns = new ColumnReferenceExpr[columns.length]; + for (int i = 0; i < columns.length; i++) { + range.columns[i] = (ColumnReferenceExpr) columns[i].clone(); + } } - if (range.specifiers != null) { + if (specifiers != null) { range.specifiers = new ArrayList(); for (int i = 0; i < specifiers.size(); i++) { range.specifiers.add(specifiers.get(i)); @@ -376,9 +382,11 @@ public boolean equals(Object object) { @Override public Object clone() throws CloneNotSupportedException { HashPartition hash = (HashPartition) super.clone(); - hash.columns = new ColumnReferenceExpr[columns.length]; - for (int i = 0; i < columns.length; i++) { - hash.columns[i] = (ColumnReferenceExpr) columns[i].clone(); + if (columns != null) { + hash.columns = new ColumnReferenceExpr[columns.length]; + for (int i = 0; i < columns.length; i++) { + hash.columns[i] = (ColumnReferenceExpr) columns[i].clone(); + } } hash.quantity = quantity; if (specifiers != null) { @@ -428,9 +436,11 @@ public boolean equals(Object object) { @Override public Object clone() throws CloneNotSupportedException { ListPartition listPartition = (ListPartition) super.clone(); - listPartition.columns = new ColumnReferenceExpr[columns.length]; - for (int i = 0; i < columns.length; i++) { - listPartition.columns[i] = (ColumnReferenceExpr) columns[i].clone(); + if (columns != null) { + listPartition.columns = new ColumnReferenceExpr[columns.length]; + for (int i = 0; i < columns.length; i++) { + listPartition.columns[i] = (ColumnReferenceExpr) columns[i].clone(); + } } if (specifiers != null) { listPartition.specifiers = new ArrayList(); @@ -472,9 +482,11 @@ public boolean equals(Object object) { @Override public Object clone() throws CloneNotSupportedException { ColumnPartition columnPartition = (ColumnPartition) super.clone(); - columnPartition.columns = new ColumnDefinition[columns.length]; - for (int i = 0; i < columns.length; i++) { - columnPartition.columns[i] = (ColumnDefinition) columns[i].clone(); + if (columns != null) { + columnPartition.columns = new ColumnDefinition[columns.length]; + for (int i = 0; i < columns.length; i++) { + columnPartition.columns[i] = (ColumnDefinition) columns[i].clone(); + } } return columnPartition; } @@ -524,7 +536,9 @@ public boolean equals(Object o) { @Override public Object clone() throws CloneNotSupportedException { RangePartitionSpecifier specifier = (RangePartitionSpecifier) super.clone(); - specifier.end = (Expr) end.clone(); + if (end != null) { + specifier.end = (Expr) end.clone(); + } specifier.maxValue = maxValue; return specifier; } diff --git a/tajo-algebra/src/main/java/org/apache/tajo/algebra/Insert.java b/tajo-algebra/src/main/java/org/apache/tajo/algebra/Insert.java index ce9b7034a6..0826d9003e 100644 --- a/tajo-algebra/src/main/java/org/apache/tajo/algebra/Insert.java +++ b/tajo-algebra/src/main/java/org/apache/tajo/algebra/Insert.java @@ -151,7 +151,9 @@ public Object clone() throws CloneNotSupportedException { insert.storageType = storageType; insert.location = location; insert.subquery = (Expr) subquery.clone(); - insert.params = new HashMap(params); + if (params != null) { + insert.params = new HashMap(params); + } return insert; } } diff --git a/tajo-algebra/src/main/java/org/apache/tajo/algebra/UnaryOperator.java b/tajo-algebra/src/main/java/org/apache/tajo/algebra/UnaryOperator.java index b85d58ce5c..46f03eb370 100644 --- a/tajo-algebra/src/main/java/org/apache/tajo/algebra/UnaryOperator.java +++ b/tajo-algebra/src/main/java/org/apache/tajo/algebra/UnaryOperator.java @@ -49,7 +49,9 @@ public int hashCode() { @Override public Object clone() throws CloneNotSupportedException { UnaryOperator unaryOperator = (UnaryOperator) super.clone(); - unaryOperator.child = (Expr) child.clone(); + if (child != null) { + unaryOperator.child = (Expr) child.clone(); + } return unaryOperator; } } diff --git a/tajo-algebra/src/main/java/org/apache/tajo/algebra/ValueListExpr.java b/tajo-algebra/src/main/java/org/apache/tajo/algebra/ValueListExpr.java index 32a5e82aff..7923e31d2d 100644 --- a/tajo-algebra/src/main/java/org/apache/tajo/algebra/ValueListExpr.java +++ b/tajo-algebra/src/main/java/org/apache/tajo/algebra/ValueListExpr.java @@ -52,7 +52,7 @@ public Object clone() throws CloneNotSupportedException { ValueListExpr valueListExpr = (ValueListExpr) super.clone(); valueListExpr.values = new Expr[values.length]; for (int i = 0; i < values.length; i++) { - valueListExpr.values = (Expr[]) values[i].clone(); + valueListExpr.values[i] = (Expr) values[i].clone(); } return valueListExpr; } diff --git a/tajo-algebra/src/test/java/org/apache/tajo/algebra/TestExpr.java b/tajo-algebra/src/test/java/org/apache/tajo/algebra/TestExpr.java index 513b66a0f1..db4c3e92a4 100644 --- a/tajo-algebra/src/test/java/org/apache/tajo/algebra/TestExpr.java +++ b/tajo-algebra/src/test/java/org/apache/tajo/algebra/TestExpr.java @@ -120,6 +120,31 @@ public void testEquals() throws Exception { assertFalse(sort.equals(different)); } + @Test + public void testClone() throws Exception { + Expr expr = new BinaryOperator(OpType.LessThan, + new LiteralValue("1", LiteralType.Unsigned_Integer), + new LiteralValue("2", LiteralType.Unsigned_Integer)); + + Relation relation = new Relation("employee"); + Selection selection = new Selection(expr); + selection.setChild(relation); + + Aggregation aggregation = new Aggregation(); + aggregation.setTargets(new NamedExpr[]{ + new NamedExpr(new ColumnReferenceExpr("col1")) + } + ); + + aggregation.setChild(selection); + + Sort.SortSpec spec = new Sort.SortSpec(new ColumnReferenceExpr("col2")); + Sort sort = new Sort(new Sort.SortSpec[]{spec}); + sort.setChild(aggregation); + + assertEquals(sort, sort.clone()); + } + private Expr generateOneExpr() { Expr expr = new BinaryOperator(OpType.LessThan, new LiteralValue("1", LiteralType.Unsigned_Integer),