You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by StevenMPhillips <gi...@git.apache.org> on 2015/10/16 09:26:14 UTC

[GitHub] drill pull request: Drill-3232: Handle promoting writers and vecto...

GitHub user StevenMPhillips opened a pull request:

    https://github.com/apache/drill/pull/207

    Drill-3232: Handle promoting writers and vectors from primitive to union type

    The PromotableFieldWriter delegates all FieldWriter API calls to an inner FieldWriter. This inner field writer can start as a specific type, and this class will promote the writer to a UnionWriter if a call is made that the specifically typed writer cannot handle. A new UnionVector is created, wrapping the original vector, and replaces the original vector in the parent vector, which can be either an AbstractMapVector or a ListVector.


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

    $ git pull https://github.com/StevenMPhillips/drill drill-3232

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

    https://github.com/apache/drill/pull/207.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 #207
    
----
commit 81a4e714a868634592bb449eb490b7a30ac990fe
Author: Steven Phillips <sm...@apache.org>
Date:   2015-10-01T10:26:34Z

    DRILL-3229: Implement Union type vector

commit 53aaacaebff923094dee4e4d3c5fc52f112c12d4
Author: Steven Phillips <sm...@apache.org>
Date:   2015-10-05T04:32:34Z

    DRILL-3233: Expression handling for Union types

