Skip to content

Commit

Permalink
ARROW-386: [Java] Respect case of struct / map field names
Browse files Browse the repository at this point in the history
Changes include:
- Remove all toLowerCase() calls on field names in MapWriters.java template file, so that the writers can respect case of the field names.
- Use lower-case keys for internalMap in UnionVector instead of camel-case (e.g. bigInt -> bigint). p.s. I don't know what is the original purpose of using camel case here. It did not conflict because all field names are converted to lower cases in the past.
- Add a simple test case of MapWriter with mixed-case field names.

Author: Jingyuan Wang <[email protected]>

Closes #261 from alphalfalfa/arrow-386 and squashes the following commits:

cd08145 [Jingyuan Wang] Remove unnecessary handleCase() call
7b28bfc [Jingyuan Wang] Pass caseSensitive Attribute down to nested MapWriters
2fe7bcf [Jingyuan Wang] Separate MapWriters with CaseSensitiveMapWriters
d269e21 [Jingyuan Wang] Configure case sensitivity when constructing ComplexWriterImpl
cba60d1 [Jingyuan Wang] Add option to MapWriters to configure the case sensitivity (defaulted as case-insensitive)
51da2a1 [Jingyuan Wang] Arrow-386: [Java] Respect case of struct / map field names
  • Loading branch information
