Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions vector/src/main/codegen/templates/AbstractFieldWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,16 @@ public void endEntry() {
throw new IllegalStateException(String.format("You tried to end a map entry when you are using a ValueWriter of type %s.", this.getClass().getSimpleName()));
}

public <T extends ExtensionHolder> void write(T var1) {
this.fail("ExtensionType");
}
public void writeExtensionType(Object var1) {
this.fail("ExtensionType");
}
public <T extends ExtensionTypeWriterFactory> void addExtensionTypeFactory(T var1) {
this.fail("ExtensionType");
}

<#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first />
<#assign fields = minor.fields!type.fields />
<#assign friendlyType = (minor.friendlyType!minor.boxedType!type.boxedType) />
Expand Down Expand Up @@ -241,6 +251,18 @@ public MapWriter map(String name, boolean keysSorted) {
fail("Map");
return null;
}

@Override
public ExtensionWriter extension(String name, ArrowType arrowType) {
fail("Extension");
return null;
}

@Override
public ExtensionWriter extension(ArrowType arrowType) {
fail("Extension");
return null;
}
<#list vv.types as type><#list type.minor as minor>
<#assign lowerName = minor.class?uncap_first />
<#if lowerName == "int" ><#assign lowerName = "integer" /></#if>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,11 @@ public MapWriter map(boolean keysSorted) {
return getWriter(MinorType.MAP, new ArrowType.Map(keysSorted));
}

@Override
public ExtensionWriter extension(ArrowType arrowType) {
return getWriter(MinorType.EXTENSIONTYPE).extension(arrowType);
}