commit 3807ff05f8232586a83b4895ea5216ebdf4c60d1
Author: Steven Phillips <sm...@apache.org>
Date:   2015-10-09T19:59:37Z

    Drill-3232: Promotable writer

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471573
  
    --- Diff: exec/java-exec/src/main/codegen/templates/UnionVector.java ---
    @@ -0,0 +1,479 @@
    +/**
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.
    + */
    +
    +import org.apache.drill.common.types.TypeProtos.MinorType;
    +
    +<@pp.dropOutputFile />
    +<@pp.changeOutputFile name="/org/apache/drill/exec/vector/complex/impl/UnionVector.java" />
    +
    +
    +<#include "/@includes/license.ftl" />
    +
    +package org.apache.drill.exec.vector.complex.impl;
    +
    +<#include "/@includes/vv_imports.ftl" />
    +import java.util.Iterator;
    +import org.apache.drill.exec.vector.complex.impl.ComplexCopier;
    +import org.apache.drill.exec.util.CallBack;
    +
    +/*
    + * This class is generated using freemarker and the ${.template_name} template.
    + */
    +@SuppressWarnings("unused")
    +
    +
    +public class UnionVector implements ValueVector {
    +
    +  private MaterializedField field;
    +  private BufferAllocator allocator;
    +  private Accessor accessor = new Accessor();
    +  private Mutator mutator = new Mutator();
    +  private int valueCount;
    +
    +  private MapVector internalMap;
    +  private SingleMapWriter internalMapWriter;
    +  private UInt1Vector typeVector;
    +
    +  private MapVector mapVector;
    +  private ListVector listVector;
    +  private NullableBigIntVector bigInt;
    +  private NullableVarCharVector varChar;
    +
    +  private FieldReader reader;
    +  private NullableBitVector bit;
    +
    +  private State state = State.INIT;
    +  private int singleType = 0;
    +  private ValueVector singleVector;
    +  private MajorType majorType;
    +
    +  private final CallBack callBack;
    +
    +  private enum State {
    +    INIT, SINGLE, MULTI
    +  }
    +
    +  public UnionVector(MaterializedField field, BufferAllocator allocator, CallBack callBack) {
    +    this.field = field.clone();
    +    this.allocator = allocator;
    +    internalMap = new MapVector("internal", allocator, callBack);
    +    internalMapWriter = new SingleMapWriter(internalMap, null, true, true);
    +    this.typeVector = internalMap.addOrGet("types", Types.required(MinorType.UINT1), UInt1Vector.class);
    +    this.field.addChild(internalMap.getField().clone());
    +    this.majorType = field.getType();
    +    this.callBack = callBack;
    +  }
    +
    +  private void updateState(ValueVector v) {
    +    if (state == State.INIT) {
    +      state = State.SINGLE;
    +      singleVector = v;
    +      singleType = v.getField().getType().getMinorType().getNumber();
    +    } else {
    +      state = State.MULTI;
    +      singleVector = null;
    +    }
    +  }
    +
    +  public List<MinorType> getSubTypes() {
    +    return majorType.getSubTypeList();
    +  }
    +
    +  private void addSubType(MinorType type) {
    +    majorType =  MajorType.newBuilder(this.majorType).addSubType(type).build();
    +    if (callBack != null) {
    +      callBack.doWork();
    +    }
    +  }
    +
    +  public boolean isSingleType() {
    +    return state == State.SINGLE && singleType != MinorType.LIST_VALUE;
    +  }
    +
    +  public ValueVector getSingleVector() {
    +    assert state != State.MULTI : "Cannot get single vector when there are multiple types";
    +    assert state != State.INIT : "Cannot get single vector when there are no types";
    +    return singleVector;
    +  }
    +
    +  private static final MajorType MAP_TYPE = Types.optional(MinorType.MAP);
    +
    +  public MapVector getMap() {
    +    if (mapVector == null) {
    +      int vectorCount = internalMap.size();
    +      mapVector = internalMap.addOrGet("map", MAP_TYPE, MapVector.class);
    +      updateState(mapVector);
    +      addSubType(MinorType.MAP);
    +      if (internalMap.size() > vectorCount) {
    +        mapVector.allocateNew();
    +      }
    +    }
    +    return mapVector;
    +  }
    +
    +  <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
    +  <#assign fields = minor.fields!type.fields />
    +  <#assign uncappedName = name?uncap_first/>
    +  <#if !minor.class?starts_with("Decimal")>
    +
    +  private Nullable${name}Vector ${uncappedName}Vector;
    +  private static final MajorType ${name?upper_case}_TYPE = Types.optional(MinorType.${name?upper_case});
    +
    +  public Nullable${name}Vector get${name}Vector() {
    +    if (${uncappedName}Vector == null) {
    +      int vectorCount = internalMap.size();
    +      ${uncappedName}Vector = internalMap.addOrGet("${uncappedName}", ${name?upper_case}_TYPE, Nullable${name}Vector.class);
    +      updateState(${uncappedName}Vector);
    +      addSubType(MinorType.${name?upper_case});
    +      if (internalMap.size() > vectorCount) {
    +        ${uncappedName}Vector.allocateNew();
    +      }
    +    }
    +    return ${uncappedName}Vector;
    +  }
    +
    +  </#if>
    +
    +  </#list></#list>
    +
    +  private static final MajorType LIST_TYPE = Types.optional(MinorType.LIST);
    +
    +  public ListVector getList() {
    +    if (listVector == null) {
    +      int vectorCount = internalMap.size();
    +      listVector = internalMap.addOrGet("list", LIST_TYPE, ListVector.class);
    +      updateState(listVector);
    +      addSubType(MinorType.LIST);
    +      if (internalMap.size() > vectorCount) {
    +        listVector.allocateNew();
    +      }
    +    }
    +    return listVector;
    +  }
    +
    +  public int getTypeValue(int index) {
    +    return typeVector.getAccessor().get(index);
    +  }
    +
    +  public UInt1Vector getTypeVector() {
    +    return typeVector;
    +  }
    +
    +  @Override
    +  public void allocateNew() throws OutOfMemoryRuntimeException {
    +    internalMap.allocateNew();
    +    if (typeVector != null) {
    +      typeVector.zeroVector();
    +    }
    +  }
    +
    +  @Override
    +  public boolean allocateNewSafe() {
    +    boolean safe = internalMap.allocateNewSafe();
    +    if (safe) {
    +      if (typeVector != null) {
    +        typeVector.zeroVector();
    +      }
    +    }
    +    return safe;
    +  }
    +
    +  @Override
    +  public void setInitialCapacity(int numRecords) {
    +  }
    +
    +  @Override
    +  public int getValueCapacity() {
    +    return Math.min(typeVector.getValueCapacity(), internalMap.getValueCapacity());
    +  }
    +
    +  @Override
    +  public void close() {
    +  }
    +
    +  @Override
    +  public void clear() {
    +    internalMap.clear();
    +  }
    +
    +  @Override
    +  public MaterializedField getField() {
    +    return field;
    +  }
    +
    +  @Override
    +  public TransferPair getTransferPair() {
    +    return new TransferImpl(field);
    +  }
    +
    +  @Override
    +  public TransferPair getTransferPair(FieldReference ref) {
    +    return new TransferImpl(field.withPath(ref));
    +  }
    +
    +  @Override
    +  public TransferPair makeTransferPair(ValueVector target) {
    +    return new TransferImpl((UnionVector) target);
    +  }
    +
    +  public void transferTo(UnionVector target) {
    +    internalMap.makeTransferPair(target.internalMap).transfer();
    +    target.valueCount = valueCount;
    +    target.majorType = majorType;
    +  }
    +
    +  public void copyFrom(int inIndex, int outIndex, UnionVector from) {
    +    from.getReader().setPosition(inIndex);
    +    getWriter().setPosition(outIndex);
    +    ComplexCopier copier = new ComplexCopier(from.reader, mutator.writer);
    +    copier.write();
    +  }
    +
    +  public void copyFromSafe(int inIndex, int outIndex, UnionVector from) {
    +    copyFrom(inIndex, outIndex, from);
    +  }
    +
    +  public void addVector(ValueVector v) {
    +    internalMap.putChild(v.getField().getType().getMinorType().name().toLowerCase(), v);
    +    addSubType(v.getField().getType().getMinorType());
    +  }
    +
    +  private class TransferImpl implements TransferPair {
    +
    +    UnionVector to;
    +
    +    public TransferImpl(MaterializedField field) {
    +      to = new UnionVector(field, allocator, null);
    +    }
    +
    +    public TransferImpl(UnionVector to) {
    +      this.to = to;
    +    }
    +
    +    @Override
    +    public void transfer() {
    +      transferTo(to);
    +    }
    +
    +    @Override
    +    public void splitAndTransfer(int startIndex, int length) {
    +
    +    }
    +
    +    @Override
    +    public ValueVector getTo() {
    +      return to;
    +    }
    +
    +    @Override
    +    public void copyValueSafe(int from, int to) {
    +      this.to.copyFrom(from, to, UnionVector.this);
    +    }
    +  }
    +
    +  @Override
    +  public Accessor getAccessor() {
    +    return accessor;
    +  }
    +
    +  @Override
    +  public Mutator getMutator() {
    +    return mutator;
    +  }
    +
    +  @Override
    +  public FieldReader getReader() {
    +    if (reader == null) {
    +      reader = new UnionReader(this);
    +    }
    +    return reader;
    +  }
    +
    +  public FieldWriter getWriter() {
    +    if (mutator.writer == null) {
    +      mutator.writer = new UnionWriter(this);
    +    }
    +    return mutator.writer;
    +  }
    +
    +  @Override
    +  public UserBitShared.SerializedField getMetadata() {
    +    SerializedField.Builder b = getField() //
    +            .getAsBuilder() //
    +            .setBufferLength(getBufferSize()) //
    +            .setValueCount(valueCount);
    +
    +    b.addChild(internalMap.getMetadata());
    +    return b.build();
    +  }
    +
    +  @Override
    +  public int getBufferSize() {
    +    return internalMap.getBufferSize();
    +  }
    +
    +  @Override
    +  public int getBufferSizeFor(final int valueCount) {
    +    if (valueCount == 0) {
    +      return 0;
    +    }
    +
    +    long bufferSize = 0;
    +    for (final ValueVector v : (Iterable<ValueVector>) this) {
    +      bufferSize += v.getBufferSizeFor(valueCount);
    +    }
    +
    +    return (int) bufferSize;
    +  }
    +
    +  @Override
    +  public DrillBuf[] getBuffers(boolean clear) {
    +    return internalMap.getBuffers(clear);
    +  }
    +
    +  @Override
    +  public void load(UserBitShared.SerializedField metadata, DrillBuf buffer) {
    +    valueCount = metadata.getValueCount();
    +
    +    internalMap.load(metadata.getChild(0), buffer);
    +  }
    +
    +  @Override
    +  public Iterator<ValueVector> iterator() {
    +    List<ValueVector> vectors = Lists.newArrayList(internalMap.iterator());
    +    vectors.add(typeVector);
    +    return vectors.iterator();
    +  }
    +
    +  public class Accessor extends BaseValueVector.BaseAccessor {
    +
    +
    +    @Override
    +    public Object getObject(int index) {
    +      int type = typeVector.getAccessor().get(index);
    +      switch (type) {
    +      case 0:
    +        return null;
    +      <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
    +      <#assign fields = minor.fields!type.fields />
    +      <#assign uncappedName = name?uncap_first/>
    +      <#if !minor.class?starts_with("Decimal")>
    +      case MinorType.${name?upper_case}_VALUE:
    +        return get${name}Vector().getAccessor().getObject(index);
    +      </#if>
    +
    +      </#list></#list>
    +      case MinorType.MAP_VALUE:
    +        return getMap().getAccessor().getObject(index);
    +      case MinorType.LIST_VALUE:
    +        return getList().getAccessor().getObject(index);
    +      default:
    +        throw new UnsupportedOperationException("Cannot support type: " + MinorType.valueOf(type));
    +      }
    +    }
    +
    +    public byte[] get(int index) {
    +      return null;
    +    }
    +
    +    public void get(int index, ComplexHolder holder) {
    +    }
    +
    +    public void get(int index, UnionHolder holder) {
    +      if (reader == null) {
    +        reader = new UnionReader(UnionVector.this);
    +      }
    +      reader.setPosition(index);
    +      holder.reader = reader;
    +    }
    +
    +    @Override
    +    public int getValueCount() {
    +      return valueCount;
    +    }
    +
    +    @Override
    +    public boolean isNull(int index) {
    +      return typeVector.getAccessor().get(index) == 0;
    +    }
    +
    +    public int isSet(int index) {
    +      return isNull(index) ? 0 : 1;
    +    }
    +  }
    +
    +  public class Mutator extends BaseValueVector.BaseMutator {
    +
    +    UnionWriter writer;
    +
    +    @Override
    +    public void setValueCount(int valueCount) {
    +      UnionVector.this.valueCount = valueCount;
    +      internalMap.getMutator().setValueCount(valueCount);
    +    }
    +
    +    public void set(int index, byte[] bytes) {
    +    }
    +
    +    public void setSafe(int index, UnionHolder holder) {
    +      FieldReader reader = holder.reader;
    --- End diff --
    
    Let's add all the individual set methods and then have this delegate to those.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472231
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/UnionFunctions.java ---
    @@ -0,0 +1,184 @@
    +/**
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.expr.fn.impl;
    +
    +import io.netty.buffer.DrillBuf;
    +import org.apache.drill.common.types.TypeProtos.MinorType;
    +import org.apache.drill.exec.expr.DrillSimpleFunc;
    +import org.apache.drill.exec.expr.annotations.FunctionTemplate;
    +import org.apache.drill.exec.expr.annotations.FunctionTemplate.NullHandling;
    +import org.apache.drill.exec.expr.annotations.Output;
    +import org.apache.drill.exec.expr.annotations.Param;
    +import org.apache.drill.exec.expr.holders.BigIntHolder;
    +import org.apache.drill.exec.expr.holders.BitHolder;
    +import org.apache.drill.exec.expr.holders.NullableIntHolder;
    +import org.apache.drill.exec.expr.holders.NullableUInt1Holder;
    +import org.apache.drill.exec.expr.holders.UnionHolder;
    +import org.apache.drill.exec.expr.holders.IntHolder;
    +import org.apache.drill.exec.expr.holders.VarCharHolder;
    +import org.apache.drill.exec.vector.complex.impl.UnionReader;
    +import org.apache.drill.exec.vector.complex.reader.FieldReader;
    +
    +import javax.inject.Inject;
    +
    +public class UnionFunctions {
    +
    +  @FunctionTemplate(names = {"typeOf"},
    +          scope = FunctionTemplate.FunctionScope.SIMPLE,
    +          nulls = NullHandling.INTERNAL)
    +  public static class GetType implements DrillSimpleFunc {
    +
    +    @Param
    +    FieldReader input;
    +    @Output
    +    VarCharHolder out;
    +    @Inject
    +    DrillBuf buf;
    +
    +    public void setup() {}
    +
    +    public void eval() {
    +
    +      byte[] type;
    +      if (input.isSet()) {
    +         type = input.getType().getMinorType().name().getBytes();
    +      } else {
    +        type = org.apache.drill.common.types.TypeProtos.MinorType.NULL.name().getBytes();
    +      }
    +      buf = buf.reallocIfNeeded(type.length);
    +      buf.setBytes(0, type);
    +      out.buffer = buf;
    +      out.start = 0;
    +      out.end = type.length;
    +    }
    +  }
    +
    +  @SuppressWarnings("unused")
    +  @FunctionTemplate(names = {"castUNION", "castToUnion"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls=NullHandling.NULL_IF_NULL)
    +  public static class CastUnionToUnion implements DrillSimpleFunc{
    +
    +    @Param FieldReader in;
    +    @Output
    +    UnionHolder out;
    +
    +    public void setup() {}
    +
    +    public void eval() {
    +      out.reader = in;
    +      out.isSet = in.isSet() ? 1 : 0;
    +    }
    +  }
    +
    +  @SuppressWarnings("unused")
    +  @FunctionTemplate(name = "asList", scope = FunctionTemplate.FunctionScope.SIMPLE, nulls=NullHandling.INTERNAL)
    +  public static class CastUnionList implements DrillSimpleFunc {
    +
    +    @Param UnionHolder in;
    +    @Output UnionHolder out;
    +
    +    public void setup() {}
    +
    +    public void eval() {
    +      if (in.isSet == 1) {
    +        out.reader = in.reader;
    +      } else {
    +        out.isSet = 0;
    +      }
    +    }
    +  }
    +
    +  @SuppressWarnings("unused")
    +  @FunctionTemplate(name = "is_LIST", scope = FunctionTemplate.FunctionScope.SIMPLE, nulls=NullHandling.INTERNAL)
    +  public static class UnionIsList implements DrillSimpleFunc {
    +
    +    @Param UnionHolder in;
    +    @Output BitHolder out;
    +
    +    public void setup() {}
    +
    +    public void eval() {
    +      if (in.isSet == 1) {
    +        out.value = in.getType().getMinorType() == org.apache.drill.common.types.TypeProtos.MinorType.LIST ? 1 : 0;
    +      } else {
    +        out.value = 0;
    +      }
    +    }
    +  }
    +
    +  @SuppressWarnings("unused")
    +  @FunctionTemplate(name = "asMap", scope = FunctionTemplate.FunctionScope.SIMPLE, nulls=NullHandling.INTERNAL)
    +  public static class CastUnionMap implements DrillSimpleFunc {
    +
    +    @Param UnionHolder in;
    +    @Output UnionHolder out;
    +
    +    public void setup() {}
    +
    +    public void eval() {
    +      if (in.isSet == 1) {
    +        out.reader = in.reader;
    --- End diff --
    
    This seems like it should do checking rather than just convert. Also, shouldn't this be assert_map to be consistent with other functions?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471602
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java ---
    @@ -242,7 +244,7 @@
           Long.MAX_VALUE, 60 * 1000 * 5);
     
       public static final String ENABLE_VERBOSE_ERRORS_KEY = "exec.errors.verbose";
    -  public static final OptionValidator ENABLE_VERBOSE_ERRORS = new BooleanValidator(ENABLE_VERBOSE_ERRORS_KEY, false);
    +  public static final OptionValidator ENABLE_VERBOSE_ERRORS = new BooleanValidator(ENABLE_VERBOSE_ERRORS_KEY, true);
    --- End diff --
    
    accidental change?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471737
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java ---
    @@ -308,10 +334,115 @@ public LogicalExpression visitFunctionCall(FunctionCall call, FunctionLookupCont
             return matchedNonDrillFuncHolder.getExpr(call.getName(), extArgsWithCast, call.getPosition());
           }
     
    +      if (hasUnionInput(call)) {
    +        return rewriteUnionFunction(call, functionLookupContext);
    +      }
    +
           logFunctionResolutionError(errorCollector, call);
           return NullExpression.INSTANCE;
         }
     
    +    private static final Set<String> UNION_FUNCTIONS;
    +
    +    static {
    +      UNION_FUNCTIONS = new HashSet<>();
    +      for (MinorType t : MinorType.values()) {
    +        UNION_FUNCTIONS.add("assert_" + t.name().toLowerCase());
    +        UNION_FUNCTIONS.add("is_" + t.name().toLowerCase());
    +      }
    +      UNION_FUNCTIONS.add("typeof");
    +    }
    +
    +    private boolean hasUnionInput(FunctionCall call) {
    +      for (LogicalExpression arg : call.args) {
    +        if (arg.getMajorType().getMinorType() == MinorType.UNION) {
    +          return true;
    +        }
    +      }
    +      return false;
    +    }
    +
    +    private LogicalExpression rewriteUnionFunction(FunctionCall call, FunctionLookupContext functionLookupContext) {
    --- End diff --
    
    javadoc please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471861
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java ---
    @@ -308,10 +334,115 @@ public LogicalExpression visitFunctionCall(FunctionCall call, FunctionLookupCont
             return matchedNonDrillFuncHolder.getExpr(call.getName(), extArgsWithCast, call.getPosition());
           }
     
    +      if (hasUnionInput(call)) {
    +        return rewriteUnionFunction(call, functionLookupContext);
    +      }
    +
           logFunctionResolutionError(errorCollector, call);
           return NullExpression.INSTANCE;
         }
     
    +    private static final Set<String> UNION_FUNCTIONS;
    +
    +    static {
    +      UNION_FUNCTIONS = new HashSet<>();
    +      for (MinorType t : MinorType.values()) {
    +        UNION_FUNCTIONS.add("assert_" + t.name().toLowerCase());
    +        UNION_FUNCTIONS.add("is_" + t.name().toLowerCase());
    +      }
    +      UNION_FUNCTIONS.add("typeof");
    +    }
    +
    +    private boolean hasUnionInput(FunctionCall call) {
    +      for (LogicalExpression arg : call.args) {
    +        if (arg.getMajorType().getMinorType() == MinorType.UNION) {
    +          return true;
    +        }
    +      }
    +      return false;
    +    }
    +
    +    private LogicalExpression rewriteUnionFunction(FunctionCall call, FunctionLookupContext functionLookupContext) {
    +      LogicalExpression[] args = new LogicalExpression[call.args.size()];
    +      call.args.toArray(args);
    +
    +      for (int i = 0; i < args.length; i++) {
    +        LogicalExpression arg = call.args.get(i);
    +        MajorType majorType = arg.getMajorType();
    +
    +        if (majorType.getMinorType() != MinorType.UNION) {
    +          continue;
    +        }
    +
    +        List<MinorType> subTypes = majorType.getSubTypeList();
    +        Preconditions.checkState(subTypes.size() > 0, "Union type has no subtypes");
    +
    +        Queue<IfCondition> ifConditions = Lists.newLinkedList();
    +
    +        for (MinorType minorType : subTypes) {
    +          LogicalExpression ifCondition = getIsTypeExpressionForType(minorType, arg.accept(new CloneVisitor(), null));
    +          args[i] = getUnionCastExpressionForType(minorType, arg.accept(new CloneVisitor(), null));
    +
    +          List<LogicalExpression> newArgs = Lists.newArrayList();
    +          for (LogicalExpression e : args) {
    +            newArgs.add(e.accept(new CloneVisitor(), null));
    +          }
    +
    +          errorCollectors.push(errorCollector);
    +          errorCollector = new ErrorCollectorImpl();
    +
    +          LogicalExpression thenExpression = new FunctionCall(call.getName(), newArgs, call.getPosition()).accept(this, functionLookupContext);
    +
    +          if (errorCollector.hasErrors()) {
    +            thenExpression = getExceptionFunction(errorCollector.toErrorString());
    +          }
    +
    +          errorCollector = errorCollectors.pop();
    +
    +          IfExpression.IfCondition condition = new IfCondition(ifCondition, thenExpression);
    +          ifConditions.add(condition);
    +        }
    +
    +        LogicalExpression ifExpression = ifConditions.poll().expression;
    +
    +        while (!ifConditions.isEmpty()) {
    +          ifExpression = IfExpression.newBuilder().setIfCondition(ifConditions.poll()).setElse(ifExpression).build();
    +        }
    +
    +        args[i] = ifExpression;
    +        return ifExpression.accept(this, functionLookupContext);
    +      }
    +      throw new UnsupportedOperationException("Did not find any Union input types");
    +    }
    +
    +    private LogicalExpression getExceptionFunction(String message) {
    +      QuotedString msg = new QuotedString(message, ExpressionPosition.UNKNOWN);
    +      List<LogicalExpression> args = Lists.newArrayList();
    +      args.add(msg);
    +      FunctionCall call = new FunctionCall(ExceptionFunction.EXCEPTION_FUNCTION_NAME, args, ExpressionPosition.UNKNOWN);
    +      return call;
    +    }
    +
    +    private LogicalExpression getUnionCastExpressionForType(MinorType type, LogicalExpression arg) {
    +      if (type == MinorType.UNION) {
    +        return arg;
    +      }
    +      if (type == MinorType.LIST || type == MinorType.MAP) {
    +        return getExceptionFunction("Unable to cast union to " + type);
    +      }
    +      String castFuncName = String.format("assert_%s", type.toString());
    +      List<LogicalExpression> args = Lists.newArrayList();
    +      args.add(arg);
    --- End diff --
    
    Probably just use Collections.singletonList() in call.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471193
  
    --- Diff: exec/java-exec/src/main/codegen/templates/UnionFunctions.java ---
    @@ -0,0 +1,121 @@
    +/**
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.
    + */
    +
    +<@pp.dropOutputFile />
    +<@pp.changeOutputFile name="/org/apache/drill/exec/expr/fn/impl/GUnionFunctions.java" />
    +
    +
    +<#include "/@includes/license.ftl" />
    +
    +package org.apache.drill.exec.expr.fn.impl;
    +
    +<#include "/@includes/vv_imports.ftl" />
    +import org.apache.drill.exec.expr.DrillSimpleFunc;
    +import org.apache.drill.exec.expr.annotations.FunctionTemplate;
    +import org.apache.drill.exec.expr.annotations.FunctionTemplate.NullHandling;
    +import org.apache.drill.exec.expr.annotations.Output;
    +import org.apache.drill.exec.expr.annotations.Param;
    +import org.apache.drill.exec.expr.holders.*;
    +import javax.inject.Inject;
    +import io.netty.buffer.DrillBuf;
    +import org.apache.drill.exec.record.RecordBatch;
    +
    +/*
    + * This class is generated using freemarker and the ${.template_name} template.
    + */
    +
    +@SuppressWarnings("unused")
    +public class GUnionFunctions {
    +
    +  <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
    +  <#assign fields = minor.fields!type.fields />
    +  <#assign uncappedName = name?uncap_first/>
    +
    +  <#if !minor.class?starts_with("Decimal")>
    +
    +  @SuppressWarnings("unused")
    +  @FunctionTemplate(name = "is_${name?upper_case}", scope = FunctionTemplate.FunctionScope.SIMPLE, nulls=NullHandling.INTERNAL)
    +  public static class UnionIs${name} implements DrillSimpleFunc {
    +
    +    @Param UnionHolder in;
    +    @Output BitHolder out;
    +
    +    public void setup() {}
    +
    +    public void eval() {
    +      if (in.isSet == 1) {
    +        out.value = in.getType().getMinorType() == org.apache.drill.common.types.TypeProtos.MinorType.${name?upper_case} ? 1 : 0;
    +      } else {
    +        out.value = 0;
    +      }
    +    }
    +  }
    +
    +  @SuppressWarnings("unused")
    +  @FunctionTemplate(name = "assert_${name?upper_case}", scope = FunctionTemplate.FunctionScope.SIMPLE, nulls=NullHandling.INTERNAL)
    +  public static class CastUnion${name} implements DrillSimpleFunc {
    +
    +    @Param UnionHolder in;
    +    @Output Nullable${name}Holder out;
    +
    +    public void setup() {}
    +
    +    public void eval() {
    +      if (in.isSet == 1) {
    +        in.reader.read(out);
    --- End diff --
    
    What happens if the reader doesn't support that type. Do we get an error? If so, is that a nice exception?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472878
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/impl/UnionListReader.java ---
    @@ -0,0 +1,101 @@
    +/*******************************************************************************
    +
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.vector.complex.impl;
    +
    +import org.apache.drill.common.types.TypeProtos.MajorType;
    +import org.apache.drill.common.types.TypeProtos.MinorType;
    +import org.apache.drill.common.types.Types;
    +import org.apache.drill.exec.expr.holders.UnionHolder;
    +import org.apache.drill.exec.vector.UInt4Vector;
    +import org.apache.drill.exec.vector.ValueVector;
    +import org.apache.drill.exec.vector.complex.ListVector;
    +import org.apache.drill.exec.vector.complex.reader.FieldReader;
    +import org.apache.drill.exec.vector.complex.writer.BaseWriter;
    +import org.apache.drill.exec.vector.complex.writer.BaseWriter.ListWriter;
    +import org.apache.drill.exec.vector.complex.writer.BaseWriter.MapWriter;
    +import org.apache.drill.exec.vector.complex.writer.FieldWriter;
    +
    +public class UnionListReader extends AbstractFieldReader {
    --- End diff --
    
    doc it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472731
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/ListVector.java ---
    @@ -0,0 +1,390 @@
    +/*******************************************************************************
    +
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.vector.complex;
    +
    +import com.google.common.collect.ObjectArrays;
    +import io.netty.buffer.DrillBuf;
    +import org.apache.drill.common.expression.FieldReference;
    +import org.apache.drill.common.expression.PathSegment;
    +import org.apache.drill.common.types.TypeProtos.DataMode;
    +import org.apache.drill.common.types.TypeProtos.MinorType;
    +import org.apache.drill.common.types.Types;
    +import org.apache.drill.exec.memory.BufferAllocator;
    +import org.apache.drill.exec.memory.OutOfMemoryRuntimeException;
    +import org.apache.drill.exec.proto.UserBitShared;
    +import org.apache.drill.exec.record.MaterializedField;
    +import org.apache.drill.exec.record.TransferPair;
    +import org.apache.drill.exec.record.TypedFieldId;
    +import org.apache.drill.exec.util.CallBack;
    +import org.apache.drill.exec.util.JsonStringArrayList;
    +import org.apache.drill.exec.vector.AddOrGetResult;
    +import org.apache.drill.exec.vector.BaseValueVector;
    +import org.apache.drill.exec.vector.UInt1Vector;
    +import org.apache.drill.exec.vector.UInt4Vector;
    +import org.apache.drill.exec.vector.ValueVector;
    +import org.apache.drill.exec.vector.VectorDescriptor;
    +import org.apache.drill.exec.vector.ZeroVector;
    +import org.apache.drill.exec.vector.complex.impl.ComplexCopier;
    +import org.apache.drill.exec.vector.complex.impl.UnionListReader;
    +import org.apache.drill.exec.vector.complex.impl.UnionListWriter;
    +import org.apache.drill.exec.vector.complex.impl.UnionVector;
    +import org.apache.drill.exec.vector.complex.reader.FieldReader;
    +import org.apache.drill.exec.vector.complex.writer.FieldWriter;
    +
    +import java.util.List;
    +
    +public class ListVector extends BaseRepeatedValueVector {
    +
    +  private UInt4Vector offsets;
    +  private final UInt1Vector bits;
    +  private Mutator mutator = new Mutator();
    +  private Accessor accessor = new Accessor();
    +  private UnionListWriter writer;
    +  private UnionListReader reader;
    +  private CallBack callBack;
    +
    +  public ListVector(MaterializedField field, BufferAllocator allocator, CallBack callBack) {
    +    super(field, allocator);
    +    this.bits = new UInt1Vector(MaterializedField.create("$bits$", Types.required(MinorType.UINT1)), allocator);
    +    offsets = getOffsetVector();
    +    this.field.addChild(getDataVector().getField());
    +    this.writer = new UnionListWriter(this);
    +    this.reader = new UnionListReader(this);
    +    this.callBack = callBack;
    +  }
    +
    +  public UnionListWriter getWriter() {
    +    return writer;
    +  }
    +
    +  @Override
    +  public void allocateNew() throws OutOfMemoryRuntimeException {
    +    super.allocateNewSafe();
    +  }
    +
    +  public void transferTo(ListVector target) {
    +    offsets.makeTransferPair(target.offsets).transfer();
    +    bits.makeTransferPair(target.bits).transfer();
    +    if (target.getDataVector() instanceof ZeroVector) {
    +      target.addOrGetVector(new VectorDescriptor(vector.getField().getType()));
    +    }
    +    getDataVector().makeTransferPair(target.getDataVector()).transfer();
    +  }
    +
    +  public void copyFromSafe(int inIndex, int outIndex, ListVector from) {
    +    copyFrom(inIndex, outIndex, from);
    +  }
    +
    +  public void copyFrom(int inIndex, int outIndex, ListVector from) {
    +    FieldReader in = from.getReader();
    +    in.setPosition(inIndex);
    +    FieldWriter out = getWriter();
    +    out.setPosition(outIndex);
    +    ComplexCopier copier = new ComplexCopier(in, out);
    +    copier.write();
    +  }
    +
    +  @Override
    +  public ValueVector getDataVector() {
    +    return vector;
    +  }
    +
    +  @Override
    +  public TransferPair getTransferPair(FieldReference ref) {
    +    return new TransferImpl(field.withPath(ref));
    +  }
    +
    +  @Override
    +  public TransferPair makeTransferPair(ValueVector target) {
    +    return new TransferImpl((ListVector) target);
    +  }
    +
    +  private class TransferImpl implements TransferPair {
    +
    +    ListVector to;
    +
    +    public TransferImpl(MaterializedField field) {
    +      to = new ListVector(field, allocator, null);
    +      to.addOrGetVector(new VectorDescriptor(vector.getField().getType()));
    +    }
    +
    +    public TransferImpl(ListVector to) {
    +      this.to = to;
    +      to.addOrGetVector(new VectorDescriptor(vector.getField().getType()));
    +    }
    +
    +    @Override
    +    public void transfer() {
    +      transferTo(to);
    +    }
    +
    +    @Override
    +    public void splitAndTransfer(int startIndex, int length) {
    +
    +    }
    +
    +    @Override
    +    public ValueVector getTo() {
    +      return to;
    +    }
    +
    +    @Override
    +    public void copyValueSafe(int from, int to) {
    +      this.to.copyFrom(from, to, ListVector.this);
    +    }
    +  }
    +
    +  @Override
    +  public Accessor getAccessor() {
    +    return accessor;
    +  }
    +
    +  @Override
    +  public Mutator getMutator() {
    +    return mutator;
    +  }
    +
    +  @Override
    +  public FieldReader getReader() {
    +    return reader;
    +  }
    +
    +  @Override
    +  public boolean allocateNewSafe() {
    +    /* boolean to keep track if all the memory allocation were successful
    +     * Used in the case of composite vectors when we need to allocate multiple
    +     * buffers for multiple vectors. If one of the allocations failed we need to
    +     * clear all the memory that we allocated
    +     */
    +    boolean success = false;
    +    try {
    +      if (!offsets.allocateNewSafe()) {
    +        return false;
    +      }
    +      success = vector.allocateNewSafe();
    +      success = success && bits.allocateNewSafe();
    +    } finally {
    +      if (!success) {
    +        clear();
    +      }
    +    }
    +    offsets.zeroVector();
    +    bits.zeroVector();
    +    return success;
    +  }
    +
    +  @Override
    +  protected UserBitShared.SerializedField.Builder getMetadataBuilder() {
    +    return getField().getAsBuilder()
    +            .setValueCount(getAccessor().getValueCount())
    +            .setBufferLength(getBufferSize())
    +            .addChild(offsets.getMetadata())
    +            .addChild(bits.getMetadata())
    +            .addChild(vector.getMetadata());
    +  }
    +  public <T extends ValueVector> AddOrGetResult<T> addOrGetVector(VectorDescriptor descriptor) {
    +    AddOrGetResult<T> result = super.addOrGetVector(descriptor);
    +    reader = new UnionListReader(this);
    +    return result;
    +  }
    +
    +  @Override
    +  public int getBufferSize() {
    +    if (getAccessor().getValueCount() == 0) {
    +      return 0;
    +    }
    +    return offsets.getBufferSize() + bits.getBufferSize() + vector.getBufferSize();
    +  }
    +
    +  @Override
    +  public void clear() {
    +    offsets.clear();
    +    vector.clear();
    +    bits.clear();
    +    lastSet = 0;
    +    super.clear();
    +  }
    +
    +  @Override
    +  public DrillBuf[] getBuffers(boolean clear) {
    +    final DrillBuf[] buffers = ObjectArrays.concat(offsets.getBuffers(false), ObjectArrays.concat(bits.getBuffers(false),
    +            vector.getBuffers(false), DrillBuf.class), DrillBuf.class);
    +    if (clear) {
    +      for (DrillBuf buffer:buffers) {
    +        buffer.retain();
    +      }
    +      clear();
    +    }
    +    return buffers;
    +  }
    +
    +  @Override
    +  public void load(UserBitShared.SerializedField metadata, DrillBuf buffer) {
    +    final UserBitShared.SerializedField offsetMetadata = metadata.getChild(0);
    +    offsets.load(offsetMetadata, buffer);
    +
    +    final int offsetLength = offsetMetadata.getBufferLength();
    +    final UserBitShared.SerializedField bitMetadata = metadata.getChild(1);
    +    final int bitLength = bitMetadata.getBufferLength();
    +    bits.load(bitMetadata, buffer.slice(offsetLength, bitLength));
    +
    +    final UserBitShared.SerializedField vectorMetadata = metadata.getChild(2);
    +    if (getDataVector() == DEFAULT_DATA_VECTOR) {
    +      addOrGetVector(VectorDescriptor.create(vectorMetadata.getMajorType()));
    +    }
    +
    +    final int vectorLength = vectorMetadata.getBufferLength();
    +    vector.load(vectorMetadata, buffer.slice(offsetLength + bitLength, vectorLength));
    +  }
    +
    +  public UnionVector promoteToUnion() {
    +    MaterializedField newField = MaterializedField.create(getField().getPath(), Types.optional(MinorType.UNION));
    +    UnionVector vector = new UnionVector(newField, allocator, null);
    +    replaceDataVector(vector);
    +    reader = new UnionListReader(this);
    +    return vector;
    +  }
    +
    +  public TypedFieldId getFieldIdIfMatches(TypedFieldId.Builder builder, boolean addToBreadCrumb, PathSegment seg) {
    +    if (seg == null) {
    +      if (addToBreadCrumb) {
    +        builder.intermediateType(this.getField().getType());
    +      }
    +      return builder.finalType(this.getField().getType()).build();
    +    }
    +
    +    if (seg.isArray()) {
    --- End diff --
    
    This seems to duplicate code elsewhere. Can it be refactored into shared location?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471136
  
    --- Diff: exec/java-exec/src/main/codegen/templates/UnionFunctions.java ---
    @@ -0,0 +1,121 @@
    +/**
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.
    + */
    +
    +<@pp.dropOutputFile />
    +<@pp.changeOutputFile name="/org/apache/drill/exec/expr/fn/impl/GUnionFunctions.java" />
    +
    +
    +<#include "/@includes/license.ftl" />
    +
    +package org.apache.drill.exec.expr.fn.impl;
    +
    +<#include "/@includes/vv_imports.ftl" />
    +import org.apache.drill.exec.expr.DrillSimpleFunc;
    +import org.apache.drill.exec.expr.annotations.FunctionTemplate;
    +import org.apache.drill.exec.expr.annotations.FunctionTemplate.NullHandling;
    +import org.apache.drill.exec.expr.annotations.Output;
    +import org.apache.drill.exec.expr.annotations.Param;
    +import org.apache.drill.exec.expr.holders.*;
    +import javax.inject.Inject;
    +import io.netty.buffer.DrillBuf;
    +import org.apache.drill.exec.record.RecordBatch;
    +
    +/*
    + * This class is generated using freemarker and the ${.template_name} template.
    + */
    +
    +@SuppressWarnings("unused")
    +public class GUnionFunctions {
    +
    +  <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
    +  <#assign fields = minor.fields!type.fields />
    +  <#assign uncappedName = name?uncap_first/>
    +
    +  <#if !minor.class?starts_with("Decimal")>
    +
    +  @SuppressWarnings("unused")
    +  @FunctionTemplate(name = "is_${name?upper_case}", scope = FunctionTemplate.FunctionScope.SIMPLE, nulls=NullHandling.INTERNAL)
    --- End diff --
    
    let's make IS instead of is just so is consistent.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by StevenMPhillips <gi...@git.apache.org>.
Github user StevenMPhillips commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43479145
  
    --- Diff: exec/java-exec/src/main/codegen/templates/NullReader.java ---
    @@ -56,11 +56,17 @@ public void copyAsValue(MapWriter writer) {}
     
       public void copyAsValue(ListWriter writer) {}
     
    -  <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first /> 
    +  public void copyAsValue(UnionWriter writer) {}
    +
    +  <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
    +  public void read(${name}Holder holder){
    +    throw new UnsupportedOperationException("NullReader cannot read into non-nullable holder");
    --- End diff --
    
    This is an internal error message, not the result of a potential user error.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471567
  
    --- Diff: exec/java-exec/src/main/codegen/templates/UnionVector.java ---
    @@ -0,0 +1,479 @@
    +/**
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.
    + */
    +
    +import org.apache.drill.common.types.TypeProtos.MinorType;
    +
    +<@pp.dropOutputFile />
    +<@pp.changeOutputFile name="/org/apache/drill/exec/vector/complex/impl/UnionVector.java" />
    +
    +
    +<#include "/@includes/license.ftl" />
    +
    +package org.apache.drill.exec.vector.complex.impl;
    +
    +<#include "/@includes/vv_imports.ftl" />
    +import java.util.Iterator;
    +import org.apache.drill.exec.vector.complex.impl.ComplexCopier;
    +import org.apache.drill.exec.util.CallBack;
    +
    +/*
    + * This class is generated using freemarker and the ${.template_name} template.
    + */
    +@SuppressWarnings("unused")
    +
    +
    +public class UnionVector implements ValueVector {
    +
    +  private MaterializedField field;
    +  private BufferAllocator allocator;
    +  private Accessor accessor = new Accessor();
    +  private Mutator mutator = new Mutator();
    +  private int valueCount;
    +
    +  private MapVector internalMap;
    +  private SingleMapWriter internalMapWriter;
    +  private UInt1Vector typeVector;
    +
    +  private MapVector mapVector;
    +  private ListVector listVector;
    +  private NullableBigIntVector bigInt;
    +  private NullableVarCharVector varChar;
    +
    +  private FieldReader reader;
    +  private NullableBitVector bit;
    +
    +  private State state = State.INIT;
    +  private int singleType = 0;
    +  private ValueVector singleVector;
    +  private MajorType majorType;
    +
    +  private final CallBack callBack;
    +
    +  private enum State {
    +    INIT, SINGLE, MULTI
    +  }
    +
    +  public UnionVector(MaterializedField field, BufferAllocator allocator, CallBack callBack) {
    +    this.field = field.clone();
    +    this.allocator = allocator;
    +    internalMap = new MapVector("internal", allocator, callBack);
    +    internalMapWriter = new SingleMapWriter(internalMap, null, true, true);
    +    this.typeVector = internalMap.addOrGet("types", Types.required(MinorType.UINT1), UInt1Vector.class);
    +    this.field.addChild(internalMap.getField().clone());
    +    this.majorType = field.getType();
    +    this.callBack = callBack;
    +  }
    +
    +  private void updateState(ValueVector v) {
    +    if (state == State.INIT) {
    +      state = State.SINGLE;
    +      singleVector = v;
    +      singleType = v.getField().getType().getMinorType().getNumber();
    +    } else {
    +      state = State.MULTI;
    +      singleVector = null;
    +    }
    +  }
    +
    +  public List<MinorType> getSubTypes() {
    +    return majorType.getSubTypeList();
    +  }
    +
    +  private void addSubType(MinorType type) {
    +    majorType =  MajorType.newBuilder(this.majorType).addSubType(type).build();
    +    if (callBack != null) {
    +      callBack.doWork();
    +    }
    +  }
    +
    +  public boolean isSingleType() {
    +    return state == State.SINGLE && singleType != MinorType.LIST_VALUE;
    +  }
    +
    +  public ValueVector getSingleVector() {
    +    assert state != State.MULTI : "Cannot get single vector when there are multiple types";
    +    assert state != State.INIT : "Cannot get single vector when there are no types";
    +    return singleVector;
    +  }
    +
    +  private static final MajorType MAP_TYPE = Types.optional(MinorType.MAP);
    +
    +  public MapVector getMap() {
    +    if (mapVector == null) {
    +      int vectorCount = internalMap.size();
    +      mapVector = internalMap.addOrGet("map", MAP_TYPE, MapVector.class);
    +      updateState(mapVector);
    +      addSubType(MinorType.MAP);
    +      if (internalMap.size() > vectorCount) {
    +        mapVector.allocateNew();
    +      }
    +    }
    +    return mapVector;
    +  }
    +
    +  <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
    +  <#assign fields = minor.fields!type.fields />
    +  <#assign uncappedName = name?uncap_first/>
    +  <#if !minor.class?starts_with("Decimal")>
    +
    +  private Nullable${name}Vector ${uncappedName}Vector;
    +  private static final MajorType ${name?upper_case}_TYPE = Types.optional(MinorType.${name?upper_case});
    +
    +  public Nullable${name}Vector get${name}Vector() {
    +    if (${uncappedName}Vector == null) {
    +      int vectorCount = internalMap.size();
    +      ${uncappedName}Vector = internalMap.addOrGet("${uncappedName}", ${name?upper_case}_TYPE, Nullable${name}Vector.class);
    +      updateState(${uncappedName}Vector);
    +      addSubType(MinorType.${name?upper_case});
    +      if (internalMap.size() > vectorCount) {
    +        ${uncappedName}Vector.allocateNew();
    +      }
    +    }
    +    return ${uncappedName}Vector;
    +  }
    +
    +  </#if>
    +
    +  </#list></#list>
    +
    +  private static final MajorType LIST_TYPE = Types.optional(MinorType.LIST);
    +
    +  public ListVector getList() {
    +    if (listVector == null) {
    +      int vectorCount = internalMap.size();
    +      listVector = internalMap.addOrGet("list", LIST_TYPE, ListVector.class);
    +      updateState(listVector);
    +      addSubType(MinorType.LIST);
    +      if (internalMap.size() > vectorCount) {
    +        listVector.allocateNew();
    +      }
    +    }
    +    return listVector;
    +  }
    +
    +  public int getTypeValue(int index) {
    +    return typeVector.getAccessor().get(index);
    +  }
    +
    +  public UInt1Vector getTypeVector() {
    +    return typeVector;
    +  }
    +
    +  @Override
    +  public void allocateNew() throws OutOfMemoryRuntimeException {
    +    internalMap.allocateNew();
    +    if (typeVector != null) {
    +      typeVector.zeroVector();
    +    }
    +  }
    +
    +  @Override
    +  public boolean allocateNewSafe() {
    +    boolean safe = internalMap.allocateNewSafe();
    +    if (safe) {
    +      if (typeVector != null) {
    +        typeVector.zeroVector();
    +      }
    +    }
    +    return safe;
    +  }
    +
    +  @Override
    +  public void setInitialCapacity(int numRecords) {
    +  }
    +
    +  @Override
    +  public int getValueCapacity() {
    +    return Math.min(typeVector.getValueCapacity(), internalMap.getValueCapacity());
    +  }
    +
    +  @Override
    +  public void close() {
    +  }
    +
    +  @Override
    +  public void clear() {
    +    internalMap.clear();
    +  }
    +
    +  @Override
    +  public MaterializedField getField() {
    +    return field;
    +  }
    +
    +  @Override
    +  public TransferPair getTransferPair() {
    +    return new TransferImpl(field);
    +  }
    +
    +  @Override
    +  public TransferPair getTransferPair(FieldReference ref) {
    +    return new TransferImpl(field.withPath(ref));
    +  }
    +
    +  @Override
    +  public TransferPair makeTransferPair(ValueVector target) {
    +    return new TransferImpl((UnionVector) target);
    +  }
    +
    +  public void transferTo(UnionVector target) {
    +    internalMap.makeTransferPair(target.internalMap).transfer();
    +    target.valueCount = valueCount;
    +    target.majorType = majorType;
    +  }
    +
    +  public void copyFrom(int inIndex, int outIndex, UnionVector from) {
    +    from.getReader().setPosition(inIndex);
    +    getWriter().setPosition(outIndex);
    +    ComplexCopier copier = new ComplexCopier(from.reader, mutator.writer);
    +    copier.write();
    +  }
    +
    +  public void copyFromSafe(int inIndex, int outIndex, UnionVector from) {
    +    copyFrom(inIndex, outIndex, from);
    +  }
    +
    +  public void addVector(ValueVector v) {
    +    internalMap.putChild(v.getField().getType().getMinorType().name().toLowerCase(), v);
    +    addSubType(v.getField().getType().getMinorType());
    +  }
    +
    +  private class TransferImpl implements TransferPair {
    +
    +    UnionVector to;
    +
    +    public TransferImpl(MaterializedField field) {
    +      to = new UnionVector(field, allocator, null);
    +    }
    +
    +    public TransferImpl(UnionVector to) {
    +      this.to = to;
    +    }
    +
    +    @Override
    +    public void transfer() {
    +      transferTo(to);
    +    }
    +
    +    @Override
    +    public void splitAndTransfer(int startIndex, int length) {
    +
    +    }
    +
    +    @Override
    +    public ValueVector getTo() {
    +      return to;
    +    }
    +
    +    @Override
    +    public void copyValueSafe(int from, int to) {
    +      this.to.copyFrom(from, to, UnionVector.this);
    +    }
    +  }
    +
    +  @Override
    +  public Accessor getAccessor() {
    +    return accessor;
    +  }
    +
    +  @Override
    +  public Mutator getMutator() {
    +    return mutator;
    +  }
    +
    +  @Override
    +  public FieldReader getReader() {
    +    if (reader == null) {
    +      reader = new UnionReader(this);
    +    }
    +    return reader;
    +  }
    +
    +  public FieldWriter getWriter() {
    +    if (mutator.writer == null) {
    +      mutator.writer = new UnionWriter(this);
    +    }
    +    return mutator.writer;
    +  }
    +
    +  @Override
    +  public UserBitShared.SerializedField getMetadata() {
    +    SerializedField.Builder b = getField() //
    +            .getAsBuilder() //
    +            .setBufferLength(getBufferSize()) //
    +            .setValueCount(valueCount);
    +
    +    b.addChild(internalMap.getMetadata());
    +    return b.build();
    +  }
    +
    +  @Override
    +  public int getBufferSize() {
    +    return internalMap.getBufferSize();
    +  }
    +
    +  @Override
    +  public int getBufferSizeFor(final int valueCount) {
    +    if (valueCount == 0) {
    +      return 0;
    +    }
    +
    +    long bufferSize = 0;
    +    for (final ValueVector v : (Iterable<ValueVector>) this) {
    +      bufferSize += v.getBufferSizeFor(valueCount);
    +    }
    +
    +    return (int) bufferSize;
    +  }
    +
    +  @Override
    +  public DrillBuf[] getBuffers(boolean clear) {
    +    return internalMap.getBuffers(clear);
    +  }
    +
    +  @Override
    +  public void load(UserBitShared.SerializedField metadata, DrillBuf buffer) {
    +    valueCount = metadata.getValueCount();
    +
    +    internalMap.load(metadata.getChild(0), buffer);
    +  }
    +
    +  @Override
    +  public Iterator<ValueVector> iterator() {
    +    List<ValueVector> vectors = Lists.newArrayList(internalMap.iterator());
    +    vectors.add(typeVector);
    +    return vectors.iterator();
    +  }
    +
    +  public class Accessor extends BaseValueVector.BaseAccessor {
    +
    +
    +    @Override
    +    public Object getObject(int index) {
    +      int type = typeVector.getAccessor().get(index);
    +      switch (type) {
    +      case 0:
    +        return null;
    +      <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
    +      <#assign fields = minor.fields!type.fields />
    +      <#assign uncappedName = name?uncap_first/>
    +      <#if !minor.class?starts_with("Decimal")>
    +      case MinorType.${name?upper_case}_VALUE:
    +        return get${name}Vector().getAccessor().getObject(index);
    +      </#if>
    +
    +      </#list></#list>
    +      case MinorType.MAP_VALUE:
    +        return getMap().getAccessor().getObject(index);
    +      case MinorType.LIST_VALUE:
    +        return getList().getAccessor().getObject(index);
    +      default:
    +        throw new UnsupportedOperationException("Cannot support type: " + MinorType.valueOf(type));
    +      }
    +    }
    +
    +    public byte[] get(int index) {
    +      return null;
    +    }
    +
    +    public void get(int index, ComplexHolder holder) {
    +    }
    +
    +    public void get(int index, UnionHolder holder) {
    +      if (reader == null) {
    +        reader = new UnionReader(UnionVector.this);
    +      }
    +      reader.setPosition(index);
    +      holder.reader = reader;
    +    }
    +
    +    @Override
    +    public int getValueCount() {
    +      return valueCount;
    +    }
    +
    +    @Override
    +    public boolean isNull(int index) {
    +      return typeVector.getAccessor().get(index) == 0;
    +    }
    +
    +    public int isSet(int index) {
    +      return isNull(index) ? 0 : 1;
    +    }
    +  }
    +
    +  public class Mutator extends BaseValueVector.BaseMutator {
    +
    +    UnionWriter writer;
    +
    +    @Override
    +    public void setValueCount(int valueCount) {
    +      UnionVector.this.valueCount = valueCount;
    +      internalMap.getMutator().setValueCount(valueCount);
    +    }
    +
    +    public void set(int index, byte[] bytes) {
    +    }
    --- End diff --
    
    We need to do something here...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by StevenMPhillips <gi...@git.apache.org>.
Github user StevenMPhillips commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43563951
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/UnionSqlAccessor.java ---
    @@ -0,0 +1,129 @@
    +/**
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.vector.accessor;
    +
    +import org.apache.drill.common.types.TypeProtos.MajorType;
    +import org.apache.drill.common.types.TypeProtos.MinorType;
    +import org.apache.drill.common.types.Types;
    +import org.apache.drill.exec.vector.complex.impl.UnionVector;
    +import org.apache.drill.exec.vector.complex.impl.UnionWriter;
    +import org.apache.drill.exec.vector.complex.reader.FieldReader;
    +
    +import java.io.InputStream;
    +import java.io.Reader;
    +import java.math.BigDecimal;
    +import java.sql.Date;
    +import java.sql.Time;
    +import java.sql.Timestamp;
    +
    +public class UnionSqlAccessor extends AbstractSqlAccessor {
    +
    +  FieldReader reader;
    --- End diff --
    
    JDBC functionality wasn't really a goal here, just bare minimum so that the results will display in sqlline


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by StevenMPhillips <gi...@git.apache.org>.
Github user StevenMPhillips commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43480218
  
    --- Diff: exec/java-exec/src/main/codegen/templates/UnionVector.java ---
    @@ -0,0 +1,479 @@
    +/**
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.
    + */
    +
    +import org.apache.drill.common.types.TypeProtos.MinorType;
    +
    +<@pp.dropOutputFile />
    +<@pp.changeOutputFile name="/org/apache/drill/exec/vector/complex/impl/UnionVector.java" />
    +
    +
    +<#include "/@includes/license.ftl" />
    +
    +package org.apache.drill.exec.vector.complex.impl;
    +
    +<#include "/@includes/vv_imports.ftl" />
    +import java.util.Iterator;
    +import org.apache.drill.exec.vector.complex.impl.ComplexCopier;
    +import org.apache.drill.exec.util.CallBack;
    +
    +/*
    + * This class is generated using freemarker and the ${.template_name} template.
    + */
    +@SuppressWarnings("unused")
    +
    +
    +public class UnionVector implements ValueVector {
    +
    +  private MaterializedField field;
    +  private BufferAllocator allocator;
    +  private Accessor accessor = new Accessor();
    +  private Mutator mutator = new Mutator();
    +  private int valueCount;
    +
    +  private MapVector internalMap;
    +  private SingleMapWriter internalMapWriter;
    +  private UInt1Vector typeVector;
    +
    +  private MapVector mapVector;
    +  private ListVector listVector;
    +  private NullableBigIntVector bigInt;
    +  private NullableVarCharVector varChar;
    +
    +  private FieldReader reader;
    +  private NullableBitVector bit;
    +
    +  private State state = State.INIT;
    +  private int singleType = 0;
    +  private ValueVector singleVector;
    +  private MajorType majorType;
    +
    +  private final CallBack callBack;
    +
    +  private enum State {
    +    INIT, SINGLE, MULTI
    +  }
    +
    +  public UnionVector(MaterializedField field, BufferAllocator allocator, CallBack callBack) {
    +    this.field = field.clone();
    +    this.allocator = allocator;
    +    internalMap = new MapVector("internal", allocator, callBack);
    +    internalMapWriter = new SingleMapWriter(internalMap, null, true, true);
    +    this.typeVector = internalMap.addOrGet("types", Types.required(MinorType.UINT1), UInt1Vector.class);
    +    this.field.addChild(internalMap.getField().clone());
    +    this.majorType = field.getType();
    +    this.callBack = callBack;
    +  }
    +
    +  private void updateState(ValueVector v) {
    +    if (state == State.INIT) {
    +      state = State.SINGLE;
    +      singleVector = v;
    +      singleType = v.getField().getType().getMinorType().getNumber();
    +    } else {
    +      state = State.MULTI;
    +      singleVector = null;
    +    }
    +  }
    +
    +  public List<MinorType> getSubTypes() {
    +    return majorType.getSubTypeList();
    +  }
    +
    +  private void addSubType(MinorType type) {
    +    majorType =  MajorType.newBuilder(this.majorType).addSubType(type).build();
    +    if (callBack != null) {
    +      callBack.doWork();
    +    }
    +  }
    +
    +  public boolean isSingleType() {
    +    return state == State.SINGLE && singleType != MinorType.LIST_VALUE;
    +  }
    +
    +  public ValueVector getSingleVector() {
    +    assert state != State.MULTI : "Cannot get single vector when there are multiple types";
    +    assert state != State.INIT : "Cannot get single vector when there are no types";
    +    return singleVector;
    +  }
    +
    +  private static final MajorType MAP_TYPE = Types.optional(MinorType.MAP);
    +
    +  public MapVector getMap() {
    +    if (mapVector == null) {
    +      int vectorCount = internalMap.size();
    +      mapVector = internalMap.addOrGet("map", MAP_TYPE, MapVector.class);
    +      updateState(mapVector);
    +      addSubType(MinorType.MAP);
    +      if (internalMap.size() > vectorCount) {
    +        mapVector.allocateNew();
    +      }
    +    }
    +    return mapVector;
    +  }
    +
    +  <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
    +  <#assign fields = minor.fields!type.fields />
    +  <#assign uncappedName = name?uncap_first/>
    +  <#if !minor.class?starts_with("Decimal")>
    +
    +  private Nullable${name}Vector ${uncappedName}Vector;
    +  private static final MajorType ${name?upper_case}_TYPE = Types.optional(MinorType.${name?upper_case});
    +
    +  public Nullable${name}Vector get${name}Vector() {
    +    if (${uncappedName}Vector == null) {
    +      int vectorCount = internalMap.size();
    +      ${uncappedName}Vector = internalMap.addOrGet("${uncappedName}", ${name?upper_case}_TYPE, Nullable${name}Vector.class);
    +      updateState(${uncappedName}Vector);
    +      addSubType(MinorType.${name?upper_case});
    +      if (internalMap.size() > vectorCount) {
    +        ${uncappedName}Vector.allocateNew();
    +      }
    +    }
    +    return ${uncappedName}Vector;
    +  }
    +
    +  </#if>
    +
    +  </#list></#list>
    +
    +  private static final MajorType LIST_TYPE = Types.optional(MinorType.LIST);
    +
    +  public ListVector getList() {
    +    if (listVector == null) {
    +      int vectorCount = internalMap.size();
    +      listVector = internalMap.addOrGet("list", LIST_TYPE, ListVector.class);
    +      updateState(listVector);
    +      addSubType(MinorType.LIST);
    +      if (internalMap.size() > vectorCount) {
    +        listVector.allocateNew();
    +      }
    +    }
    +    return listVector;
    +  }
    +
    +  public int getTypeValue(int index) {
    +    return typeVector.getAccessor().get(index);
    +  }
    +
    +  public UInt1Vector getTypeVector() {
    +    return typeVector;
    +  }
    +
    +  @Override
    +  public void allocateNew() throws OutOfMemoryRuntimeException {
    +    internalMap.allocateNew();
    +    if (typeVector != null) {
    +      typeVector.zeroVector();
    +    }
    +  }
    +
    +  @Override
    +  public boolean allocateNewSafe() {
    +    boolean safe = internalMap.allocateNewSafe();
    +    if (safe) {
    +      if (typeVector != null) {
    +        typeVector.zeroVector();
    +      }
    +    }
    +    return safe;
    +  }
    +
    +  @Override
    +  public void setInitialCapacity(int numRecords) {
    +  }
    +
    +  @Override
    +  public int getValueCapacity() {
    +    return Math.min(typeVector.getValueCapacity(), internalMap.getValueCapacity());
    +  }
    +
    +  @Override
    +  public void close() {
    +  }
    +
    +  @Override
    +  public void clear() {
    +    internalMap.clear();
    +  }
    +
    +  @Override
    +  public MaterializedField getField() {
    +    return field;
    +  }
    +
    +  @Override
    +  public TransferPair getTransferPair() {
    +    return new TransferImpl(field);
    +  }
    +
    +  @Override
    +  public TransferPair getTransferPair(FieldReference ref) {
    +    return new TransferImpl(field.withPath(ref));
    +  }
    +
    +  @Override
    +  public TransferPair makeTransferPair(ValueVector target) {
    +    return new TransferImpl((UnionVector) target);
    +  }
    +
    +  public void transferTo(UnionVector target) {
    +    internalMap.makeTransferPair(target.internalMap).transfer();
    +    target.valueCount = valueCount;
    +    target.majorType = majorType;
    +  }
    +
    +  public void copyFrom(int inIndex, int outIndex, UnionVector from) {
    +    from.getReader().setPosition(inIndex);
    +    getWriter().setPosition(outIndex);
    +    ComplexCopier copier = new ComplexCopier(from.reader, mutator.writer);
    --- End diff --
    
    I agree, there is no reason this shouldn't be a static method.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43470768
  
    --- Diff: common/src/main/java/org/apache/drill/common/types/Types.java ---
    @@ -34,6 +34,13 @@
       public static final MajorType REQUIRED_BIT = required(MinorType.BIT);
       public static final MajorType OPTIONAL_BIT = optional(MinorType.BIT);
     
    +  public static boolean isUnion(MajorType toType) {
    +    if (toType.getMinorType() == MinorType.UNION) {
    --- End diff --
    
    probably better as 
    
    return toType.getMinorType() == MinorType.UNION;


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by StevenMPhillips <gi...@git.apache.org>.
Github user StevenMPhillips commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43479756
  
    --- Diff: exec/java-exec/src/main/codegen/templates/UnionVector.java ---
    @@ -0,0 +1,479 @@
    +/**
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.
    + */
    +
    +import org.apache.drill.common.types.TypeProtos.MinorType;
    +
    +<@pp.dropOutputFile />
    +<@pp.changeOutputFile name="/org/apache/drill/exec/vector/complex/impl/UnionVector.java" />
    +
    +
    +<#include "/@includes/license.ftl" />
    +
    +package org.apache.drill.exec.vector.complex.impl;
    +
    +<#include "/@includes/vv_imports.ftl" />
    +import java.util.Iterator;
    +import org.apache.drill.exec.vector.complex.impl.ComplexCopier;
    +import org.apache.drill.exec.util.CallBack;
    +
    +/*
    + * This class is generated using freemarker and the ${.template_name} template.
    + */
    +@SuppressWarnings("unused")
    +
    +
    +public class UnionVector implements ValueVector {
    +
    +  private MaterializedField field;
    +  private BufferAllocator allocator;
    +  private Accessor accessor = new Accessor();
    +  private Mutator mutator = new Mutator();
    +  private int valueCount;
    +
    +  private MapVector internalMap;
    +  private SingleMapWriter internalMapWriter;
    +  private UInt1Vector typeVector;
    +
    +  private MapVector mapVector;
    +  private ListVector listVector;
    +  private NullableBigIntVector bigInt;
    +  private NullableVarCharVector varChar;
    +
    +  private FieldReader reader;
    +  private NullableBitVector bit;
    +
    +  private State state = State.INIT;
    +  private int singleType = 0;
    +  private ValueVector singleVector;
    +  private MajorType majorType;
    +
    +  private final CallBack callBack;
    +
    +  private enum State {
    +    INIT, SINGLE, MULTI
    +  }
    +
    +  public UnionVector(MaterializedField field, BufferAllocator allocator, CallBack callBack) {
    +    this.field = field.clone();
    +    this.allocator = allocator;
    +    internalMap = new MapVector("internal", allocator, callBack);
    +    internalMapWriter = new SingleMapWriter(internalMap, null, true, true);
    +    this.typeVector = internalMap.addOrGet("types", Types.required(MinorType.UINT1), UInt1Vector.class);
    +    this.field.addChild(internalMap.getField().clone());
    +    this.majorType = field.getType();
    +    this.callBack = callBack;
    +  }
    +
    +  private void updateState(ValueVector v) {
    +    if (state == State.INIT) {
    +      state = State.SINGLE;
    +      singleVector = v;
    +      singleType = v.getField().getType().getMinorType().getNumber();
    +    } else {
    +      state = State.MULTI;
    +      singleVector = null;
    +    }
    +  }
    +
    +  public List<MinorType> getSubTypes() {
    +    return majorType.getSubTypeList();
    +  }
    +
    +  private void addSubType(MinorType type) {
    +    majorType =  MajorType.newBuilder(this.majorType).addSubType(type).build();
    +    if (callBack != null) {
    +      callBack.doWork();
    +    }
    +  }
    +
    +  public boolean isSingleType() {
    +    return state == State.SINGLE && singleType != MinorType.LIST_VALUE;
    +  }
    +
    +  public ValueVector getSingleVector() {
    +    assert state != State.MULTI : "Cannot get single vector when there are multiple types";
    --- End diff --
    
    This State was originally designed to eliminate some of the overhead when there was only a single subtype, but with PromotableWriter, this is really not necessary, and this State is no longer used. So I removed this code.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43470824
  
    --- Diff: exec/java-exec/src/main/codegen/templates/AbstractFieldReader.java ---
    @@ -57,9 +61,9 @@ public void copyAsValue(MapWriter writer){
       public void copyAsField(String name, MapWriter writer){
         fail("CopyAsField MapWriter");
       }
    -  public void copyAsValue(ListWriter writer){
    -    fail("CopyAsValueList");
    -  }
    +//  public void copyAsValue(ListWriter writer){
    --- End diff --
    
    delete?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471517
  
    --- Diff: exec/java-exec/src/main/codegen/templates/UnionVector.java ---
    @@ -0,0 +1,479 @@
    +/**
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.
    + */
    +
    +import org.apache.drill.common.types.TypeProtos.MinorType;
    +
    +<@pp.dropOutputFile />
    +<@pp.changeOutputFile name="/org/apache/drill/exec/vector/complex/impl/UnionVector.java" />
    +
    +
    +<#include "/@includes/license.ftl" />
    +
    +package org.apache.drill.exec.vector.complex.impl;
    +
    +<#include "/@includes/vv_imports.ftl" />
    +import java.util.Iterator;
    +import org.apache.drill.exec.vector.complex.impl.ComplexCopier;
    +import org.apache.drill.exec.util.CallBack;
    +
    +/*
    + * This class is generated using freemarker and the ${.template_name} template.
    + */
    +@SuppressWarnings("unused")
    +
    +
    +public class UnionVector implements ValueVector {
    +
    +  private MaterializedField field;
    +  private BufferAllocator allocator;
    +  private Accessor accessor = new Accessor();
    +  private Mutator mutator = new Mutator();
    +  private int valueCount;
    +
    +  private MapVector internalMap;
    +  private SingleMapWriter internalMapWriter;
    +  private UInt1Vector typeVector;
    +
    +  private MapVector mapVector;
    +  private ListVector listVector;
    +  private NullableBigIntVector bigInt;
    +  private NullableVarCharVector varChar;
    +
    +  private FieldReader reader;
    +  private NullableBitVector bit;
    +
    +  private State state = State.INIT;
    +  private int singleType = 0;
    +  private ValueVector singleVector;
    +  private MajorType majorType;
    +
    +  private final CallBack callBack;
    +
    +  private enum State {
    +    INIT, SINGLE, MULTI
    +  }
    +
    +  public UnionVector(MaterializedField field, BufferAllocator allocator, CallBack callBack) {
    +    this.field = field.clone();
    +    this.allocator = allocator;
    +    internalMap = new MapVector("internal", allocator, callBack);
    +    internalMapWriter = new SingleMapWriter(internalMap, null, true, true);
    +    this.typeVector = internalMap.addOrGet("types", Types.required(MinorType.UINT1), UInt1Vector.class);
    +    this.field.addChild(internalMap.getField().clone());
    +    this.majorType = field.getType();
    +    this.callBack = callBack;
    +  }
    +
    +  private void updateState(ValueVector v) {
    +    if (state == State.INIT) {
    +      state = State.SINGLE;
    +      singleVector = v;
    +      singleType = v.getField().getType().getMinorType().getNumber();
    +    } else {
    +      state = State.MULTI;
    +      singleVector = null;
    +    }
    +  }
    +
    +  public List<MinorType> getSubTypes() {
    +    return majorType.getSubTypeList();
    +  }
    +
    +  private void addSubType(MinorType type) {
    +    majorType =  MajorType.newBuilder(this.majorType).addSubType(type).build();
    +    if (callBack != null) {
    +      callBack.doWork();
    +    }
    +  }
    +
    +  public boolean isSingleType() {
    +    return state == State.SINGLE && singleType != MinorType.LIST_VALUE;
    +  }
    +
    +  public ValueVector getSingleVector() {
    +    assert state != State.MULTI : "Cannot get single vector when there are multiple types";
    +    assert state != State.INIT : "Cannot get single vector when there are no types";
    +    return singleVector;
    +  }
    +
    +  private static final MajorType MAP_TYPE = Types.optional(MinorType.MAP);
    +
    +  public MapVector getMap() {
    +    if (mapVector == null) {
    +      int vectorCount = internalMap.size();
    +      mapVector = internalMap.addOrGet("map", MAP_TYPE, MapVector.class);
    +      updateState(mapVector);
    +      addSubType(MinorType.MAP);
    +      if (internalMap.size() > vectorCount) {
    +        mapVector.allocateNew();
    +      }
    +    }
    +    return mapVector;
    +  }
    +
    +  <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
    +  <#assign fields = minor.fields!type.fields />
    +  <#assign uncappedName = name?uncap_first/>
    +  <#if !minor.class?starts_with("Decimal")>
    +
    +  private Nullable${name}Vector ${uncappedName}Vector;
    +  private static final MajorType ${name?upper_case}_TYPE = Types.optional(MinorType.${name?upper_case});
    +
    +  public Nullable${name}Vector get${name}Vector() {
    +    if (${uncappedName}Vector == null) {
    +      int vectorCount = internalMap.size();
    +      ${uncappedName}Vector = internalMap.addOrGet("${uncappedName}", ${name?upper_case}_TYPE, Nullable${name}Vector.class);
    +      updateState(${uncappedName}Vector);
    +      addSubType(MinorType.${name?upper_case});
    +      if (internalMap.size() > vectorCount) {
    +        ${uncappedName}Vector.allocateNew();
    +      }
    +    }
    +    return ${uncappedName}Vector;
    +  }
    +
    +  </#if>
    +
    +  </#list></#list>
    +
    +  private static final MajorType LIST_TYPE = Types.optional(MinorType.LIST);
    +
    +  public ListVector getList() {
    +    if (listVector == null) {
    +      int vectorCount = internalMap.size();
    +      listVector = internalMap.addOrGet("list", LIST_TYPE, ListVector.class);
    +      updateState(listVector);
    +      addSubType(MinorType.LIST);
    +      if (internalMap.size() > vectorCount) {
    +        listVector.allocateNew();
    +      }
    +    }
    +    return listVector;
    +  }
    +
    +  public int getTypeValue(int index) {
    +    return typeVector.getAccessor().get(index);
    +  }
    +
    +  public UInt1Vector getTypeVector() {
    +    return typeVector;
    +  }
    +
    +  @Override
    +  public void allocateNew() throws OutOfMemoryRuntimeException {
    +    internalMap.allocateNew();
    +    if (typeVector != null) {
    +      typeVector.zeroVector();
    +    }
    +  }
    +
    +  @Override
    +  public boolean allocateNewSafe() {
    +    boolean safe = internalMap.allocateNewSafe();
    +    if (safe) {
    +      if (typeVector != null) {
    +        typeVector.zeroVector();
    +      }
    +    }
    +    return safe;
    +  }
    +
    +  @Override
    +  public void setInitialCapacity(int numRecords) {
    +  }
    +
    +  @Override
    +  public int getValueCapacity() {
    +    return Math.min(typeVector.getValueCapacity(), internalMap.getValueCapacity());
    +  }
    +
    +  @Override
    +  public void close() {
    +  }
    +
    +  @Override
    +  public void clear() {
    +    internalMap.clear();
    +  }
    +
    +  @Override
    +  public MaterializedField getField() {
    +    return field;
    +  }
    +
    +  @Override
    +  public TransferPair getTransferPair() {
    +    return new TransferImpl(field);
    +  }
    +
    +  @Override
    +  public TransferPair getTransferPair(FieldReference ref) {
    +    return new TransferImpl(field.withPath(ref));
    +  }
    +
    +  @Override
    +  public TransferPair makeTransferPair(ValueVector target) {
    +    return new TransferImpl((UnionVector) target);
    +  }
    +
    +  public void transferTo(UnionVector target) {
    +    internalMap.makeTransferPair(target.internalMap).transfer();
    +    target.valueCount = valueCount;
    +    target.majorType = majorType;
    +  }
    +
    +  public void copyFrom(int inIndex, int outIndex, UnionVector from) {
    +    from.getReader().setPosition(inIndex);
    +    getWriter().setPosition(outIndex);
    +    ComplexCopier copier = new ComplexCopier(from.reader, mutator.writer);
    --- End diff --
    
    Wondering whether we should make it that the ComplexCopier has a static method so we avoid creation here?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471654
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/EvaluationVisitor.java ---
    @@ -411,7 +413,9 @@ private HoldingContainer visitValueVectorReadExpression(ValueVectorReadExpressio
                 if (out.isOptional()) {
                   ifNoVal.assign(out.getIsSet(), JExpr.lit(0));
                 }
    -            ifNoVal.assign(isNull,  JExpr.lit(1));
    +            if (isNull != null) {
    --- End diff --
    
    comment please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by StevenMPhillips <gi...@git.apache.org>.
Github user StevenMPhillips commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43479065
  
    --- Diff: exec/java-exec/src/main/codegen/templates/MapWriters.java ---
    @@ -52,9 +52,18 @@
       private final Map<String, FieldWriter> fields = Maps.newHashMap();
       <#if mode == "Repeated">private int currentChildIndex = 0;</#if>
     
    -  public ${mode}MapWriter(${containerClass} container, FieldWriter parent) {
    +  private final boolean unionEnabled;
    +  private final boolean unionInternalMap;
    --- End diff --
    
    It's actually not needed, it is a relic of an older version. I removed it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472119
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionGenerationHelper.java ---
    @@ -127,7 +133,7 @@ protected static boolean isComparableType(MajorType type) {
     
       private static String formatCanNotCompareMsg(MajorType left, MajorType right) {
         StringBuilder sb = new StringBuilder();
    -    sb.append("Map, Array or repeated scalar type should not be used in group by, order by or in a comparison operator. Drill does not support compare between ");
    +    sb.append("Map, Array, Union or repeated scalar type should not be used in group by, order by or in a comparison operator. Drill does not support compare between ");
    --- End diff --
    
    Union here seems like a problem. The two values could very much be comparable. Shouldn't this just be a reasonably large case statement?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472178
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/UnionFunctions.java ---
    @@ -0,0 +1,184 @@
    +/**
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.expr.fn.impl;
    +
    +import io.netty.buffer.DrillBuf;
    +import org.apache.drill.common.types.TypeProtos.MinorType;
    +import org.apache.drill.exec.expr.DrillSimpleFunc;
    +import org.apache.drill.exec.expr.annotations.FunctionTemplate;
    +import org.apache.drill.exec.expr.annotations.FunctionTemplate.NullHandling;
    +import org.apache.drill.exec.expr.annotations.Output;
    +import org.apache.drill.exec.expr.annotations.Param;
    +import org.apache.drill.exec.expr.holders.BigIntHolder;
    +import org.apache.drill.exec.expr.holders.BitHolder;
    +import org.apache.drill.exec.expr.holders.NullableIntHolder;
    +import org.apache.drill.exec.expr.holders.NullableUInt1Holder;
    +import org.apache.drill.exec.expr.holders.UnionHolder;
    +import org.apache.drill.exec.expr.holders.IntHolder;
    +import org.apache.drill.exec.expr.holders.VarCharHolder;
    +import org.apache.drill.exec.vector.complex.impl.UnionReader;
    +import org.apache.drill.exec.vector.complex.reader.FieldReader;
    +
    +import javax.inject.Inject;
    +
    +public class UnionFunctions {
    --- End diff --
    
    I'm confused why there are GUnionFunctions and UnionFunctions.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472317
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/HashAggBatch.java ---
    @@ -211,6 +212,10 @@ private HashAggregator createAggregatorInternal() throws SchemaChangeException,
           final LogicalExpression expr =
               ExpressionTreeMaterializer.materialize(ne.getExpr(), incoming, collector, context.getFunctionRegistry());
     
    +      if (expr instanceof IfExpression) {
    +        throw new SchemaChangeException("Union type not supported in aggregate functions");
    --- End diff --
    
    Let's use UserException.Unsupported


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471262
  
    --- Diff: exec/java-exec/src/main/codegen/templates/UnionVector.java ---
    @@ -0,0 +1,479 @@
    +/**
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.
    + */
    +
    +import org.apache.drill.common.types.TypeProtos.MinorType;
    +
    +<@pp.dropOutputFile />
    +<@pp.changeOutputFile name="/org/apache/drill/exec/vector/complex/impl/UnionVector.java" />
    +
    +
    +<#include "/@includes/license.ftl" />
    +
    +package org.apache.drill.exec.vector.complex.impl;
    +
    +<#include "/@includes/vv_imports.ftl" />
    +import java.util.Iterator;
    +import org.apache.drill.exec.vector.complex.impl.ComplexCopier;
    +import org.apache.drill.exec.util.CallBack;
    +
    +/*
    + * This class is generated using freemarker and the ${.template_name} template.
    + */
    +@SuppressWarnings("unused")
    +
    +
    +public class UnionVector implements ValueVector {
    --- End diff --
    
    Can you please add description of the construction here?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by StevenMPhillips <gi...@git.apache.org>.
Github user StevenMPhillips commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43550818
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java ---
    @@ -26,12 +26,23 @@
     import org.apache.drill.common.util.FileUtils;
     import org.apache.drill.common.util.TestTools;
     import org.apache.drill.exec.ExecConstants;
    +import org.apache.drill.exec.planner.physical.PlannerSettings;
    +import org.junit.After;
     import org.junit.Ignore;
     import org.junit.Test;
     
     public class TestExampleQueries extends BaseTestQuery {
     //  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TestExampleQueries.class);
     
    +  @After
    --- End diff --
    
    I've removed this code altogether. It's not needed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471716
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java ---
    @@ -187,14 +206,17 @@ private static void logFunctionResolutionError(ErrorCollector errorCollector, Fu
     
       private static class MaterializeVisitor extends AbstractExprVisitor<LogicalExpression, FunctionLookupContext, RuntimeException> {
         private ExpressionValidator validator = new ExpressionValidator();
    -    private final ErrorCollector errorCollector;
    +    private ErrorCollector errorCollector;
    +    private Deque<ErrorCollector> errorCollectors = new ArrayDeque<>();
         private final VectorAccessible batch;
         private final boolean allowComplexWriter;
    +    private final boolean unionTypeEnabled;
     
    -    public MaterializeVisitor(VectorAccessible batch, ErrorCollector errorCollector, boolean allowComplexWriter) {
    +    public MaterializeVisitor(VectorAccessible batch, ErrorCollector errorCollector, boolean allowComplexWriter, boolean unionTypeEnabled) {
           this.batch = batch;
           this.errorCollector = errorCollector;
           this.allowComplexWriter = allowComplexWriter;
    +      this.unionTypeEnabled = unionTypeEnabled;
         }
     
         private LogicalExpression validateNewExpr(LogicalExpression newExpr) {
    --- End diff --
    
    can you add a javadoc please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by StevenMPhillips <gi...@git.apache.org>.
Github user StevenMPhillips commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43563231
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionGenerationHelper.java ---
    @@ -127,7 +133,7 @@ protected static boolean isComparableType(MajorType type) {
     
       private static String formatCanNotCompareMsg(MajorType left, MajorType right) {
         StringBuilder sb = new StringBuilder();
    -    sb.append("Map, Array or repeated scalar type should not be used in group by, order by or in a comparison operator. Drill does not support compare between ");
    +    sb.append("Map, Array, Union or repeated scalar type should not be used in group by, order by or in a comparison operator. Drill does not support compare between ");
    --- End diff --
    
    Yes, they could be comparable, but that functionality has not been implemented yet.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471705
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java ---
    @@ -239,6 +261,10 @@ public LogicalExpression visitFunctionCall(FunctionCall call, FunctionLookupCont
           //replace with a new function call, since its argument could be changed.
           call = new FunctionCall(call.getName(), args, call.getPosition());
     
    +//      if (hasUnionInput(call)) {
    --- End diff --
    
    delete?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471797
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java ---
    @@ -308,10 +334,115 @@ public LogicalExpression visitFunctionCall(FunctionCall call, FunctionLookupCont
             return matchedNonDrillFuncHolder.getExpr(call.getName(), extArgsWithCast, call.getPosition());
           }
     
    +      if (hasUnionInput(call)) {
    +        return rewriteUnionFunction(call, functionLookupContext);
    +      }
    +
           logFunctionResolutionError(errorCollector, call);
           return NullExpression.INSTANCE;
         }
     
    +    private static final Set<String> UNION_FUNCTIONS;
    +
    +    static {
    +      UNION_FUNCTIONS = new HashSet<>();
    +      for (MinorType t : MinorType.values()) {
    +        UNION_FUNCTIONS.add("assert_" + t.name().toLowerCase());
    +        UNION_FUNCTIONS.add("is_" + t.name().toLowerCase());
    +      }
    +      UNION_FUNCTIONS.add("typeof");
    +    }
    +
    +    private boolean hasUnionInput(FunctionCall call) {
    +      for (LogicalExpression arg : call.args) {
    +        if (arg.getMajorType().getMinorType() == MinorType.UNION) {
    +          return true;
    +        }
    +      }
    +      return false;
    +    }
    +
    +    private LogicalExpression rewriteUnionFunction(FunctionCall call, FunctionLookupContext functionLookupContext) {
    +      LogicalExpression[] args = new LogicalExpression[call.args.size()];
    +      call.args.toArray(args);
    +
    +      for (int i = 0; i < args.length; i++) {
    +        LogicalExpression arg = call.args.get(i);
    +        MajorType majorType = arg.getMajorType();
    +
    +        if (majorType.getMinorType() != MinorType.UNION) {
    +          continue;
    +        }
    +
    +        List<MinorType> subTypes = majorType.getSubTypeList();
    +        Preconditions.checkState(subTypes.size() > 0, "Union type has no subtypes");
    +
    +        Queue<IfCondition> ifConditions = Lists.newLinkedList();
    +
    +        for (MinorType minorType : subTypes) {
    +          LogicalExpression ifCondition = getIsTypeExpressionForType(minorType, arg.accept(new CloneVisitor(), null));
    +          args[i] = getUnionCastExpressionForType(minorType, arg.accept(new CloneVisitor(), null));
    +
    +          List<LogicalExpression> newArgs = Lists.newArrayList();
    +          for (LogicalExpression e : args) {
    +            newArgs.add(e.accept(new CloneVisitor(), null));
    +          }
    +
    +          errorCollectors.push(errorCollector);
    +          errorCollector = new ErrorCollectorImpl();
    +
    +          LogicalExpression thenExpression = new FunctionCall(call.getName(), newArgs, call.getPosition()).accept(this, functionLookupContext);
    +
    +          if (errorCollector.hasErrors()) {
    --- End diff --
    
    Let's add comment here about what we're doing re: exceptions.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43470816
  
    --- Diff: exec/java-exec/src/main/codegen/templates/AbstractFieldReader.java ---
    @@ -33,6 +33,10 @@
         super();
       }
     
    +  public boolean isSet() {
    --- End diff --
    
    javadoc


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by StevenMPhillips <gi...@git.apache.org>.
Github user StevenMPhillips commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43546226
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java ---
    @@ -173,7 +173,7 @@ public ValueVector getChildByOrdinal(int id) {
        *
        * Note that this method does not enforce any vector type check nor throws a schema change exception.
        */
    -  protected void putChild(String name, ValueVector vector) {
    +  public void putChild(String name, ValueVector vector) {
         putVector(name, vector);
    --- End diff --
    
    I'm not sure what it means for what is loaded to be right. But I went ahead and moved the UnionVector into the same package as AbstractMapVector, so I can use this method without making it public.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by StevenMPhillips <gi...@git.apache.org>.
Github user StevenMPhillips commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43479369
  
    --- Diff: exec/java-exec/src/main/codegen/templates/UnionFunctions.java ---
    @@ -0,0 +1,121 @@
    +/**
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.
    + */
    +
    +<@pp.dropOutputFile />
    +<@pp.changeOutputFile name="/org/apache/drill/exec/expr/fn/impl/GUnionFunctions.java" />
    +
    +
    +<#include "/@includes/license.ftl" />
    +
    +package org.apache.drill.exec.expr.fn.impl;
    +
    +<#include "/@includes/vv_imports.ftl" />
    +import org.apache.drill.exec.expr.DrillSimpleFunc;
    +import org.apache.drill.exec.expr.annotations.FunctionTemplate;
    +import org.apache.drill.exec.expr.annotations.FunctionTemplate.NullHandling;
    +import org.apache.drill.exec.expr.annotations.Output;
    +import org.apache.drill.exec.expr.annotations.Param;
    +import org.apache.drill.exec.expr.holders.*;
    +import javax.inject.Inject;
    +import io.netty.buffer.DrillBuf;
    +import org.apache.drill.exec.record.RecordBatch;
    +
    +/*
    + * This class is generated using freemarker and the ${.template_name} template.
    + */
    +
    +@SuppressWarnings("unused")
    +public class GUnionFunctions {
    +
    +  <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
    +  <#assign fields = minor.fields!type.fields />
    +  <#assign uncappedName = name?uncap_first/>
    +
    +  <#if !minor.class?starts_with("Decimal")>
    +
    +  @SuppressWarnings("unused")
    +  @FunctionTemplate(name = "is_${name?upper_case}", scope = FunctionTemplate.FunctionScope.SIMPLE, nulls=NullHandling.INTERNAL)
    +  public static class UnionIs${name} implements DrillSimpleFunc {
    +
    +    @Param UnionHolder in;
    +    @Output BitHolder out;
    +
    +    public void setup() {}
    +
    +    public void eval() {
    +      if (in.isSet == 1) {
    +        out.value = in.getType().getMinorType() == org.apache.drill.common.types.TypeProtos.MinorType.${name?upper_case} ? 1 : 0;
    +      } else {
    +        out.value = 0;
    +      }
    +    }
    +  }
    +
    +  @SuppressWarnings("unused")
    +  @FunctionTemplate(name = "assert_${name?upper_case}", scope = FunctionTemplate.FunctionScope.SIMPLE, nulls=NullHandling.INTERNAL)
    +  public static class CastUnion${name} implements DrillSimpleFunc {
    +
    +    @Param UnionHolder in;
    +    @Output Nullable${name}Holder out;
    +
    +    public void setup() {}
    +
    +    public void eval() {
    +      if (in.isSet == 1) {
    +        in.reader.read(out);
    --- End diff --
    
    It throws a message like:
    "You tried to read a [%s] type when you are using a field reader of type [%s]."
    
    I think these functions are internal and not for general use, so this type of message is problem fine.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472385
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/project/ProjectRecordBatch.java ---
    @@ -103,6 +104,10 @@ private void clear() {
         }
       }
     
    +  public static void crap() {
    +    System.out.print("");
    --- End diff --
    
    This seems like it should print "crap"...


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471835
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java ---
    @@ -308,10 +334,115 @@ public LogicalExpression visitFunctionCall(FunctionCall call, FunctionLookupCont
             return matchedNonDrillFuncHolder.getExpr(call.getName(), extArgsWithCast, call.getPosition());
           }
     
    +      if (hasUnionInput(call)) {
    +        return rewriteUnionFunction(call, functionLookupContext);
    +      }
    +
           logFunctionResolutionError(errorCollector, call);
           return NullExpression.INSTANCE;
         }
     
    +    private static final Set<String> UNION_FUNCTIONS;
    +
    +    static {
    +      UNION_FUNCTIONS = new HashSet<>();
    +      for (MinorType t : MinorType.values()) {
    +        UNION_FUNCTIONS.add("assert_" + t.name().toLowerCase());
    +        UNION_FUNCTIONS.add("is_" + t.name().toLowerCase());
    +      }
    +      UNION_FUNCTIONS.add("typeof");
    +    }
    +
    +    private boolean hasUnionInput(FunctionCall call) {
    +      for (LogicalExpression arg : call.args) {
    +        if (arg.getMajorType().getMinorType() == MinorType.UNION) {
    +          return true;
    +        }
    +      }
    +      return false;
    +    }
    +
    +    private LogicalExpression rewriteUnionFunction(FunctionCall call, FunctionLookupContext functionLookupContext) {
    +      LogicalExpression[] args = new LogicalExpression[call.args.size()];
    +      call.args.toArray(args);
    +
    +      for (int i = 0; i < args.length; i++) {
    +        LogicalExpression arg = call.args.get(i);
    +        MajorType majorType = arg.getMajorType();
    +
    +        if (majorType.getMinorType() != MinorType.UNION) {
    +          continue;
    +        }
    +
    +        List<MinorType> subTypes = majorType.getSubTypeList();
    +        Preconditions.checkState(subTypes.size() > 0, "Union type has no subtypes");
    +
    +        Queue<IfCondition> ifConditions = Lists.newLinkedList();
    +
    +        for (MinorType minorType : subTypes) {
    +          LogicalExpression ifCondition = getIsTypeExpressionForType(minorType, arg.accept(new CloneVisitor(), null));
    +          args[i] = getUnionCastExpressionForType(minorType, arg.accept(new CloneVisitor(), null));
    +
    +          List<LogicalExpression> newArgs = Lists.newArrayList();
    +          for (LogicalExpression e : args) {
    +            newArgs.add(e.accept(new CloneVisitor(), null));
    +          }
    +
    +          errorCollectors.push(errorCollector);
    +          errorCollector = new ErrorCollectorImpl();
    +
    +          LogicalExpression thenExpression = new FunctionCall(call.getName(), newArgs, call.getPosition()).accept(this, functionLookupContext);
    +
    +          if (errorCollector.hasErrors()) {
    +            thenExpression = getExceptionFunction(errorCollector.toErrorString());
    +          }
    +
    +          errorCollector = errorCollectors.pop();
    +
    +          IfExpression.IfCondition condition = new IfCondition(ifCondition, thenExpression);
    +          ifConditions.add(condition);
    +        }
    +
    +        LogicalExpression ifExpression = ifConditions.poll().expression;
    +
    +        while (!ifConditions.isEmpty()) {
    +          ifExpression = IfExpression.newBuilder().setIfCondition(ifConditions.poll()).setElse(ifExpression).build();
    +        }
    +
    +        args[i] = ifExpression;
    +        return ifExpression.accept(this, functionLookupContext);
    +      }
    +      throw new UnsupportedOperationException("Did not find any Union input types");
    +    }
    +
    +    private LogicalExpression getExceptionFunction(String message) {
    +      QuotedString msg = new QuotedString(message, ExpressionPosition.UNKNOWN);
    +      List<LogicalExpression> args = Lists.newArrayList();
    +      args.add(msg);
    +      FunctionCall call = new FunctionCall(ExceptionFunction.EXCEPTION_FUNCTION_NAME, args, ExpressionPosition.UNKNOWN);
    +      return call;
    +    }
    +
    +    private LogicalExpression getUnionCastExpressionForType(MinorType type, LogicalExpression arg) {
    --- End diff --
    
    javadoc


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471990
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/GetSetVectorHelper.java ---
    @@ -114,6 +115,9 @@ public static JInvocation write(MajorType type, JVar vector, HoldingContainer in
     
         JInvocation setMethod = vector.invoke("getMutator").invoke(setMethodName).arg(indexVariable);
     
    +    if (type.getMinorType() == MinorType.UNION) {
    --- End diff --
    
    Why isn't this part of the lower case statement?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471955
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java ---
    @@ -323,23 +454,50 @@ public LogicalExpression visitIfExpression(IfExpression ifExpr, FunctionLookupCo
     
           MinorType thenType = conditions.expression.getMajorType().getMinorType();
           MinorType elseType = newElseExpr.getMajorType().getMinorType();
    -
    -      // Check if we need a cast
    -      if (thenType != elseType && !(thenType == MinorType.NULL || elseType == MinorType.NULL)) {
    -
    -        MinorType leastRestrictive = TypeCastRules.getLeastRestrictiveType((Arrays.asList(thenType, elseType)));
    -        if (leastRestrictive != thenType) {
    -          // Implicitly cast the then expression
    +      boolean hasUnion = thenType == MinorType.UNION || elseType == MinorType.UNION;
    +      if (unionTypeEnabled) {
    +        if (thenType != elseType && !(thenType == MinorType.NULL || elseType == MinorType.NULL)) {
    +
    +          MinorType leastRestrictive = MinorType.UNION;
    +          MajorType.Builder builder = MajorType.newBuilder().setMinorType(MinorType.UNION).setMode(DataMode.OPTIONAL);
    +          if (thenType == MinorType.UNION) {
    +            for (MinorType subType : conditions.expression.getMajorType().getSubTypeList()) {
    +              builder.addSubType(subType);
    +            }
    +          } else {
    +            builder.addSubType(thenType);
    +          }
    +          if (elseType == MinorType.UNION) {
    +            for (MinorType subType : newElseExpr.getMajorType().getSubTypeList()) {
    +              builder.addSubType(subType);
    +            }
    +          } else {
    +            builder.addSubType(elseType);
    +          }
    +          MajorType unionType = builder.build();
               conditions = new IfExpression.IfCondition(newCondition,
    -          addCastExpression(conditions.expression, newElseExpr.getMajorType(), functionLookupContext, errorCollector));
    -        } else if (leastRestrictive != elseType) {
    -          // Implicitly cast the else expression
    -          newElseExpr = addCastExpression(newElseExpr, conditions.expression.getMajorType(), functionLookupContext, errorCollector);
    -        } else {
    -          /* Cannot cast one of the two expressions to make the output type of if and else expression
    -           * to be the same. Raise error.
    -           */
    -          throw new DrillRuntimeException("Case expression should have similar output type on all its branches");
    +                  addCastExpression(conditions.expression, unionType, functionLookupContext, errorCollector));
    +          newElseExpr = addCastExpression(newElseExpr, unionType, functionLookupContext, errorCollector);
    +        }
    +
    +      } else {
    +        // Check if we need a cast
    +        if (thenType != elseType && !(thenType == MinorType.NULL || elseType == MinorType.NULL)) {
    +
    +          MinorType leastRestrictive = TypeCastRules.getLeastRestrictiveType((Arrays.asList(thenType, elseType)));
    +          if (leastRestrictive != thenType) {
    +            // Implicitly cast the then expression
    +            conditions = new IfExpression.IfCondition(newCondition,
    +            addCastExpression(conditions.expression, newElseExpr.getMajorType(), functionLookupContext, errorCollector));
    +          } else if (leastRestrictive != elseType) {
    +            // Implicitly cast the else expression
    +            newElseExpr = addCastExpression(newElseExpr, conditions.expression.getMajorType(), functionLookupContext, errorCollector);
    +          } else {
    +            /* Cannot cast one of the two expressions to make the output type of if and else expression
    +             * to be the same. Raise error.
    +             */
    +            throw new DrillRuntimeException("Case expression should have similar output type on all its branches");
    --- End diff --
    
    With union type enabled, we should never get here, right? It seems like you're adding the Union implicit casts to the implicit casting map. Are we going to get union casts even if union type is disabled because of the implicit casting map?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43470892
  
    --- Diff: exec/java-exec/src/main/codegen/templates/AbstractRecordWriter.java ---
    @@ -55,6 +55,11 @@ public FieldConverter getNewMapConverter(int fieldId, String fieldName, FieldRea
       }
     
       @Override
    +  public FieldConverter getNewUnionConverter(int fieldId, String fieldName, FieldReader reader) {
    +    throw new UnsupportedOperationException("Doesn't support writing Union type'");
    --- End diff --
    
    Can you update to the UserException.Unsupported()


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472925
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/TestExampleQueries.java ---
    @@ -26,12 +26,23 @@
     import org.apache.drill.common.util.FileUtils;
     import org.apache.drill.common.util.TestTools;
     import org.apache.drill.exec.ExecConstants;
    +import org.apache.drill.exec.planner.physical.PlannerSettings;
    +import org.junit.After;
     import org.junit.Ignore;
     import org.junit.Test;
     
     public class TestExampleQueries extends BaseTestQuery {
     //  private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TestExampleQueries.class);
     
    +  @After
    --- End diff --
    
    Can you just add ALTER SESSION RESET ALL to the basetestquery end of test method?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by StevenMPhillips <gi...@git.apache.org>.
Github user StevenMPhillips commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43482191
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java ---
    @@ -242,7 +244,7 @@
           Long.MAX_VALUE, 60 * 1000 * 5);
     
       public static final String ENABLE_VERBOSE_ERRORS_KEY = "exec.errors.verbose";
    -  public static final OptionValidator ENABLE_VERBOSE_ERRORS = new BooleanValidator(ENABLE_VERBOSE_ERRORS_KEY, false);
    +  public static final OptionValidator ENABLE_VERBOSE_ERRORS = new BooleanValidator(ENABLE_VERBOSE_ERRORS_KEY, true);
    --- End diff --
    
    reverted


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472061
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ExceptionFunction.java ---
    @@ -0,0 +1,53 @@
    +/**
    + * 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.expr.fn;
    +
    +import org.apache.drill.common.exceptions.DrillRuntimeException;
    +import org.apache.drill.exec.expr.DrillSimpleFunc;
    +import org.apache.drill.exec.expr.annotations.FunctionTemplate;
    +import org.apache.drill.exec.expr.annotations.FunctionTemplate.FunctionScope;
    +import org.apache.drill.exec.expr.annotations.Output;
    +import org.apache.drill.exec.expr.annotations.Param;
    +import org.apache.drill.exec.expr.holders.BigIntHolder;
    +import org.apache.drill.exec.expr.holders.VarCharHolder;
    +
    +public class ExceptionFunction {
    +
    +  public static final String EXCEPTION_FUNCTION_NAME = "throwException";
    --- End diff --
    
    can we call it __throwException (double underscore for system functions?)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472340
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/aggregate/StreamingAggBatch.java ---
    @@ -280,6 +281,9 @@ private StreamingAggregator createAggregatorInternal() throws SchemaChangeExcept
         for (int i = 0; i < valueExprs.length; i++) {
           final NamedExpression ne = popConfig.getExprs()[i];
           final LogicalExpression expr = ExpressionTreeMaterializer.materialize(ne.getExpr(), incoming, collector, context.getFunctionRegistry());
    +      if (expr instanceof IfExpression) {
    +        throw new SchemaChangeException("Union type not supported in aggregate functions");
    --- End diff --
    
    UserException


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471615
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/CloneVisitor.java ---
    @@ -0,0 +1,194 @@
    +/**
    + * 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.expr;
    +
    +import com.google.common.collect.Lists;
    +import org.apache.drill.common.expression.BooleanOperator;
    +import org.apache.drill.common.expression.CastExpression;
    +import org.apache.drill.common.expression.ConvertExpression;
    +import org.apache.drill.common.expression.FunctionCall;
    +import org.apache.drill.common.expression.FunctionHolderExpression;
    +import org.apache.drill.common.expression.IfExpression;
    +import org.apache.drill.common.expression.IfExpression.IfCondition;
    +import org.apache.drill.common.expression.LogicalExpression;
    +import org.apache.drill.common.expression.NullExpression;
    +import org.apache.drill.common.expression.SchemaPath;
    +import org.apache.drill.common.expression.TypedNullConstant;
    +import org.apache.drill.common.expression.ValueExpressions.BooleanExpression;
    +import org.apache.drill.common.expression.ValueExpressions.DateExpression;
    +import org.apache.drill.common.expression.ValueExpressions.Decimal18Expression;
    +import org.apache.drill.common.expression.ValueExpressions.Decimal28Expression;
    +import org.apache.drill.common.expression.ValueExpressions.Decimal38Expression;
    +import org.apache.drill.common.expression.ValueExpressions.Decimal9Expression;
    +import org.apache.drill.common.expression.ValueExpressions.DoubleExpression;
    +import org.apache.drill.common.expression.ValueExpressions.FloatExpression;
    +import org.apache.drill.common.expression.ValueExpressions.IntExpression;
    +import org.apache.drill.common.expression.ValueExpressions.IntervalDayExpression;
    +import org.apache.drill.common.expression.ValueExpressions.IntervalYearExpression;
    +import org.apache.drill.common.expression.ValueExpressions.LongExpression;
    +import org.apache.drill.common.expression.ValueExpressions.QuotedString;
    +import org.apache.drill.common.expression.ValueExpressions.TimeExpression;
    +import org.apache.drill.common.expression.ValueExpressions.TimeStampExpression;
    +import org.apache.drill.common.expression.visitors.AbstractExprVisitor;
    +import org.apache.drill.exec.expr.fn.DrillFuncHolder;
    +
    +import java.util.List;
    +
    +public class CloneVisitor extends AbstractExprVisitor<LogicalExpression,Void,RuntimeException> {
    --- End diff --
    
    javadoc


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: Drill-3232: Handle promoting writers and vecto...

Posted by StevenMPhillips <gi...@git.apache.org>.
Github user StevenMPhillips commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r42285176
  
    --- Diff: protocol/src/main/protobuf/Types.proto ---
    @@ -73,6 +74,7 @@ message MajorType {
       optional int32 precision = 4; // used for decimal types
       optional int32 scale = 5; // used for decimal types
       optional int32 timeZone = 6; // used by TimeStamp type
    +  repeated MinorType sub_type = 7; // used by Union type
    --- End diff --
    
    We basically don't allow a non-leaf/non-root node of an expression tree to have a complex type. Expressions where an intermediate node are complex, the expression is split apart into separate operators. So we only care about the subtypes of a map or list vector when reading from the vector.
    
    In the case of a Union vector, it is possible that intermediate nodes might have Union type. We pass the possible subtypes up the tree so that the code generated only includes the code needed for the specific types that are present, rather than having to generate code for every type at every node.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/drill/pull/207


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by StevenMPhillips <gi...@git.apache.org>.
Github user StevenMPhillips commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43482198
  
    --- Diff: exec/java-exec/src/main/codegen/templates/UnionVector.java ---
    @@ -0,0 +1,479 @@
    +/**
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.
    + */
    +
    +import org.apache.drill.common.types.TypeProtos.MinorType;
    +
    +<@pp.dropOutputFile />
    +<@pp.changeOutputFile name="/org/apache/drill/exec/vector/complex/impl/UnionVector.java" />
    +
    +
    +<#include "/@includes/license.ftl" />
    +
    +package org.apache.drill.exec.vector.complex.impl;
    +
    +<#include "/@includes/vv_imports.ftl" />
    +import java.util.Iterator;
    +import org.apache.drill.exec.vector.complex.impl.ComplexCopier;
    +import org.apache.drill.exec.util.CallBack;
    +
    +/*
    + * This class is generated using freemarker and the ${.template_name} template.
    + */
    +@SuppressWarnings("unused")
    +
    +
    +public class UnionVector implements ValueVector {
    +
    +  private MaterializedField field;
    +  private BufferAllocator allocator;
    +  private Accessor accessor = new Accessor();
    +  private Mutator mutator = new Mutator();
    +  private int valueCount;
    +
    +  private MapVector internalMap;
    +  private SingleMapWriter internalMapWriter;
    +  private UInt1Vector typeVector;
    +
    +  private MapVector mapVector;
    +  private ListVector listVector;
    +  private NullableBigIntVector bigInt;
    +  private NullableVarCharVector varChar;
    +
    +  private FieldReader reader;
    +  private NullableBitVector bit;
    +
    +  private State state = State.INIT;
    +  private int singleType = 0;
    +  private ValueVector singleVector;
    +  private MajorType majorType;
    +
    +  private final CallBack callBack;
    +
    +  private enum State {
    +    INIT, SINGLE, MULTI
    +  }
    +
    +  public UnionVector(MaterializedField field, BufferAllocator allocator, CallBack callBack) {
    +    this.field = field.clone();
    +    this.allocator = allocator;
    +    internalMap = new MapVector("internal", allocator, callBack);
    +    internalMapWriter = new SingleMapWriter(internalMap, null, true, true);
    +    this.typeVector = internalMap.addOrGet("types", Types.required(MinorType.UINT1), UInt1Vector.class);
    +    this.field.addChild(internalMap.getField().clone());
    +    this.majorType = field.getType();
    +    this.callBack = callBack;
    +  }
    +
    +  private void updateState(ValueVector v) {
    +    if (state == State.INIT) {
    +      state = State.SINGLE;
    +      singleVector = v;
    +      singleType = v.getField().getType().getMinorType().getNumber();
    +    } else {
    +      state = State.MULTI;
    +      singleVector = null;
    +    }
    +  }
    +
    +  public List<MinorType> getSubTypes() {
    +    return majorType.getSubTypeList();
    +  }
    +
    +  private void addSubType(MinorType type) {
    +    majorType =  MajorType.newBuilder(this.majorType).addSubType(type).build();
    +    if (callBack != null) {
    +      callBack.doWork();
    +    }
    +  }
    +
    +  public boolean isSingleType() {
    +    return state == State.SINGLE && singleType != MinorType.LIST_VALUE;
    +  }
    +
    +  public ValueVector getSingleVector() {
    +    assert state != State.MULTI : "Cannot get single vector when there are multiple types";
    +    assert state != State.INIT : "Cannot get single vector when there are no types";
    +    return singleVector;
    +  }
    +
    +  private static final MajorType MAP_TYPE = Types.optional(MinorType.MAP);
    +
    +  public MapVector getMap() {
    +    if (mapVector == null) {
    +      int vectorCount = internalMap.size();
    +      mapVector = internalMap.addOrGet("map", MAP_TYPE, MapVector.class);
    +      updateState(mapVector);
    +      addSubType(MinorType.MAP);
    +      if (internalMap.size() > vectorCount) {
    +        mapVector.allocateNew();
    +      }
    +    }
    +    return mapVector;
    +  }
    +
    +  <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
    +  <#assign fields = minor.fields!type.fields />
    +  <#assign uncappedName = name?uncap_first/>
    +  <#if !minor.class?starts_with("Decimal")>
    +
    +  private Nullable${name}Vector ${uncappedName}Vector;
    +  private static final MajorType ${name?upper_case}_TYPE = Types.optional(MinorType.${name?upper_case});
    +
    +  public Nullable${name}Vector get${name}Vector() {
    +    if (${uncappedName}Vector == null) {
    +      int vectorCount = internalMap.size();
    +      ${uncappedName}Vector = internalMap.addOrGet("${uncappedName}", ${name?upper_case}_TYPE, Nullable${name}Vector.class);
    +      updateState(${uncappedName}Vector);
    +      addSubType(MinorType.${name?upper_case});
    +      if (internalMap.size() > vectorCount) {
    +        ${uncappedName}Vector.allocateNew();
    +      }
    +    }
    +    return ${uncappedName}Vector;
    +  }
    +
    +  </#if>
    +
    +  </#list></#list>
    +
    +  private static final MajorType LIST_TYPE = Types.optional(MinorType.LIST);
    +
    +  public ListVector getList() {
    +    if (listVector == null) {
    +      int vectorCount = internalMap.size();
    +      listVector = internalMap.addOrGet("list", LIST_TYPE, ListVector.class);
    +      updateState(listVector);
    +      addSubType(MinorType.LIST);
    +      if (internalMap.size() > vectorCount) {
    +        listVector.allocateNew();
    +      }
    +    }
    +    return listVector;
    +  }
    +
    +  public int getTypeValue(int index) {
    +    return typeVector.getAccessor().get(index);
    +  }
    +
    +  public UInt1Vector getTypeVector() {
    +    return typeVector;
    +  }
    +
    +  @Override
    +  public void allocateNew() throws OutOfMemoryRuntimeException {
    +    internalMap.allocateNew();
    +    if (typeVector != null) {
    +      typeVector.zeroVector();
    +    }
    +  }
    +
    +  @Override
    +  public boolean allocateNewSafe() {
    +    boolean safe = internalMap.allocateNewSafe();
    +    if (safe) {
    +      if (typeVector != null) {
    +        typeVector.zeroVector();
    +      }
    +    }
    +    return safe;
    +  }
    +
    +  @Override
    +  public void setInitialCapacity(int numRecords) {
    +  }
    +
    +  @Override
    +  public int getValueCapacity() {
    +    return Math.min(typeVector.getValueCapacity(), internalMap.getValueCapacity());
    +  }
    +
    +  @Override
    +  public void close() {
    +  }
    +
    +  @Override
    +  public void clear() {
    +    internalMap.clear();
    +  }
    +
    +  @Override
    +  public MaterializedField getField() {
    +    return field;
    +  }
    +
    +  @Override
    +  public TransferPair getTransferPair() {
    +    return new TransferImpl(field);
    +  }
    +
    +  @Override
    +  public TransferPair getTransferPair(FieldReference ref) {
    +    return new TransferImpl(field.withPath(ref));
    +  }
    +
    +  @Override
    +  public TransferPair makeTransferPair(ValueVector target) {
    +    return new TransferImpl((UnionVector) target);
    +  }
    +
    +  public void transferTo(UnionVector target) {
    +    internalMap.makeTransferPair(target.internalMap).transfer();
    +    target.valueCount = valueCount;
    +    target.majorType = majorType;
    +  }
    +
    +  public void copyFrom(int inIndex, int outIndex, UnionVector from) {
    +    from.getReader().setPosition(inIndex);
    +    getWriter().setPosition(outIndex);
    +    ComplexCopier copier = new ComplexCopier(from.reader, mutator.writer);
    +    copier.write();
    +  }
    +
    +  public void copyFromSafe(int inIndex, int outIndex, UnionVector from) {
    +    copyFrom(inIndex, outIndex, from);
    +  }
    +
    +  public void addVector(ValueVector v) {
    +    internalMap.putChild(v.getField().getType().getMinorType().name().toLowerCase(), v);
    +    addSubType(v.getField().getType().getMinorType());
    +  }
    +
    +  private class TransferImpl implements TransferPair {
    +
    +    UnionVector to;
    +
    +    public TransferImpl(MaterializedField field) {
    +      to = new UnionVector(field, allocator, null);
    +    }
    +
    +    public TransferImpl(UnionVector to) {
    +      this.to = to;
    +    }
    +
    +    @Override
    +    public void transfer() {
    +      transferTo(to);
    +    }
    +
    +    @Override
    +    public void splitAndTransfer(int startIndex, int length) {
    +
    +    }
    +
    +    @Override
    +    public ValueVector getTo() {
    +      return to;
    +    }
    +
    +    @Override
    +    public void copyValueSafe(int from, int to) {
    +      this.to.copyFrom(from, to, UnionVector.this);
    +    }
    +  }
    +
    +  @Override
    +  public Accessor getAccessor() {
    +    return accessor;
    +  }
    +
    +  @Override
    +  public Mutator getMutator() {
    +    return mutator;
    +  }
    +
    +  @Override
    +  public FieldReader getReader() {
    +    if (reader == null) {
    +      reader = new UnionReader(this);
    +    }
    +    return reader;
    +  }
    +
    +  public FieldWriter getWriter() {
    +    if (mutator.writer == null) {
    +      mutator.writer = new UnionWriter(this);
    +    }
    +    return mutator.writer;
    +  }
    +
    +  @Override
    +  public UserBitShared.SerializedField getMetadata() {
    +    SerializedField.Builder b = getField() //
    +            .getAsBuilder() //
    +            .setBufferLength(getBufferSize()) //
    +            .setValueCount(valueCount);
    +
    +    b.addChild(internalMap.getMetadata());
    +    return b.build();
    +  }
    +
    +  @Override
    +  public int getBufferSize() {
    +    return internalMap.getBufferSize();
    +  }
    +
    +  @Override
    +  public int getBufferSizeFor(final int valueCount) {
    +    if (valueCount == 0) {
    +      return 0;
    +    }
    +
    +    long bufferSize = 0;
    +    for (final ValueVector v : (Iterable<ValueVector>) this) {
    +      bufferSize += v.getBufferSizeFor(valueCount);
    +    }
    +
    +    return (int) bufferSize;
    +  }
    +
    +  @Override
    +  public DrillBuf[] getBuffers(boolean clear) {
    +    return internalMap.getBuffers(clear);
    +  }
    +
    +  @Override
    +  public void load(UserBitShared.SerializedField metadata, DrillBuf buffer) {
    +    valueCount = metadata.getValueCount();
    +
    +    internalMap.load(metadata.getChild(0), buffer);
    +  }
    +
    +  @Override
    +  public Iterator<ValueVector> iterator() {
    +    List<ValueVector> vectors = Lists.newArrayList(internalMap.iterator());
    +    vectors.add(typeVector);
    +    return vectors.iterator();
    +  }
    +
    +  public class Accessor extends BaseValueVector.BaseAccessor {
    +
    +
    +    @Override
    +    public Object getObject(int index) {
    +      int type = typeVector.getAccessor().get(index);
    +      switch (type) {
    +      case 0:
    +        return null;
    +      <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
    +      <#assign fields = minor.fields!type.fields />
    +      <#assign uncappedName = name?uncap_first/>
    +      <#if !minor.class?starts_with("Decimal")>
    +      case MinorType.${name?upper_case}_VALUE:
    +        return get${name}Vector().getAccessor().getObject(index);
    +      </#if>
    +
    +      </#list></#list>
    +      case MinorType.MAP_VALUE:
    +        return getMap().getAccessor().getObject(index);
    +      case MinorType.LIST_VALUE:
    +        return getList().getAccessor().getObject(index);
    +      default:
    +        throw new UnsupportedOperationException("Cannot support type: " + MinorType.valueOf(type));
    +      }
    +    }
    +
    +    public byte[] get(int index) {
    +      return null;
    +    }
    +
    +    public void get(int index, ComplexHolder holder) {
    +    }
    +
    +    public void get(int index, UnionHolder holder) {
    +      if (reader == null) {
    +        reader = new UnionReader(UnionVector.this);
    +      }
    +      reader.setPosition(index);
    +      holder.reader = reader;
    +    }
    +
    +    @Override
    +    public int getValueCount() {
    +      return valueCount;
    +    }
    +
    +    @Override
    +    public boolean isNull(int index) {
    +      return typeVector.getAccessor().get(index) == 0;
    +    }
    +
    +    public int isSet(int index) {
    +      return isNull(index) ? 0 : 1;
    +    }
    +  }
    +
    +  public class Mutator extends BaseValueVector.BaseMutator {
    +
    +    UnionWriter writer;
    +
    +    @Override
    +    public void setValueCount(int valueCount) {
    +      UnionVector.this.valueCount = valueCount;
    +      internalMap.getMutator().setValueCount(valueCount);
    +    }
    +
    +    public void set(int index, byte[] bytes) {
    +    }
    +
    +    public void setSafe(int index, UnionHolder holder) {
    +      FieldReader reader = holder.reader;
    --- End diff --
    
    ok


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471535
  
    --- Diff: exec/java-exec/src/main/codegen/templates/UnionVector.java ---
    @@ -0,0 +1,479 @@
    +/**
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.
    + */
    +
    +import org.apache.drill.common.types.TypeProtos.MinorType;
    +
    +<@pp.dropOutputFile />
    +<@pp.changeOutputFile name="/org/apache/drill/exec/vector/complex/impl/UnionVector.java" />
    +
    +
    +<#include "/@includes/license.ftl" />
    +
    +package org.apache.drill.exec.vector.complex.impl;
    +
    +<#include "/@includes/vv_imports.ftl" />
    +import java.util.Iterator;
    +import org.apache.drill.exec.vector.complex.impl.ComplexCopier;
    +import org.apache.drill.exec.util.CallBack;
    +
    +/*
    + * This class is generated using freemarker and the ${.template_name} template.
    + */
    +@SuppressWarnings("unused")
    +
    +
    +public class UnionVector implements ValueVector {
    +
    +  private MaterializedField field;
    +  private BufferAllocator allocator;
    +  private Accessor accessor = new Accessor();
    +  private Mutator mutator = new Mutator();
    +  private int valueCount;
    +
    +  private MapVector internalMap;
    +  private SingleMapWriter internalMapWriter;
    +  private UInt1Vector typeVector;
    +
    +  private MapVector mapVector;
    +  private ListVector listVector;
    +  private NullableBigIntVector bigInt;
    +  private NullableVarCharVector varChar;
    +
    +  private FieldReader reader;
    +  private NullableBitVector bit;
    +
    +  private State state = State.INIT;
    +  private int singleType = 0;
    +  private ValueVector singleVector;
    +  private MajorType majorType;
    +
    +  private final CallBack callBack;
    +
    +  private enum State {
    +    INIT, SINGLE, MULTI
    +  }
    +
    +  public UnionVector(MaterializedField field, BufferAllocator allocator, CallBack callBack) {
    +    this.field = field.clone();
    +    this.allocator = allocator;
    +    internalMap = new MapVector("internal", allocator, callBack);
    +    internalMapWriter = new SingleMapWriter(internalMap, null, true, true);
    +    this.typeVector = internalMap.addOrGet("types", Types.required(MinorType.UINT1), UInt1Vector.class);
    +    this.field.addChild(internalMap.getField().clone());
    +    this.majorType = field.getType();
    +    this.callBack = callBack;
    +  }
    +
    +  private void updateState(ValueVector v) {
    +    if (state == State.INIT) {
    +      state = State.SINGLE;
    +      singleVector = v;
    +      singleType = v.getField().getType().getMinorType().getNumber();
    +    } else {
    +      state = State.MULTI;
    +      singleVector = null;
    +    }
    +  }
    +
    +  public List<MinorType> getSubTypes() {
    +    return majorType.getSubTypeList();
    +  }
    +
    +  private void addSubType(MinorType type) {
    +    majorType =  MajorType.newBuilder(this.majorType).addSubType(type).build();
    +    if (callBack != null) {
    +      callBack.doWork();
    +    }
    +  }
    +
    +  public boolean isSingleType() {
    +    return state == State.SINGLE && singleType != MinorType.LIST_VALUE;
    +  }
    +
    +  public ValueVector getSingleVector() {
    +    assert state != State.MULTI : "Cannot get single vector when there are multiple types";
    +    assert state != State.INIT : "Cannot get single vector when there are no types";
    +    return singleVector;
    +  }
    +
    +  private static final MajorType MAP_TYPE = Types.optional(MinorType.MAP);
    +
    +  public MapVector getMap() {
    +    if (mapVector == null) {
    +      int vectorCount = internalMap.size();
    +      mapVector = internalMap.addOrGet("map", MAP_TYPE, MapVector.class);
    +      updateState(mapVector);
    +      addSubType(MinorType.MAP);
    +      if (internalMap.size() > vectorCount) {
    +        mapVector.allocateNew();
    +      }
    +    }
    +    return mapVector;
    +  }
    +
    +  <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
    +  <#assign fields = minor.fields!type.fields />
    +  <#assign uncappedName = name?uncap_first/>
    +  <#if !minor.class?starts_with("Decimal")>
    +
    +  private Nullable${name}Vector ${uncappedName}Vector;
    +  private static final MajorType ${name?upper_case}_TYPE = Types.optional(MinorType.${name?upper_case});
    +
    +  public Nullable${name}Vector get${name}Vector() {
    +    if (${uncappedName}Vector == null) {
    +      int vectorCount = internalMap.size();
    +      ${uncappedName}Vector = internalMap.addOrGet("${uncappedName}", ${name?upper_case}_TYPE, Nullable${name}Vector.class);
    +      updateState(${uncappedName}Vector);
    +      addSubType(MinorType.${name?upper_case});
    +      if (internalMap.size() > vectorCount) {
    +        ${uncappedName}Vector.allocateNew();
    +      }
    +    }
    +    return ${uncappedName}Vector;
    +  }
    +
    +  </#if>
    +
    +  </#list></#list>
    +
    +  private static final MajorType LIST_TYPE = Types.optional(MinorType.LIST);
    +
    +  public ListVector getList() {
    +    if (listVector == null) {
    +      int vectorCount = internalMap.size();
    +      listVector = internalMap.addOrGet("list", LIST_TYPE, ListVector.class);
    +      updateState(listVector);
    +      addSubType(MinorType.LIST);
    +      if (internalMap.size() > vectorCount) {
    +        listVector.allocateNew();
    +      }
    +    }
    +    return listVector;
    +  }
    +
    +  public int getTypeValue(int index) {
    +    return typeVector.getAccessor().get(index);
    +  }
    +
    +  public UInt1Vector getTypeVector() {
    +    return typeVector;
    +  }
    +
    +  @Override
    +  public void allocateNew() throws OutOfMemoryRuntimeException {
    +    internalMap.allocateNew();
    +    if (typeVector != null) {
    +      typeVector.zeroVector();
    +    }
    +  }
    +
    +  @Override
    +  public boolean allocateNewSafe() {
    +    boolean safe = internalMap.allocateNewSafe();
    +    if (safe) {
    +      if (typeVector != null) {
    +        typeVector.zeroVector();
    +      }
    +    }
    +    return safe;
    +  }
    +
    +  @Override
    +  public void setInitialCapacity(int numRecords) {
    +  }
    +
    +  @Override
    +  public int getValueCapacity() {
    +    return Math.min(typeVector.getValueCapacity(), internalMap.getValueCapacity());
    +  }
    +
    +  @Override
    +  public void close() {
    +  }
    +
    +  @Override
    +  public void clear() {
    +    internalMap.clear();
    +  }
    +
    +  @Override
    +  public MaterializedField getField() {
    +    return field;
    +  }
    +
    +  @Override
    +  public TransferPair getTransferPair() {
    +    return new TransferImpl(field);
    +  }
    +
    +  @Override
    +  public TransferPair getTransferPair(FieldReference ref) {
    +    return new TransferImpl(field.withPath(ref));
    +  }
    +
    +  @Override
    +  public TransferPair makeTransferPair(ValueVector target) {
    +    return new TransferImpl((UnionVector) target);
    +  }
    +
    +  public void transferTo(UnionVector target) {
    +    internalMap.makeTransferPair(target.internalMap).transfer();
    +    target.valueCount = valueCount;
    +    target.majorType = majorType;
    +  }
    +
    +  public void copyFrom(int inIndex, int outIndex, UnionVector from) {
    +    from.getReader().setPosition(inIndex);
    +    getWriter().setPosition(outIndex);
    +    ComplexCopier copier = new ComplexCopier(from.reader, mutator.writer);
    +    copier.write();
    +  }
    +
    +  public void copyFromSafe(int inIndex, int outIndex, UnionVector from) {
    +    copyFrom(inIndex, outIndex, from);
    +  }
    +
    +  public void addVector(ValueVector v) {
    +    internalMap.putChild(v.getField().getType().getMinorType().name().toLowerCase(), v);
    --- End diff --
    
    Can we put some kind of type checking here?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472564
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/vector/accessor/UnionSqlAccessor.java ---
    @@ -0,0 +1,129 @@
    +/**
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.vector.accessor;
    +
    +import org.apache.drill.common.types.TypeProtos.MajorType;
    +import org.apache.drill.common.types.TypeProtos.MinorType;
    +import org.apache.drill.common.types.Types;
    +import org.apache.drill.exec.vector.complex.impl.UnionVector;
    +import org.apache.drill.exec.vector.complex.impl.UnionWriter;
    +import org.apache.drill.exec.vector.complex.reader.FieldReader;
    +
    +import java.io.InputStream;
    +import java.io.Reader;
    +import java.math.BigDecimal;
    +import java.sql.Date;
    +import java.sql.Time;
    +import java.sql.Timestamp;
    +
    +public class UnionSqlAccessor extends AbstractSqlAccessor {
    +
    +  FieldReader reader;
    --- End diff --
    
    You need to use accessors here. There is a bunch of SqlAccessor functionality that is encapsulated in those that you should use. Probably we should simply interrogate the type then delegate to the appropriate accessor.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43470982
  
    --- Diff: exec/java-exec/src/main/codegen/templates/EventBasedRecordWriter.java ---
    @@ -118,6 +119,9 @@ public void endField() throws IOException {
       }
     
       public static FieldConverter getConverter(RecordWriter recordWriter, int fieldId, String fieldName, FieldReader reader) {
    +    if (reader instanceof UnionReader) {
    --- End diff --
    
    why isn't this part of the case statement below (instead of instanceof)?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472628
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/BaseRepeatedValueVector.java ---
    @@ -202,6 +202,11 @@ public int size() {
         return new AddOrGetResult<>((T)vector, created);
       }
     
    +  protected void replaceDataVector(ValueVector v) {
    --- End diff --
    
    javadoc


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43470926
  
    --- Diff: exec/java-exec/src/main/codegen/templates/ComplexCopier.java ---
    @@ -0,0 +1,134 @@
    +/**
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.
    + */
    +
    +<@pp.dropOutputFile />
    +<@pp.changeOutputFile name="/org/apache/drill/exec/vector/complex/impl/ComplexCopier.java" />
    +
    +
    +<#include "/@includes/license.ftl" />
    +
    +package org.apache.drill.exec.vector.complex.impl;
    +
    +<#include "/@includes/vv_imports.ftl" />
    +
    +/*
    + * This class is generated using freemarker and the ${.template_name} template.
    + */
    +@SuppressWarnings("unused")
    +public class ComplexCopier {
    +
    +  FieldReader in;
    +  FieldWriter out;
    +
    +  public ComplexCopier(FieldReader in, FieldWriter out) {
    +    this.in = in;
    +    this.out = out;
    +  }
    +
    +  public void write() {
    --- End diff --
    
    javadoc


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472247
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/UnionFunctions.java ---
    @@ -0,0 +1,184 @@
    +/**
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.expr.fn.impl;
    +
    +import io.netty.buffer.DrillBuf;
    +import org.apache.drill.common.types.TypeProtos.MinorType;
    +import org.apache.drill.exec.expr.DrillSimpleFunc;
    +import org.apache.drill.exec.expr.annotations.FunctionTemplate;
    +import org.apache.drill.exec.expr.annotations.FunctionTemplate.NullHandling;
    +import org.apache.drill.exec.expr.annotations.Output;
    +import org.apache.drill.exec.expr.annotations.Param;
    +import org.apache.drill.exec.expr.holders.BigIntHolder;
    +import org.apache.drill.exec.expr.holders.BitHolder;
    +import org.apache.drill.exec.expr.holders.NullableIntHolder;
    +import org.apache.drill.exec.expr.holders.NullableUInt1Holder;
    +import org.apache.drill.exec.expr.holders.UnionHolder;
    +import org.apache.drill.exec.expr.holders.IntHolder;
    +import org.apache.drill.exec.expr.holders.VarCharHolder;
    +import org.apache.drill.exec.vector.complex.impl.UnionReader;
    +import org.apache.drill.exec.vector.complex.reader.FieldReader;
    +
    +import javax.inject.Inject;
    +
    +public class UnionFunctions {
    +
    +  @FunctionTemplate(names = {"typeOf"},
    +          scope = FunctionTemplate.FunctionScope.SIMPLE,
    +          nulls = NullHandling.INTERNAL)
    +  public static class GetType implements DrillSimpleFunc {
    +
    +    @Param
    +    FieldReader input;
    +    @Output
    +    VarCharHolder out;
    +    @Inject
    +    DrillBuf buf;
    +
    +    public void setup() {}
    +
    +    public void eval() {
    +
    +      byte[] type;
    +      if (input.isSet()) {
    +         type = input.getType().getMinorType().name().getBytes();
    +      } else {
    +        type = org.apache.drill.common.types.TypeProtos.MinorType.NULL.name().getBytes();
    +      }
    +      buf = buf.reallocIfNeeded(type.length);
    +      buf.setBytes(0, type);
    +      out.buffer = buf;
    +      out.start = 0;
    +      out.end = type.length;
    +    }
    +  }
    +
    +  @SuppressWarnings("unused")
    +  @FunctionTemplate(names = {"castUNION", "castToUnion"}, scope = FunctionTemplate.FunctionScope.SIMPLE, nulls=NullHandling.NULL_IF_NULL)
    +  public static class CastUnionToUnion implements DrillSimpleFunc{
    +
    +    @Param FieldReader in;
    +    @Output
    +    UnionHolder out;
    +
    +    public void setup() {}
    +
    +    public void eval() {
    +      out.reader = in;
    +      out.isSet = in.isSet() ? 1 : 0;
    +    }
    +  }
    +
    +  @SuppressWarnings("unused")
    +  @FunctionTemplate(name = "asList", scope = FunctionTemplate.FunctionScope.SIMPLE, nulls=NullHandling.INTERNAL)
    --- End diff --
    
    ASSERT_LIST ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471089
  
    --- Diff: exec/java-exec/src/main/codegen/templates/NullReader.java ---
    @@ -56,11 +56,17 @@ public void copyAsValue(MapWriter writer) {}
     
       public void copyAsValue(ListWriter writer) {}
     
    -  <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first /> 
    +  public void copyAsValue(UnionWriter writer) {}
    +
    +  <#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
    +  public void read(${name}Holder holder){
    +    throw new UnsupportedOperationException("NullReader cannot read into non-nullable holder");
    --- End diff --
    
    UserException? I'm not sure what this message even means or what would cause this. I'm thinking it is a dev only message and thus Unsupported would be fine but want to confirm.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471302
  
    --- Diff: exec/java-exec/src/main/codegen/templates/UnionVector.java ---
    @@ -0,0 +1,479 @@
    +/**
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.
    + */
    +
    +import org.apache.drill.common.types.TypeProtos.MinorType;
    +
    +<@pp.dropOutputFile />
    +<@pp.changeOutputFile name="/org/apache/drill/exec/vector/complex/impl/UnionVector.java" />
    +
    +
    +<#include "/@includes/license.ftl" />
    +
    +package org.apache.drill.exec.vector.complex.impl;
    +
    +<#include "/@includes/vv_imports.ftl" />
    +import java.util.Iterator;
    +import org.apache.drill.exec.vector.complex.impl.ComplexCopier;
    +import org.apache.drill.exec.util.CallBack;
    +
    +/*
    + * This class is generated using freemarker and the ${.template_name} template.
    + */
    +@SuppressWarnings("unused")
    +
    +
    +public class UnionVector implements ValueVector {
    +
    +  private MaterializedField field;
    +  private BufferAllocator allocator;
    +  private Accessor accessor = new Accessor();
    +  private Mutator mutator = new Mutator();
    +  private int valueCount;
    +
    +  private MapVector internalMap;
    +  private SingleMapWriter internalMapWriter;
    +  private UInt1Vector typeVector;
    +
    +  private MapVector mapVector;
    +  private ListVector listVector;
    +  private NullableBigIntVector bigInt;
    +  private NullableVarCharVector varChar;
    +
    +  private FieldReader reader;
    +  private NullableBitVector bit;
    +
    +  private State state = State.INIT;
    +  private int singleType = 0;
    +  private ValueVector singleVector;
    +  private MajorType majorType;
    +
    +  private final CallBack callBack;
    +
    +  private enum State {
    +    INIT, SINGLE, MULTI
    +  }
    +
    +  public UnionVector(MaterializedField field, BufferAllocator allocator, CallBack callBack) {
    +    this.field = field.clone();
    +    this.allocator = allocator;
    +    internalMap = new MapVector("internal", allocator, callBack);
    +    internalMapWriter = new SingleMapWriter(internalMap, null, true, true);
    +    this.typeVector = internalMap.addOrGet("types", Types.required(MinorType.UINT1), UInt1Vector.class);
    +    this.field.addChild(internalMap.getField().clone());
    +    this.majorType = field.getType();
    +    this.callBack = callBack;
    +  }
    +
    +  private void updateState(ValueVector v) {
    +    if (state == State.INIT) {
    +      state = State.SINGLE;
    +      singleVector = v;
    +      singleType = v.getField().getType().getMinorType().getNumber();
    +    } else {
    +      state = State.MULTI;
    +      singleVector = null;
    +    }
    +  }
    +
    +  public List<MinorType> getSubTypes() {
    +    return majorType.getSubTypeList();
    +  }
    +
    +  private void addSubType(MinorType type) {
    +    majorType =  MajorType.newBuilder(this.majorType).addSubType(type).build();
    +    if (callBack != null) {
    +      callBack.doWork();
    +    }
    +  }
    +
    +  public boolean isSingleType() {
    +    return state == State.SINGLE && singleType != MinorType.LIST_VALUE;
    +  }
    +
    +  public ValueVector getSingleVector() {
    +    assert state != State.MULTI : "Cannot get single vector when there are multiple types";
    --- End diff --
    
    Is this a performance critical path? If not, let's move to Preconditions


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472371
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/flatten/FlattenRecordBatch.java ---
    @@ -288,7 +288,7 @@ protected boolean setupNewSchema() throws SchemaChangeException {
         final IntOpenHashSet transferFieldIds = new IntOpenHashSet();
     
         final NamedExpression flattenExpr = new NamedExpression(popConfig.getColumn(), new FieldReference(popConfig.getColumn()));
    -    final ValueVectorReadExpression vectorRead = (ValueVectorReadExpression)ExpressionTreeMaterializer.materialize(flattenExpr.getExpr(), incoming, collector, context.getFunctionRegistry(), true);
    +    final ValueVectorReadExpression vectorRead = (ValueVectorReadExpression)ExpressionTreeMaterializer.materialize(flattenExpr.getExpr(), incoming, collector, context.getFunctionRegistry(), true, false);
    --- End diff --
    
    Can you just update materialize to have a version that doesn't take unionTypeEnabled and then uses false as the input. That way we don't need to change all the expressions (and then change them back once union type is always enabled).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471025
  
    --- Diff: exec/java-exec/src/main/codegen/templates/MapWriters.java ---
    @@ -52,9 +52,18 @@
       private final Map<String, FieldWriter> fields = Maps.newHashMap();
       <#if mode == "Repeated">private int currentChildIndex = 0;</#if>
     
    -  public ${mode}MapWriter(${containerClass} container, FieldWriter parent) {
    +  private final boolean unionEnabled;
    +  private final boolean unionInternalMap;
    --- End diff --
    
    Can you give a description of the unionInternalMap parameter and why that is needed in a javadoc?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43471006
  
    --- Diff: exec/java-exec/src/main/codegen/templates/MapWriters.java ---
    @@ -70,10 +79,15 @@ public MaterializedField getField() {
       @Override
       public MapWriter map(String name) {
           FieldWriter writer = fields.get(name.toLowerCase());
    -    if(writer == null) {
    -      int vectorCount = container.size();
    -      MapVector vector = container.addOrGet(name, MapVector.TYPE, MapVector.class);
    -      writer = new SingleMapWriter(vector, this);
    +    if(writer == null){
    +      int vectorCount=container.size();
    +      if(!unionEnabled || unionInternalMap){
    +        MapVector vector=container.addOrGet(name,MapVector.TYPE,MapVector.class);
    --- End diff --
    
    let's pull out the common expression between these two cases


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472622
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/AbstractMapVector.java ---
    @@ -173,7 +173,7 @@ public ValueVector getChildByOrdinal(int id) {
        *
        * Note that this method does not enforce any vector type check nor throws a schema change exception.
        */
    -  protected void putChild(String name, ValueVector vector) {
    +  public void putChild(String name, ValueVector vector) {
         putVector(name, vector);
    --- End diff --
    
    If we're making this more public, isn't there some way we can verify that what is being loaded is right? Also, why isn't protected enough?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by StevenMPhillips <gi...@git.apache.org>.
Github user StevenMPhillips commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43550594
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java ---
    @@ -275,83 +275,86 @@ private void consumeEntireNextValue() throws IOException {
       private void writeData(MapWriter map, FieldSelection selection, boolean moveForward) throws IOException {
         //
         map.start();
    -    outside: while (true) {
    -
    -      JsonToken t;
    -      if(moveForward){
    -        t = parser.nextToken();
    -      }else{
    -        t = parser.getCurrentToken();
    -        moveForward = true;
    -      }
    +    try {
    --- End diff --
    
    Yes, I am pretty sure that is the case. I don't remember why exactly I needed to put this in a try-finally block.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: Drill-3232: Handle promoting writers and vecto...

Posted by julienledem <gi...@git.apache.org>.
Github user julienledem commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r42252771
  
    --- Diff: protocol/src/main/protobuf/Types.proto ---
    @@ -73,6 +74,7 @@ message MajorType {
       optional int32 precision = 4; // used for decimal types
       optional int32 scale = 5; // used for decimal types
       optional int32 timeZone = 6; // used by TimeStamp type
    +  repeated MinorType sub_type = 7; // used by Union type
    --- End diff --
    
    Why do we need this for Union when we don't need it for other complex types (LIST, MAP, ...)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472821
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/fn/JsonReader.java ---
    @@ -275,83 +275,86 @@ private void consumeEntireNextValue() throws IOException {
       private void writeData(MapWriter map, FieldSelection selection, boolean moveForward) throws IOException {
         //
         map.start();
    -    outside: while (true) {
    -
    -      JsonToken t;
    -      if(moveForward){
    -        t = parser.nextToken();
    -      }else{
    -        t = parser.getCurrentToken();
    -        moveForward = true;
    -      }
    +    try {
    --- End diff --
    
    Is the only thing that is different here is that you ensure map.end() in a finally clause?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472417
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/physical/impl/validate/IteratorValidatorBatchIterator.java ---
    @@ -133,6 +134,7 @@ public IterOutcome next() {
             throw new IllegalStateException (String.format("Incoming batch of %s has size %d, which is beyond the limit of %d",  incoming.getClass().getName(), incoming.getRecordCount(), MAX_BATCH_SIZE));
           }
     
    +//      BatchPrinter.printBatch(incoming);
    --- End diff --
    
    delete


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43470918
  
    --- Diff: exec/java-exec/src/main/codegen/templates/ComplexCopier.java ---
    @@ -0,0 +1,134 @@
    +/**
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.
    + */
    +
    +<@pp.dropOutputFile />
    +<@pp.changeOutputFile name="/org/apache/drill/exec/vector/complex/impl/ComplexCopier.java" />
    +
    +
    +<#include "/@includes/license.ftl" />
    +
    +package org.apache.drill.exec.vector.complex.impl;
    +
    +<#include "/@includes/vv_imports.ftl" />
    +
    +/*
    + * This class is generated using freemarker and the ${.template_name} template.
    + */
    +@SuppressWarnings("unused")
    +public class ComplexCopier {
    --- End diff --
    
    Actual purpose of class javadoc please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472075
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/ExceptionFunction.java ---
    @@ -0,0 +1,53 @@
    +/**
    + * 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.expr.fn;
    +
    +import org.apache.drill.common.exceptions.DrillRuntimeException;
    +import org.apache.drill.exec.expr.DrillSimpleFunc;
    +import org.apache.drill.exec.expr.annotations.FunctionTemplate;
    +import org.apache.drill.exec.expr.annotations.FunctionTemplate.FunctionScope;
    +import org.apache.drill.exec.expr.annotations.Output;
    +import org.apache.drill.exec.expr.annotations.Param;
    +import org.apache.drill.exec.expr.holders.BigIntHolder;
    +import org.apache.drill.exec.expr.holders.VarCharHolder;
    +
    +public class ExceptionFunction {
    +
    +  public static final String EXCEPTION_FUNCTION_NAME = "throwException";
    +
    +  @FunctionTemplate(name = EXCEPTION_FUNCTION_NAME, isRandom = true,
    +          scope = FunctionScope.SIMPLE, nulls = FunctionTemplate.NullHandling.NULL_IF_NULL)
    +  public static class ThrowException implements DrillSimpleFunc {
    +
    +    @Param VarCharHolder message;
    +    @Output BigIntHolder out;
    +
    +    public void setup() {
    +    }
    +
    +    public void eval() {
    +      byte[] bytes = new byte[message.end - message.start];
    --- End diff --
    
    I'm pretty sure we have a utility method that converts from buffer to string.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472443
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/record/SimpleVectorWrapper.java ---
    @@ -103,6 +114,25 @@ public TypedFieldId getFieldIdIfMatches(int id, SchemaPath expectedPath) {
         }
         PathSegment seg = expectedPath.getRootSegment();
     
    +    if (v instanceof UnionVector) {
    --- End diff --
    
    need comments/javadoc


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by StevenMPhillips <gi...@git.apache.org>.
Github user StevenMPhillips commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43546612
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/ListVector.java ---
    @@ -0,0 +1,390 @@
    +/*******************************************************************************
    +
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.vector.complex;
    +
    +import com.google.common.collect.ObjectArrays;
    +import io.netty.buffer.DrillBuf;
    +import org.apache.drill.common.expression.FieldReference;
    +import org.apache.drill.common.expression.PathSegment;
    +import org.apache.drill.common.types.TypeProtos.DataMode;
    +import org.apache.drill.common.types.TypeProtos.MinorType;
    +import org.apache.drill.common.types.Types;
    +import org.apache.drill.exec.memory.BufferAllocator;
    +import org.apache.drill.exec.memory.OutOfMemoryRuntimeException;
    +import org.apache.drill.exec.proto.UserBitShared;
    +import org.apache.drill.exec.record.MaterializedField;
    +import org.apache.drill.exec.record.TransferPair;
    +import org.apache.drill.exec.record.TypedFieldId;
    +import org.apache.drill.exec.util.CallBack;
    +import org.apache.drill.exec.util.JsonStringArrayList;
    +import org.apache.drill.exec.vector.AddOrGetResult;
    +import org.apache.drill.exec.vector.BaseValueVector;
    +import org.apache.drill.exec.vector.UInt1Vector;
    +import org.apache.drill.exec.vector.UInt4Vector;
    +import org.apache.drill.exec.vector.ValueVector;
    +import org.apache.drill.exec.vector.VectorDescriptor;
    +import org.apache.drill.exec.vector.ZeroVector;
    +import org.apache.drill.exec.vector.complex.impl.ComplexCopier;
    +import org.apache.drill.exec.vector.complex.impl.UnionListReader;
    +import org.apache.drill.exec.vector.complex.impl.UnionListWriter;
    +import org.apache.drill.exec.vector.complex.impl.UnionVector;
    +import org.apache.drill.exec.vector.complex.reader.FieldReader;
    +import org.apache.drill.exec.vector.complex.writer.FieldWriter;
    +
    +import java.util.List;
    +
    +public class ListVector extends BaseRepeatedValueVector {
    +
    +  private UInt4Vector offsets;
    +  private final UInt1Vector bits;
    +  private Mutator mutator = new Mutator();
    +  private Accessor accessor = new Accessor();
    +  private UnionListWriter writer;
    +  private UnionListReader reader;
    +  private CallBack callBack;
    +
    +  public ListVector(MaterializedField field, BufferAllocator allocator, CallBack callBack) {
    +    super(field, allocator);
    +    this.bits = new UInt1Vector(MaterializedField.create("$bits$", Types.required(MinorType.UINT1)), allocator);
    +    offsets = getOffsetVector();
    +    this.field.addChild(getDataVector().getField());
    +    this.writer = new UnionListWriter(this);
    +    this.reader = new UnionListReader(this);
    +    this.callBack = callBack;
    +  }
    +
    +  public UnionListWriter getWriter() {
    +    return writer;
    +  }
    +
    +  @Override
    +  public void allocateNew() throws OutOfMemoryRuntimeException {
    +    super.allocateNewSafe();
    +  }
    +
    +  public void transferTo(ListVector target) {
    +    offsets.makeTransferPair(target.offsets).transfer();
    +    bits.makeTransferPair(target.bits).transfer();
    +    if (target.getDataVector() instanceof ZeroVector) {
    +      target.addOrGetVector(new VectorDescriptor(vector.getField().getType()));
    +    }
    +    getDataVector().makeTransferPair(target.getDataVector()).transfer();
    +  }
    +
    +  public void copyFromSafe(int inIndex, int outIndex, ListVector from) {
    +    copyFrom(inIndex, outIndex, from);
    +  }
    +
    +  public void copyFrom(int inIndex, int outIndex, ListVector from) {
    +    FieldReader in = from.getReader();
    +    in.setPosition(inIndex);
    +    FieldWriter out = getWriter();
    +    out.setPosition(outIndex);
    +    ComplexCopier copier = new ComplexCopier(in, out);
    +    copier.write();
    +  }
    +
    +  @Override
    +  public ValueVector getDataVector() {
    +    return vector;
    +  }
    +
    +  @Override
    +  public TransferPair getTransferPair(FieldReference ref) {
    +    return new TransferImpl(field.withPath(ref));
    +  }
    +
    +  @Override
    +  public TransferPair makeTransferPair(ValueVector target) {
    +    return new TransferImpl((ListVector) target);
    +  }
    +
    +  private class TransferImpl implements TransferPair {
    +
    +    ListVector to;
    +
    +    public TransferImpl(MaterializedField field) {
    +      to = new ListVector(field, allocator, null);
    +      to.addOrGetVector(new VectorDescriptor(vector.getField().getType()));
    +    }
    +
    +    public TransferImpl(ListVector to) {
    +      this.to = to;
    +      to.addOrGetVector(new VectorDescriptor(vector.getField().getType()));
    +    }
    +
    +    @Override
    +    public void transfer() {
    +      transferTo(to);
    +    }
    +
    +    @Override
    +    public void splitAndTransfer(int startIndex, int length) {
    +
    +    }
    +
    +    @Override
    +    public ValueVector getTo() {
    +      return to;
    +    }
    +
    +    @Override
    +    public void copyValueSafe(int from, int to) {
    +      this.to.copyFrom(from, to, ListVector.this);
    +    }
    +  }
    +
    +  @Override
    +  public Accessor getAccessor() {
    +    return accessor;
    +  }
    +
    +  @Override
    +  public Mutator getMutator() {
    +    return mutator;
    +  }
    +
    +  @Override
    +  public FieldReader getReader() {
    +    return reader;
    +  }
    +
    +  @Override
    +  public boolean allocateNewSafe() {
    +    /* boolean to keep track if all the memory allocation were successful
    +     * Used in the case of composite vectors when we need to allocate multiple
    +     * buffers for multiple vectors. If one of the allocations failed we need to
    +     * clear all the memory that we allocated
    +     */
    +    boolean success = false;
    +    try {
    +      if (!offsets.allocateNewSafe()) {
    +        return false;
    +      }
    +      success = vector.allocateNewSafe();
    +      success = success && bits.allocateNewSafe();
    +    } finally {
    +      if (!success) {
    +        clear();
    +      }
    +    }
    +    offsets.zeroVector();
    --- End diff --
    
    Do you mean we should NOT call these if we fail to allocate?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472659
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/vector/complex/ListVector.java ---
    @@ -0,0 +1,390 @@
    +/*******************************************************************************
    +
    + * 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
    + *
    + * http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * 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.vector.complex;
    +
    +import com.google.common.collect.ObjectArrays;
    +import io.netty.buffer.DrillBuf;
    +import org.apache.drill.common.expression.FieldReference;
    +import org.apache.drill.common.expression.PathSegment;
    +import org.apache.drill.common.types.TypeProtos.DataMode;
    +import org.apache.drill.common.types.TypeProtos.MinorType;
    +import org.apache.drill.common.types.Types;
    +import org.apache.drill.exec.memory.BufferAllocator;
    +import org.apache.drill.exec.memory.OutOfMemoryRuntimeException;
    +import org.apache.drill.exec.proto.UserBitShared;
    +import org.apache.drill.exec.record.MaterializedField;
    +import org.apache.drill.exec.record.TransferPair;
    +import org.apache.drill.exec.record.TypedFieldId;
    +import org.apache.drill.exec.util.CallBack;
    +import org.apache.drill.exec.util.JsonStringArrayList;
    +import org.apache.drill.exec.vector.AddOrGetResult;
    +import org.apache.drill.exec.vector.BaseValueVector;
    +import org.apache.drill.exec.vector.UInt1Vector;
    +import org.apache.drill.exec.vector.UInt4Vector;
    +import org.apache.drill.exec.vector.ValueVector;
    +import org.apache.drill.exec.vector.VectorDescriptor;
    +import org.apache.drill.exec.vector.ZeroVector;
    +import org.apache.drill.exec.vector.complex.impl.ComplexCopier;
    +import org.apache.drill.exec.vector.complex.impl.UnionListReader;
    +import org.apache.drill.exec.vector.complex.impl.UnionListWriter;
    +import org.apache.drill.exec.vector.complex.impl.UnionVector;
    +import org.apache.drill.exec.vector.complex.reader.FieldReader;
    +import org.apache.drill.exec.vector.complex.writer.FieldWriter;
    +
    +import java.util.List;
    +
    +public class ListVector extends BaseRepeatedValueVector {
    +
    +  private UInt4Vector offsets;
    +  private final UInt1Vector bits;
    +  private Mutator mutator = new Mutator();
    +  private Accessor accessor = new Accessor();
    +  private UnionListWriter writer;
    +  private UnionListReader reader;
    +  private CallBack callBack;
    +
    +  public ListVector(MaterializedField field, BufferAllocator allocator, CallBack callBack) {
    +    super(field, allocator);
    +    this.bits = new UInt1Vector(MaterializedField.create("$bits$", Types.required(MinorType.UINT1)), allocator);
    +    offsets = getOffsetVector();
    +    this.field.addChild(getDataVector().getField());
    +    this.writer = new UnionListWriter(this);
    +    this.reader = new UnionListReader(this);
    +    this.callBack = callBack;
    +  }
    +
    +  public UnionListWriter getWriter() {
    +    return writer;
    +  }
    +
    +  @Override
    +  public void allocateNew() throws OutOfMemoryRuntimeException {
    +    super.allocateNewSafe();
    +  }
    +
    +  public void transferTo(ListVector target) {
    +    offsets.makeTransferPair(target.offsets).transfer();
    +    bits.makeTransferPair(target.bits).transfer();
    +    if (target.getDataVector() instanceof ZeroVector) {
    +      target.addOrGetVector(new VectorDescriptor(vector.getField().getType()));
    +    }
    +    getDataVector().makeTransferPair(target.getDataVector()).transfer();
    +  }
    +
    +  public void copyFromSafe(int inIndex, int outIndex, ListVector from) {
    +    copyFrom(inIndex, outIndex, from);
    +  }
    +
    +  public void copyFrom(int inIndex, int outIndex, ListVector from) {
    +    FieldReader in = from.getReader();
    +    in.setPosition(inIndex);
    +    FieldWriter out = getWriter();
    +    out.setPosition(outIndex);
    +    ComplexCopier copier = new ComplexCopier(in, out);
    +    copier.write();
    +  }
    +
    +  @Override
    +  public ValueVector getDataVector() {
    +    return vector;
    +  }
    +
    +  @Override
    +  public TransferPair getTransferPair(FieldReference ref) {
    +    return new TransferImpl(field.withPath(ref));
    +  }
    +
    +  @Override
    +  public TransferPair makeTransferPair(ValueVector target) {
    +    return new TransferImpl((ListVector) target);
    +  }
    +
    +  private class TransferImpl implements TransferPair {
    +
    +    ListVector to;
    +
    +    public TransferImpl(MaterializedField field) {
    +      to = new ListVector(field, allocator, null);
    +      to.addOrGetVector(new VectorDescriptor(vector.getField().getType()));
    +    }
    +
    +    public TransferImpl(ListVector to) {
    +      this.to = to;
    +      to.addOrGetVector(new VectorDescriptor(vector.getField().getType()));
    +    }
    +
    +    @Override
    +    public void transfer() {
    +      transferTo(to);
    +    }
    +
    +    @Override
    +    public void splitAndTransfer(int startIndex, int length) {
    +
    +    }
    +
    +    @Override
    +    public ValueVector getTo() {
    +      return to;
    +    }
    +
    +    @Override
    +    public void copyValueSafe(int from, int to) {
    +      this.to.copyFrom(from, to, ListVector.this);
    +    }
    +  }
    +
    +  @Override
    +  public Accessor getAccessor() {
    +    return accessor;
    +  }
    +
    +  @Override
    +  public Mutator getMutator() {
    +    return mutator;
    +  }
    +
    +  @Override
    +  public FieldReader getReader() {
    +    return reader;
    +  }
    +
    +  @Override
    +  public boolean allocateNewSafe() {
    +    /* boolean to keep track if all the memory allocation were successful
    +     * Used in the case of composite vectors when we need to allocate multiple
    +     * buffers for multiple vectors. If one of the allocations failed we need to
    +     * clear all the memory that we allocated
    +     */
    +    boolean success = false;
    +    try {
    +      if (!offsets.allocateNewSafe()) {
    +        return false;
    +      }
    +      success = vector.allocateNewSafe();
    +      success = success && bits.allocateNewSafe();
    +    } finally {
    +      if (!success) {
    +        clear();
    +      }
    +    }
    +    offsets.zeroVector();
    --- End diff --
    
    are these legal if we failed to allocate? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by jacques-n <gi...@git.apache.org>.
Github user jacques-n commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43472435
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/record/AbstractRecordBatch.java ---
    @@ -62,6 +65,12 @@ protected AbstractRecordBatch(final T popConfig, final FragmentContext context,
         } else {
           state = BatchState.FIRST;
         }
    +    OptionValue option = context.getOptions().getOption(ExecConstants.ENABLE_UNION_TYPE.getOptionName());
    --- End diff --
    
    Please use the validator method so strongly typed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] drill pull request: DRILL-3232: Handle promoting writers and vecto...

Posted by StevenMPhillips <gi...@git.apache.org>.
Github user StevenMPhillips commented on a diff in the pull request:

    https://github.com/apache/drill/pull/207#discussion_r43563065
  
    --- Diff: exec/java-exec/src/main/java/org/apache/drill/exec/expr/ExpressionTreeMaterializer.java ---
    @@ -323,23 +454,50 @@ public LogicalExpression visitIfExpression(IfExpression ifExpr, FunctionLookupCo
     
           MinorType thenType = conditions.expression.getMajorType().getMinorType();
           MinorType elseType = newElseExpr.getMajorType().getMinorType();
    -
    -      // Check if we need a cast
    -      if (thenType != elseType && !(thenType == MinorType.NULL || elseType == MinorType.NULL)) {
    -
    -        MinorType leastRestrictive = TypeCastRules.getLeastRestrictiveType((Arrays.asList(thenType, elseType)));
    -        if (leastRestrictive != thenType) {
    -          // Implicitly cast the then expression
    +      boolean hasUnion = thenType == MinorType.UNION || elseType == MinorType.UNION;
    +      if (unionTypeEnabled) {
    +        if (thenType != elseType && !(thenType == MinorType.NULL || elseType == MinorType.NULL)) {
    +
    +          MinorType leastRestrictive = MinorType.UNION;
    +          MajorType.Builder builder = MajorType.newBuilder().setMinorType(MinorType.UNION).setMode(DataMode.OPTIONAL);
    +          if (thenType == MinorType.UNION) {
    +            for (MinorType subType : conditions.expression.getMajorType().getSubTypeList()) {
    +              builder.addSubType(subType);
    +            }
    +          } else {
    +            builder.addSubType(thenType);
    +          }
    +          if (elseType == MinorType.UNION) {
    +            for (MinorType subType : newElseExpr.getMajorType().getSubTypeList()) {
    +              builder.addSubType(subType);
    +            }
    +          } else {
    +            builder.addSubType(elseType);
    +          }
    +          MajorType unionType = builder.build();
               conditions = new IfExpression.IfCondition(newCondition,
    -          addCastExpression(conditions.expression, newElseExpr.getMajorType(), functionLookupContext, errorCollector));
    -        } else if (leastRestrictive != elseType) {
    -          // Implicitly cast the else expression
    -          newElseExpr = addCastExpression(newElseExpr, conditions.expression.getMajorType(), functionLookupContext, errorCollector);
    -        } else {
    -          /* Cannot cast one of the two expressions to make the output type of if and else expression
    -           * to be the same. Raise error.
    -           */
    -          throw new DrillRuntimeException("Case expression should have similar output type on all its branches");
    +                  addCastExpression(conditions.expression, unionType, functionLookupContext, errorCollector));
    +          newElseExpr = addCastExpression(newElseExpr, unionType, functionLookupContext, errorCollector);
    +        }
    +
    +      } else {
    +        // Check if we need a cast
    +        if (thenType != elseType && !(thenType == MinorType.NULL || elseType == MinorType.NULL)) {
    +
    +          MinorType leastRestrictive = TypeCastRules.getLeastRestrictiveType((Arrays.asList(thenType, elseType)));
    +          if (leastRestrictive != thenType) {
    +            // Implicitly cast the then expression
    +            conditions = new IfExpression.IfCondition(newCondition,
    +            addCastExpression(conditions.expression, newElseExpr.getMajorType(), functionLookupContext, errorCollector));
    +          } else if (leastRestrictive != elseType) {
    +            // Implicitly cast the else expression
    +            newElseExpr = addCastExpression(newElseExpr, conditions.expression.getMajorType(), functionLookupContext, errorCollector);
    +          } else {
    +            /* Cannot cast one of the two expressions to make the output type of if and else expression
    +             * to be the same. Raise error.
    +             */
    +            throw new DrillRuntimeException("Case expression should have similar output type on all its branches");
    --- End diff --
    
    I don't think this is a problem. In this case, the case statement will not coerce to a type unless one of the branches output is that type. So as long as there is no Union type in the input, there won't be a union in the output.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---