alphalfalfa authored and wesm committed Jan 20, 2017
1 parent 6811d3f commit 512bc16
Show file tree
Hide file tree
Showing 10 changed files with 253 additions and 29 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/**
* 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 />
<#list ["Nullable", "Single"] as mode>
<@pp.changeOutputFile name="/org/apache/arrow/vector/complex/impl/${mode}CaseSensitiveMapWriter.java" />
<#assign index = "idx()">
<#if mode == "Single">
<#assign containerClass = "MapVector" />
<#else>
<#assign containerClass = "NullableMapVector" />
</#if>
<#include "/@includes/license.ftl" />
package org.apache.arrow.vector.complex.impl;
<#include "/@includes/vv_imports.ftl" />
/*
* This class is generated using FreeMarker and the ${.template_name} template.
*/
@SuppressWarnings("unused")
public class ${mode}CaseSensitiveMapWriter extends ${mode}MapWriter {
public ${mode}CaseSensitiveMapWriter(${containerClass} container) {
super(container);
}

@Override
protected String handleCase(final String input){
return input;
}

@Override
protected NullableMapWriterFactory getNullableMapWriterFactory() {
return NullableMapWriterFactory.getNullableCaseSensitiveMapWriterFactoryInstance();
}

}
</#list>
35 changes: 22 additions & 13 deletions java/vector/src/main/codegen/templates/MapWriters.java
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ public class ${mode}MapWriter extends AbstractFieldWriter {
protected final ${containerClass} container;
private final Map<String, FieldWriter> fields = Maps.newHashMap();
public ${mode}MapWriter(${containerClass} container) {
<#if mode == "Single">
if (container instanceof NullableMapVector) {
Expand All @@ -65,8 +64,8 @@ public class ${mode}MapWriter extends AbstractFieldWriter {
list(child.getName());
break;
case UNION:
UnionWriter writer = new UnionWriter(container.addOrGet(child.getName(), MinorType.UNION, UnionVector.class));
fields.put(child.getName().toLowerCase(), writer);
UnionWriter writer = new UnionWriter(container.addOrGet(child.getName(), MinorType.UNION, UnionVector.class), getNullableMapWriterFactory());
fields.put(handleCase(child.getName()), writer);
break;
<#list vv.types as type><#list type.minor as minor>
<#assign lowerName = minor.class?uncap_first />
Expand All @@ -85,6 +84,14 @@ public class ${mode}MapWriter extends AbstractFieldWriter {
}
}
protected String handleCase(final String input) {
return input.toLowerCase();
}
protected NullableMapWriterFactory getNullableMapWriterFactory() {
return NullableMapWriterFactory.getNullableMapWriterFactoryInstance();
}
@Override
public int getValueCapacity() {
return container.getValueCapacity();
Expand All @@ -102,16 +109,17 @@ public Field getField() {
@Override
public MapWriter map(String name) {
FieldWriter writer = fields.get(name.toLowerCase());
String finalName = handleCase(name);
FieldWriter writer = fields.get(finalName);
if(writer == null){
int vectorCount=container.size();
NullableMapVector vector = container.addOrGet(name, MinorType.MAP, NullableMapVector.class);
writer = new PromotableWriter(vector, container);
writer = new PromotableWriter(vector, container, getNullableMapWriterFactory());
if(vectorCount != container.size()) {
writer.allocate();
}
writer.setPosition(idx());
fields.put(name.toLowerCase(), writer);
fields.put(finalName, writer);
} else {
if (writer instanceof PromotableWriter) {
// ensure writers are initialized
Expand Down Expand Up @@ -145,15 +153,16 @@ public void clear() {
@Override
public ListWriter list(String name) {
FieldWriter writer = fields.get(name.toLowerCase());
String finalName = handleCase(name);
FieldWriter writer = fields.get(finalName);
int vectorCount = container.size();
if(writer == null) {
writer = new PromotableWriter(container.addOrGet(name, MinorType.LIST, ListVector.class), container);
writer = new PromotableWriter(container.addOrGet(name, MinorType.LIST, ListVector.class), container, getNullableMapWriterFactory());
if (container.size() > vectorCount) {
writer.allocate();
}
writer.setPosition(idx());
fields.put(name.toLowerCase(), writer);
fields.put(finalName, writer);
} else {
if (writer instanceof PromotableWriter) {
// ensure writers are initialized
Expand Down Expand Up @@ -199,7 +208,7 @@ public void end() {
<#if minor.class?starts_with("Decimal") >
public ${minor.class}Writer ${lowerName}(String name) {
// returns existing writer
final FieldWriter writer = fields.get(name.toLowerCase());
final FieldWriter writer = fields.get(handleCase(name));
assert writer != null;
return writer;
}
Expand All @@ -209,18 +218,18 @@ public void end() {
@Override
public ${minor.class}Writer ${lowerName}(String name) {
</#if>
FieldWriter writer = fields.get(name.toLowerCase());
FieldWriter writer = fields.get(handleCase(name));
if(writer == null) {
ValueVector vector;
ValueVector currentVector = container.getChild(name);
${vectName}Vector v = container.addOrGet(name, MinorType.${upperName}, ${vectName}Vector.class<#if minor.class == "Decimal"> , new int[] {precision, scale}</#if>);
writer = new PromotableWriter(v, container);
writer = new PromotableWriter(v, container, getNullableMapWriterFactory());
vector = v;
if (currentVector == null || currentVector != vector) {
vector.allocateNewSafe();
}
writer.setPosition(idx());
fields.put(name.toLowerCase(), writer);
fields.put(handleCase(name), writer);
} else {
if (writer instanceof PromotableWriter) {
// ensure writers are initialized
Expand Down
6 changes: 5 additions & 1 deletion java/vector/src/main/codegen/templates/UnionListWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,12 @@ public class UnionListWriter extends AbstractFieldWriter {
private int lastIndex = 0;

public UnionListWriter(ListVector vector) {
this(vector, NullableMapWriterFactory.getNullableMapWriterFactoryInstance());
}

public UnionListWriter(ListVector vector, NullableMapWriterFactory nullableMapWriterFactory) {
this.vector = vector;
this.writer = new PromotableWriter(vector.getDataVector(), vector);
this.writer = new PromotableWriter(vector.getDataVector(), vector, nullableMapWriterFactory);
this.offsets = vector.getOffsetVector();
}

Expand Down
3 changes: 2 additions & 1 deletion java/vector/src/main/codegen/templates/UnionVector.java
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,15 @@ public NullableMapVector getMap() {
<#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/>
<#assign lowerCaseName = name?lower_case/>
<#if !minor.class?starts_with("Decimal")>
private Nullable${name}Vector ${uncappedName}Vector;
public Nullable${name}Vector get${name}Vector() {
if (${uncappedName}Vector == null) {
int vectorCount = internalMap.size();
${uncappedName}Vector = internalMap.addOrGet("${uncappedName}", MinorType.${name?upper_case}, Nullable${name}Vector.class);
${uncappedName}Vector = internalMap.addOrGet("${lowerCaseName}", MinorType.${name?upper_case}, Nullable${name}Vector.class);
if (internalMap.size() > vectorCount) {
${uncappedName}Vector.allocateNew();
if (callBack != null) {
Expand Down
12 changes: 10 additions & 2 deletions java/vector/src/main/codegen/templates/UnionWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
* limitations under the License.
*/

import org.apache.arrow.vector.complex.impl.NullableMapWriterFactory;

<@pp.dropOutputFile />
<@pp.changeOutputFile name="/org/apache/arrow/vector/complex/impl/UnionWriter.java" />

Expand All @@ -38,9 +40,15 @@ public class UnionWriter extends AbstractFieldWriter implements FieldWriter {
private MapWriter mapWriter;
private UnionListWriter listWriter;
private List<BaseWriter> writers = Lists.newArrayList();
private final NullableMapWriterFactory nullableMapWriterFactory;

public UnionWriter(UnionVector vector) {
this(vector, NullableMapWriterFactory.getNullableMapWriterFactoryInstance());
}

public UnionWriter(UnionVector vector, NullableMapWriterFactory nullableMapWriterFactory) {
data = vector;
this.nullableMapWriterFactory = nullableMapWriterFactory;
}

@Override
Expand Down Expand Up @@ -76,7 +84,7 @@ public void endList() {

private MapWriter getMapWriter() {
if (mapWriter == null) {
mapWriter = new NullableMapWriter(data.getMap());
mapWriter = nullableMapWriterFactory.build(data.getMap());
mapWriter.setPosition(idx());
writers.add(mapWriter);
}
Expand All @@ -90,7 +98,7 @@ public MapWriter asMap() {

private ListWriter getListWriter() {
if (listWriter == null) {
listWriter = new UnionListWriter(data.getList());
listWriter = new UnionListWriter(data.getList(), nullableMapWriterFactory);
listWriter.setPosition(idx());
writers.add(listWriter);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ public ValueVector getChildByOrdinal(int id) {
*/
@Override
public <T extends FieldVector> T getChild(String name, Class<T> clazz) {
final ValueVector v = vectors.get(name.toLowerCase());
final ValueVector v = vectors.get(name);
if (v == null) {
return null;
}
Expand Down Expand Up @@ -191,7 +191,7 @@ protected void putChild(String name, FieldVector vector) {
*/
protected void putVector(String name, FieldVector vector) {
final ValueVector old = vectors.put(
Preconditions.checkNotNull(name, "field name cannot be null").toLowerCase(),
Preconditions.checkNotNull(name, "field name cannot be null"),
Preconditions.checkNotNull(vector, "vector cannot be null")
);
if (old != null && old != vector) {
Expand Down Expand Up @@ -254,7 +254,7 @@ public List<ValueVector> getPrimitiveVectors() {
*/
@Override
public VectorWithOrdinal getChildVectorWithOrdinal(String name) {
final int ordinal = vectors.getOrdinal(name.toLowerCase());
final int ordinal = vectors.getOrdinal(name);
if (ordinal < 0) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,20 @@ public class ComplexWriterImpl extends AbstractFieldWriter implements ComplexWri
Mode mode = Mode.INIT;
private final String name;
private final boolean unionEnabled;
private final NullableMapWriterFactory nullableMapWriterFactory;

private enum Mode { INIT, MAP, LIST };

public ComplexWriterImpl(String name, MapVector container, boolean unionEnabled){
public ComplexWriterImpl(String name, MapVector container, boolean unionEnabled, boolean caseSensitive){
this.name = name;
this.container = container;
this.unionEnabled = unionEnabled;
nullableMapWriterFactory = caseSensitive? NullableMapWriterFactory.getNullableCaseSensitiveMapWriterFactoryInstance() :
NullableMapWriterFactory.getNullableMapWriterFactoryInstance();
}

public ComplexWriterImpl(String name, MapVector container, boolean unionEnabled) {
this(name, container, unionEnabled, false);
}

public ComplexWriterImpl(String name, MapVector container){
Expand Down Expand Up @@ -122,8 +129,7 @@ public MapWriter directMap(){
switch(mode){

case INIT:
NullableMapVector map = (NullableMapVector) container;
mapRoot = new NullableMapWriter(map);
mapRoot = nullableMapWriterFactory.build((NullableMapVector) container);
mapRoot.setPosition(idx());
mode = Mode.MAP;
break;
Expand All @@ -144,7 +150,7 @@ public MapWriter rootAsMap() {

case INIT:
NullableMapVector map = container.addOrGet(name, MinorType.MAP, NullableMapVector.class);
mapRoot = new NullableMapWriter(map);
mapRoot = nullableMapWriterFactory.build(map);
mapRoot.setPosition(idx());
mode = Mode.MAP;
break;
Expand All @@ -159,7 +165,6 @@ public MapWriter rootAsMap() {
return mapRoot;
}


@Override
public void allocate() {
if(mapRoot != null) {
Expand All @@ -179,7 +184,7 @@ public ListWriter rootAsList() {
if (container.size() > vectorCount) {
listVector.allocateNew();
}
listRoot = new UnionListWriter(listVector);
listRoot = new UnionListWriter(listVector, nullableMapWriterFactory);
listRoot.setPosition(idx());
mode = Mode.LIST;
break;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/**
* 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.arrow.vector.complex.impl;

import org.apache.arrow.vector.complex.NullableMapVector;

public class NullableMapWriterFactory {
private final boolean caseSensitive;
private static final NullableMapWriterFactory nullableMapWriterFactory = new NullableMapWriterFactory(false);
private static final NullableMapWriterFactory nullableCaseSensitiveWriterFactory = new NullableMapWriterFactory(true);

public NullableMapWriterFactory(boolean caseSensitive) {
this.caseSensitive = caseSensitive;
}

public NullableMapWriter build(NullableMapVector container) {
return this.caseSensitive? new NullableCaseSensitiveMapWriter(container) : new NullableMapWriter(container);
}

public static NullableMapWriterFactory getNullableMapWriterFactoryInstance() {
return nullableMapWriterFactory;
}

public static NullableMapWriterFactory getNullableCaseSensitiveMapWriterFactoryInstance() {
return nullableCaseSensitiveWriterFactory;
}
}
Loading

0 comments on commit 512bc16

Please sign in to comment.