@Override
public StructWriter struct(String name) {
return getWriter(MinorType.STRUCT).struct(name);
Expand All @@ -318,6 +323,11 @@ public MapWriter map(String name, boolean keysSorted) {
return getWriter(MinorType.STRUCT).map(name, keysSorted);
}

@Override
public ExtensionWriter extension(String name, ArrowType arrowType) {
return getWriter(MinorType.EXTENSIONTYPE).extension(name, arrowType);
}

<#list vv.types as type><#list type.minor as minor>
<#assign lowerName = minor.class?uncap_first />
<#if lowerName == "int" ><#assign lowerName = "integer" /></#if>
Expand Down
9 changes: 9 additions & 0 deletions vector/src/main/codegen/templates/BaseWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ public interface StructWriter extends BaseWriter {

void copyReaderToField(String name, FieldReader reader);
StructWriter struct(String name);
ExtensionWriter extension(String name, ArrowType arrowType);
ListWriter list(String name);
ListWriter listView(String name);
MapWriter map(String name);
Expand All @@ -79,6 +80,7 @@ public interface ListWriter extends BaseWriter {
ListWriter listView();
MapWriter map();
MapWriter map(boolean keysSorted);
ExtensionWriter extension(ArrowType arrowType);
void copyReader(FieldReader reader);

<#list vv.types as type><#list type.minor as minor>
Expand All @@ -101,6 +103,13 @@ public interface MapWriter extends ListWriter {
MapWriter value();
}

public interface ExtensionWriter extends BaseWriter {
void writeNull();
<T extends ExtensionHolder> void write(T var1);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here - why not just void write(ExtensionHolder value)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

void writeExtensionType(Object var1);
<T extends ExtensionTypeWriterFactory> void addExtensionTypeFactory(T var1);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While the existing code is short on docstrings, can we add docstrings for new code going forward? In particular it's not clear what addExtensionTypeFactory is or how to use it

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the generic bounds actually useful here? Why can't it just be ExtensionTypeWriterFactory var1?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we replace var1 with meaningful parameter names?

}

public interface ScalarWriter extends
<#list vv.types as type><#list type.minor as minor><#assign name = minor.class?cap_first /> ${name}Writer, </#list></#list> BaseWriter {}

Expand Down
14 changes: 14 additions & 0 deletions vector/src/main/codegen/templates/PromotableWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,9 @@ protected void setWriter(ValueVector v) {
case UNION:
writer = new UnionWriter((UnionVector) vector, nullableStructWriterFactory);
break;
case EXTENSIONTYPE:
writer = new UnionExtensionWriter((ExtensionTypeVector) vector);
break;
default:
writer = type.getNewFieldWriter(vector);
break;
Expand Down Expand Up @@ -316,6 +319,7 @@ protected boolean requiresArrowType(MinorType type) {
|| type == MinorType.MAP
|| type == MinorType.DURATION
|| type == MinorType.FIXEDSIZEBINARY
|| type == MinorType.EXTENSIONTYPE
|| (type.name().startsWith("TIMESTAMP") && type.name().endsWith("TZ"));
}

Expand Down Expand Up @@ -536,6 +540,16 @@ public void writeLargeVarChar(String value) {
getWriter(MinorType.LARGEVARCHAR).writeLargeVarChar(value);
}

@Override
public void writeExtensionType(Object value) {
getWriter(MinorType.EXTENSIONTYPE).writeExtensionType(value);
}

@Override
public <T extends ExtensionTypeWriterFactory> void addExtensionTypeFactory(T var1) {
getWriter(MinorType.EXTENSIONTYPE).addExtensionTypeFactory(var1);
}

@Override
public void allocate() {
getWriter().allocate();
Expand Down
26 changes: 26 additions & 0 deletions vector/src/main/codegen/templates/StructWriters.java
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ public class ${mode}StructWriter extends AbstractFieldWriter {
fields.put(handleCase(child.getName()), writer);
break;
}
case EXTENSIONTYPE:
extension(child.getName(), child.getType());
break;
case UNION:
FieldType fieldType = new FieldType(addVectorAsNullable, MinorType.UNION.getType(), null, null);
UnionWriter writer = new UnionWriter(container.addOrGet(child.getName(), fieldType, UnionVector.class), getNullableStructWriterFactory());
Expand Down Expand Up @@ -159,6 +162,29 @@ public StructWriter struct(String name) {
return writer;
}

@Override
public ExtensionWriter extension(String name, ArrowType arrowType) {
String finalName = handleCase(name);
FieldWriter writer = fields.get(finalName);
if(writer == null){
int vectorCount=container.size();
FieldType fieldType = new FieldType(addVectorAsNullable, arrowType, null, null);
ExtensionTypeVector vector = container.addOrGet(name, fieldType, ExtensionTypeVector.class);
writer = new PromotableWriter(vector, container, getNullableStructWriterFactory());
if(vectorCount != container.size()) {
writer.allocate();
}
writer.setPosition(idx());
fields.put(finalName, writer);
} else {
if (writer instanceof PromotableWriter) {
// ensure writers are initialized
((PromotableWriter)writer).getWriter(MinorType.EXTENSIONTYPE, arrowType);
}
}
return (ExtensionWriter) writer;
}

@Override
public void close() throws Exception {
clear();
Expand Down
23 changes: 23 additions & 0 deletions vector/src/main/codegen/templates/UnionListWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,17 @@ public MapWriter map(String name, boolean keysSorted) {
return mapWriter;
}

@Override
public ExtensionWriter extension(ArrowType arrowType) {
writer.extension(arrowType);
return writer;
}
@Override
public ExtensionWriter extension(String name, ArrowType arrowType) {
ExtensionWriter extensionWriter = writer.extension(name, arrowType);
return extensionWriter;
}

<#if listName == "LargeList">
@Override
public void startList() {
Expand Down Expand Up @@ -323,6 +334,18 @@ public void writeNull() {
}
}

@Override
public void writeExtensionType(Object value) {
writer.writeExtensionType(value);
}
@Override
public <T extends ExtensionTypeWriterFactory> void addExtensionTypeFactory(T var1) {
writer.addExtensionTypeFactory(var1);
}
public <T extends ExtensionHolder> void write(T var1) {
writer.write(var1);
}

<#list vv.types as type>
<#list type.minor as minor>
<#assign name = minor.class?cap_first />
Expand Down
12 changes: 12 additions & 0 deletions vector/src/main/codegen/templates/UnionMapWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -231,4 +231,16 @@ public MapWriter map() {
return super.map();
}
}

@Override
public ExtensionWriter extension(ArrowType type) {
switch (mode) {
case KEY:
return entryWriter.extension(MapVector.KEY_NAME, type);
case VALUE:
return entryWriter.extension(MapVector.VALUE_NAME, type);
default:
return super.extension(type);
}
}
}
20 changes: 20 additions & 0 deletions vector/src/main/codegen/templates/UnionWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,10 @@ public MapWriter asMap(ArrowType arrowType) {
return getMapWriter(arrowType);
}

private ExtensionWriter getExtensionWriter(ArrowType arrowType) {
throw new UnsupportedOperationException("ExtensionTypes are not supported yet.");
}

BaseWriter getWriter(MinorType minorType) {
return getWriter(minorType, null);
}
Expand All @@ -227,6 +231,8 @@ BaseWriter getWriter(MinorType minorType, ArrowType arrowType) {
return getListViewWriter();
case MAP:
return getMapWriter(arrowType);
case EXTENSIONTYPE:
return getExtensionWriter(arrowType);
<#list vv.types as type>
<#list type.minor as minor>
<#assign name = minor.class?cap_first />
Expand Down Expand Up @@ -460,6 +466,20 @@ public MapWriter map(String name, boolean keysSorted) {
return getStructWriter().map(name, keysSorted);
}

@Override
public ExtensionWriter extension(ArrowType arrowType) {
data.setType(idx(), MinorType.EXTENSIONTYPE);
getListWriter().setPosition(idx());
return getListWriter().extension(arrowType);
}

@Override
public ExtensionWriter extension(String name, ArrowType arrowType) {
data.setType(idx(), MinorType.EXTENSIONTYPE);
getStructWriter().setPosition(idx());
return getStructWriter().extension(name, arrowType);
}

<#list vv.types as type><#list type.minor as minor>
<#assign lowerName = minor.class?uncap_first />
<#if lowerName == "int" ><#assign lowerName = "integer" /></#if>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* 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.arrow.vector.complex.impl;

import org.apache.arrow.vector.ExtensionTypeVector;
import org.apache.arrow.vector.types.pojo.Field;

/**
* Base {@link AbstractFieldWriter} class for an {@link
* org.apache.arrow.vector.ExtensionTypeVector}.
*
* @param <T> a specific {@link ExtensionTypeVector}.
*/
public class AbstractExtensionTypeWriter<T extends ExtensionTypeVector>
extends AbstractFieldWriter {
protected final T vector;

public AbstractExtensionTypeWriter(T vector) {
this.vector = vector;
}

@Override
public Field getField() {
return this.vector.getField();
}

@Override
public int getValueCapacity() {
return this.vector.getValueCapacity();
}

@Override
public void allocate() {
this.vector.allocateNew();
}

@Override
public void close() {
this.vector.close();
}

@Override
public void clear() {
this.vector.clear();
}

@Override
protected int idx() {
return super.idx();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, is the only reason we override here to make it protected instead of package-private? (Should we just use getPosition() instead as it is already public and does the same thing?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, makes sense - logic with idx() method added for all other writers.

}

@Override
public void writeNull() {
this.vector.setNull(this.idx());
this.vector.setValueCount(this.idx() + 1);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* 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.arrow.vector.complex.impl;

import org.apache.arrow.vector.ExtensionTypeVector;

/**
* A factory for {@link ExtensionTypeWriter} instances. The factory allow to configure writer
* implementation for specific ExtensionTypeVector.
*
* @param <T> writer implementation for specific {@link ExtensionTypeVector}.
*/
public interface ExtensionTypeWriterFactory<T extends AbstractFieldWriter> {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we document this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the type bound need to be AbstractFieldWriter or can it just be FieldWriter (the interface)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I chose AbstractFieldWriter just to make sure that the user will use some specific implementation of writer, but in general yes - it could be FieldWriter

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO abstract implementation classes should not go in generic bounds - it should be the interface type

T getWriterImpl(ExtensionTypeVector vector);
Comment thread
lidavidm marked this conversation as resolved.
}
Loading
Loading