Repository: drill Updated Branches: refs/heads/master b7c99dde0 -> b56086436
DRILL-3143: MaterializedField#clone should deep copy itself without disregarding its children Project: http://git-wip-us.apache.org/repos/asf/drill/repo Commit: http://git-wip-us.apache.org/repos/asf/drill/commit/a84eb583 Tree: http://git-wip-us.apache.org/repos/asf/drill/tree/a84eb583 Diff: http://git-wip-us.apache.org/repos/asf/drill/diff/a84eb583 Branch: refs/heads/master Commit: a84eb583ad15eece9dca64b10eabdd32584cc20d Parents: b7c99dd Author: Hanifi Gunes <[email protected]> Authored: Mon May 18 17:25:31 2015 -0700 Committer: Hanifi Gunes <[email protected]> Committed: Thu May 28 19:47:23 2015 -0700 ---------------------------------------------------------------------- .../codegen/templates/FixedValueVectors.java | 2 +- .../codegen/templates/NullableValueVectors.java | 2 +- .../codegen/templates/RepeatedValueVectors.java | 2 +- .../templates/VariableLengthVectors.java | 2 +- .../exec/physical/impl/join/HashJoinBatch.java | 2 +- .../drill/exec/record/MaterializedField.java | 40 ++++++--- .../drill/exec/vector/BaseValueVector.java | 2 +- .../org/apache/drill/exec/vector/BitVector.java | 2 +- .../exec/record/TestMaterializedField.java | 87 ++++++++++++++++++++ 9 files changed, 122 insertions(+), 19 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/drill/blob/a84eb583/exec/java-exec/src/main/codegen/templates/FixedValueVectors.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/codegen/templates/FixedValueVectors.java b/exec/java-exec/src/main/codegen/templates/FixedValueVectors.java index 0dffa0b..7103a17 100644 --- a/exec/java-exec/src/main/codegen/templates/FixedValueVectors.java +++ b/exec/java-exec/src/main/codegen/templates/FixedValueVectors.java @@ -168,7 +168,7 @@ public final class ${minor.class}Vector extends BaseDataValueVector implements F return new TransferImpl(getField()); } public TransferPair getTransferPair(FieldReference ref){ - return new TransferImpl(getField().clone(ref)); + return new TransferImpl(getField().withPath(ref)); } public TransferPair makeTransferPair(ValueVector to) { http://git-wip-us.apache.org/repos/asf/drill/blob/a84eb583/exec/java-exec/src/main/codegen/templates/NullableValueVectors.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/codegen/templates/NullableValueVectors.java b/exec/java-exec/src/main/codegen/templates/NullableValueVectors.java index ce6a3a7..90ec6be 100644 --- a/exec/java-exec/src/main/codegen/templates/NullableValueVectors.java +++ b/exec/java-exec/src/main/codegen/templates/NullableValueVectors.java @@ -234,7 +234,7 @@ public final class ${className} extends BaseDataValueVector implements <#if type return new TransferImpl(getField()); } public TransferPair getTransferPair(FieldReference ref){ - return new TransferImpl(getField().clone(ref)); + return new TransferImpl(getField().withPath(ref)); } public TransferPair makeTransferPair(ValueVector to) { http://git-wip-us.apache.org/repos/asf/drill/blob/a84eb583/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java b/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java index 37b8fac..7b2b78d 100644 --- a/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java +++ b/exec/java-exec/src/main/codegen/templates/RepeatedValueVectors.java @@ -90,7 +90,7 @@ public final class Repeated${minor.class}Vector extends BaseRepeatedValueVector @Override public TransferPair getTransferPair(FieldReference ref){ - return new TransferImpl(getField().clone(ref)); + return new TransferImpl(getField().withPath(ref)); } @Override http://git-wip-us.apache.org/repos/asf/drill/blob/a84eb583/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java b/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java index 529f21b..b3389e2 100644 --- a/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java +++ b/exec/java-exec/src/main/codegen/templates/VariableLengthVectors.java @@ -172,7 +172,7 @@ public final class ${minor.class}Vector extends BaseDataValueVector implements V return new TransferImpl(getField()); } public TransferPair getTransferPair(FieldReference ref){ - return new TransferImpl(getField().clone(ref)); + return new TransferImpl(getField().withPath(ref)); } public TransferPair makeTransferPair(ValueVector to) { http://git-wip-us.apache.org/repos/asf/drill/blob/a84eb583/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java index 5fd866f..73f3435 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/join/HashJoinBatch.java @@ -443,7 +443,7 @@ public class HashJoinBatch extends AbstractRecordBatch<HashJoinPOP> { } // make sure to project field with children for children to show up in the schema - final MaterializedField projected = field.cloneWithType(outputType); + final MaterializedField projected = field.withType(outputType); // Add the vector to our output container container.addOrGet(projected); http://git-wip-us.apache.org/repos/asf/drill/blob/a84eb583/exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java b/exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java index 64ba861..39b0af5 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/record/MaterializedField.java @@ -19,6 +19,7 @@ package org.apache.drill.exec.record; import java.util.Collection; import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; @@ -34,13 +35,17 @@ import org.apache.drill.exec.proto.UserBitShared.SerializedField; public class MaterializedField { - private Key key; + private final Key key; // use an ordered set as existing code relies on order (e,g. parquet writer) - private Set<MaterializedField> children = Sets.newLinkedHashSet(); + private final LinkedHashSet<MaterializedField> children; private MaterializedField(SchemaPath path, MajorType type) { - super(); - key = new Key(path, type); + this(path, type, new LinkedHashSet<MaterializedField>()); + } + + private MaterializedField(SchemaPath path, MajorType type, LinkedHashSet<MaterializedField> children) { + this.key = new Key(path, type); + this.children = children; } public static MaterializedField create(SerializedField serField){ @@ -77,14 +82,25 @@ public class MaterializedField { children.add(field); } - public MaterializedField cloneWithType(MajorType type) { - final MaterializedField clone = new MaterializedField(key.path, type); - clone.children = Sets.newLinkedHashSet(children); - return clone; + public MaterializedField clone() { + return withPathAndType(getPath(), getType()); + } + + public MaterializedField withType(MajorType type) { + return withPathAndType(getPath(), type); } - public MaterializedField clone(FieldReference ref){ - return create(ref, key.type); + public MaterializedField withPath(SchemaPath path) { + return withPathAndType(path, getType()); + } + + public MaterializedField withPathAndType(final SchemaPath path, final MajorType type) { + final LinkedHashSet<MaterializedField> newChildren = new LinkedHashSet<>(children.size()); + final MaterializedField clone = new MaterializedField(path, type, newChildren); + for (final MaterializedField child:children) { + newChildren.add(child.clone()); + } + return clone; } public String getLastName(){ @@ -268,8 +284,8 @@ public class MaterializedField { */ public class Key { - private SchemaPath path; - private MajorType type; + private final SchemaPath path; + private final MajorType type; private Key(SchemaPath path, MajorType type) { this.path = path; http://git-wip-us.apache.org/repos/asf/drill/blob/a84eb583/exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseValueVector.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseValueVector.java b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseValueVector.java index c8b6d4d..ec409a3 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseValueVector.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/BaseValueVector.java @@ -55,7 +55,7 @@ public abstract class BaseValueVector implements ValueVector { } public MaterializedField getField(FieldReference ref){ - return getField().clone(ref); + return getField().withPath(ref); } @Override http://git-wip-us.apache.org/repos/asf/drill/blob/a84eb583/exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java index f88a7bc..10bdf07 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/vector/BitVector.java @@ -191,7 +191,7 @@ public final class BitVector extends BaseDataValueVector implements FixedWidthVe return new TransferImpl(getField()); } public TransferPair getTransferPair(FieldReference ref) { - return new TransferImpl(getField().clone(ref)); + return new TransferImpl(getField().withPath(ref)); } public TransferPair makeTransferPair(ValueVector to) { http://git-wip-us.apache.org/repos/asf/drill/blob/a84eb583/exec/java-exec/src/test/java/org/apache/drill/exec/record/TestMaterializedField.java ---------------------------------------------------------------------- diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/record/TestMaterializedField.java b/exec/java-exec/src/test/java/org/apache/drill/exec/record/TestMaterializedField.java new file mode 100644 index 0000000..dbe4dc2 --- /dev/null +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/record/TestMaterializedField.java @@ -0,0 +1,87 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * <p/> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p/> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.drill.exec.record; + +import org.apache.drill.common.expression.SchemaPath; +import org.apache.drill.common.types.TypeProtos; +import org.apache.drill.common.types.Types; +import static org.junit.Assert.assertTrue; + +import org.junit.Before; +import org.junit.Test; + +public class TestMaterializedField { + + private static final String PARENT_NAME = "parent"; + private static final String PARENT_SECOND_NAME = "parent2"; + private static final String CHILD_NAME = "child"; + private static final String CHILD_SECOND_NAME = "child2"; + private static final TypeProtos.MajorType PARENT_TYPE = Types.repeated(TypeProtos.MinorType.MAP); + private static final TypeProtos.MajorType PARENT_SECOND_TYPE = Types.repeated(TypeProtos.MinorType.LIST); + private static final TypeProtos.MajorType CHILD_TYPE = Types.repeated(TypeProtos.MinorType.MAP); + private static final TypeProtos.MajorType CHILD_SECOND_TYPE = Types.repeated(TypeProtos.MinorType.MAP); + + // set of (name, type) tuples representing a test case + private static final Object[][] matrix = { + {PARENT_SECOND_NAME, PARENT_TYPE}, + {PARENT_NAME, PARENT_SECOND_TYPE}, + {CHILD_SECOND_NAME, CHILD_TYPE}, + {CHILD_NAME, CHILD_SECOND_TYPE}, + }; + + private MaterializedField parent; + private MaterializedField child; + + @Before + public void initialize() { + parent = MaterializedField.create(PARENT_NAME, PARENT_TYPE); + child = MaterializedField.create(CHILD_NAME, CHILD_TYPE); + parent.addChild(child); + } + + @Test + public void testClone() { + final MaterializedField cloneParent = parent.clone(); + final boolean isParentEqual = parent.equals(cloneParent); + assertTrue("Cloned parent does not match the original", isParentEqual); + + final MaterializedField cloneChild = child.clone(); + final boolean isChildEqual = child.equals(cloneChild); + assertTrue("Cloned child does not match the original", isChildEqual); + + for (final MaterializedField field:new MaterializedField[]{parent, child}) { + for (Object[] args:matrix) { + final SchemaPath path = SchemaPath.getSimplePath(args[0].toString()); + final TypeProtos.MajorType type = TypeProtos.MajorType.class.cast(args[1]); + + final MaterializedField clone = field.withPathAndType(path, type); + + final boolean isPathEqual = path.equals(clone.getPath()); + assertTrue("Cloned path does not match the original", isPathEqual); + + final boolean isTypeEqual = type.equals(clone.getType()); + assertTrue("Cloned type does not match the original", isTypeEqual); + + final boolean isChildrenEqual = field.getChildren().equals(clone.getChildren()); + assertTrue("Cloned children do not match the original", isChildrenEqual); + } + } + + } + +}
