Skip to content
Closed
Show file tree
Hide file tree
Changes from all 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
4 changes: 2 additions & 2 deletions cpp/src/arrow/ipc/json-internal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1274,8 +1274,8 @@ class ArrayReader {

Status Visit(const MapType& type) {
auto list_type = std::make_shared<ListType>(field(
"item",
struct_({field("key", type.key_type(), false), field("item", type.item_type())}),
"entries",
struct_({field("key", type.key_type(), false), field("value", type.item_type())}),
false));
std::shared_ptr<Array> list_array;
RETURN_NOT_OK(CreateList(list_type, &list_array));
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/ipc/json-test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ TEST(TestJsonArrayWriter, NestedTypes) {

TestArrayRoundTrip(list_array);

// List
// Map
auto map_type = map(utf8(), int32());
auto keys_array = ArrayFromJSON(utf8(), R"(["a", "b", "c", "d", "a", "b", "c"])");

Expand Down
4 changes: 1 addition & 3 deletions cpp/src/arrow/ipc/metadata-internal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -320,9 +320,7 @@ Status ConcreteTypeFromFlatbuffer(flatbuf::Type type, const void* type_data,
if (children.size() != 1) {
return Status::Invalid("Map must have exactly 1 child field");
}
if ( // FIXME(bkietz) temporarily disabled: this field is sometimes read nullable
// children[0]->nullable() ||
children[0]->type()->id() != Type::STRUCT ||
if (children[0]->nullable() || children[0]->type()->id() != Type::STRUCT ||
children[0]->type()->num_children() != 2) {
return Status::Invalid("Map's key-item pairs must be non-nullable structs");
}
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/ipc/test-common.cc
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ Status MakeRandomMapArray(const std::shared_ptr<Array>& key_array,
bool include_nulls, MemoryPool* pool,
std::shared_ptr<Array>* out) {
auto pair_type = struct_(
{field("key", key_array->type(), false), field("item", item_array->type())});
{field("key", key_array->type(), false), field("value", item_array->type())});

auto pair_array = std::make_shared<StructArray>(pair_type, num_maps,
ArrayVector{key_array, item_array});
Expand Down
7 changes: 5 additions & 2 deletions cpp/src/arrow/type.cc
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,11 @@ std::string ListType::ToString() const {

MapType::MapType(const std::shared_ptr<DataType>& key_type,
const std::shared_ptr<DataType>& item_type, bool keys_sorted)
: ListType(struct_({std::make_shared<Field>("key", key_type, false),
std::make_shared<Field>("item", item_type)})),
: ListType(std::make_shared<Field>(
"entries",
struct_({std::make_shared<Field>("key", key_type, false),
std::make_shared<Field>("value", item_type)}),
false)),
keys_sorted_(keys_sorted) {
id_ = type_id;
}
Expand Down
12 changes: 5 additions & 7 deletions integration/integration_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -714,7 +714,7 @@ def __init__(self, name, key_type, item_type, nullable=True,
assert not key_type.nullable
self.key_type = key_type
self.item_type = item_type
self.pair_type = StructType('item', [key_type, item_type], False)
self.pair_type = StructType('entries', [key_type, item_type], False)
self.keysSorted = keysSorted

def _get_type(self):
Expand Down Expand Up @@ -1060,10 +1060,10 @@ def generate_interval_case():

def generate_map_case():
# TODO(bkietz): separated from nested_case so it can be
# independently skipped, consolidate after Java supports map
# independently skipped, consolidate after JS supports map
fields = [
MapType('map_nullable', get_field('key', 'utf8', False),
get_field('item', 'int32')),
get_field('value', 'int32')),
]

batch_sizes = [7, 10]
Expand Down Expand Up @@ -1220,12 +1220,10 @@ def _compare_implementations(self, producer, consumer):

file_id = guid()[:8]

if (('JS' in (producer.name, consumer.name) or
'Java' in (producer.name, consumer.name)) and
if ('JS' in (producer.name, consumer.name) and
"map" in test_case.name):
print('TODO(ARROW-1279): Enable map tests ' +
' for Java and JS once Java supports them and JS\'' +
' are unbroken')
' for JS once they are unbroken')
continue

if ('JS' in (producer.name, consumer.name) and
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,17 @@
import org.apache.arrow.vector.AddOrGetResult;
import org.apache.arrow.vector.FieldVector;
import org.apache.arrow.vector.ValueVector;
import org.apache.arrow.vector.ZeroVector;
import org.apache.arrow.vector.complex.impl.UnionMapReader;
import org.apache.arrow.vector.complex.impl.UnionMapWriter;
import org.apache.arrow.vector.types.Types;
import org.apache.arrow.vector.types.Types.MinorType;
import org.apache.arrow.vector.types.pojo.ArrowType.ArrowTypeID;
import org.apache.arrow.vector.types.pojo.ArrowType.Map;
import org.apache.arrow.vector.types.pojo.Field;
import org.apache.arrow.vector.types.pojo.FieldType;
import org.apache.arrow.vector.util.CallBack;
import org.apache.arrow.vector.util.SchemaChangeRuntimeException;

/**
* A MapVector is used to store entries of key/value pairs. It is a container vector that is
Expand All @@ -46,6 +49,9 @@ public class MapVector extends ListVector {
public static final String KEY_NAME = "key";
public static final String VALUE_NAME = "value";

// TODO: this is only used for addOrGetVector because ListVector declares it private
protected CallBack callBack;

/**
* Construct an empty MapVector with no data. Child vectors must be added subsequently.
*
Expand All @@ -68,6 +74,7 @@ public static MapVector empty(String name, BufferAllocator allocator, boolean ke
*/
public MapVector(String name, BufferAllocator allocator, FieldType fieldType, CallBack callBack) {
super(name, allocator, fieldType, callBack);
this.callBack = callBack;
reader = new UnionMapReader(this);
}

Expand Down Expand Up @@ -121,9 +128,29 @@ public UnionMapReader getReader() {
*/
@Override
public <T extends ValueVector> AddOrGetResult<T> addOrGetVector(FieldType fieldType) {
AddOrGetResult<T> result = super.addOrGetVector(fieldType);

// TODO: can call super method once DATA_VECTOR_NAME is configurable
boolean created = false;
if (vector instanceof ZeroVector) {
vector = fieldType.createNewSingleVector("entries", allocator, callBack);
Copy link
Member Author

Choose a reason for hiding this comment

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

cc @pravindra on the name change for MapVector struct field from "$data$" to "entries"

// returned vector must have the same field
created = true;
if (callBack != null &&
// not a schema change if changing from ZeroVector to ZeroVector
(fieldType.getType().getTypeID() != ArrowTypeID.Null)) {
callBack.doWork();
}
}

if (vector.getField().getType().getTypeID() != fieldType.getType().getTypeID()) {
final String msg = String.format("Inner vector type mismatch. Requested type: [%s], actual type: [%s]",
fieldType.getType().getTypeID(), vector.getField().getType().getTypeID());
throw new SchemaChangeRuntimeException(msg);
}

reader = new UnionMapReader(this);
return result;

return new AddOrGetResult<>((T) vector, created);
}

/**
Expand Down