sankarh commented on a change in pull request #2689:
URL: https://github.com/apache/hive/pull/2689#discussion_r721068047
##########
File path:
ql/src/java/org/apache/hadoop/hive/ql/io/arrow/ArrowColumnarBatchSerDe.java
##########
@@ -170,7 +171,7 @@ private static Field toField(String name, TypeInfo
typeInfo) {
for (int i = 0; i < structSize; i++) {
structFields.add(toField(fieldNames.get(i), fieldTypeInfos.get(i)));
}
- return new Field(name, FieldType.nullable(MinorType.STRUCT.getType()),
structFields);
+ return new Field(name, new FieldType(false, new ArrowType.Struct(),
null), structFields);
Review comment:
I think, this point holds good only if Struct is a child element type of
Map column. If the column type is STRUCT, then it should be nullable. Can we
make this change only for first case?
##########
File path:
ql/src/java/org/apache/hadoop/hive/llap/WritableByteChannelAdapter.java
##########
@@ -93,7 +93,7 @@ public int write(ByteBuffer src) throws IOException {
int size = src.remaining();
//Down the semaphore or block until available
takeWriteResources(1);
- ByteBuf buf = allocator.buffer(size);
+ ByteBuf buf = allocator.getAsByteBufAllocator().buffer(size);
Review comment:
Why this change relevant to this jira?
##########
File path: ql/src/java/org/apache/hadoop/hive/ql/io/arrow/Serializer.java
##########
@@ -226,7 +226,7 @@ public ArrowWrapperWritable
serializeBatch(VectorizedRowBatch vectorizedRowBatch
}
private static FieldType toFieldType(TypeInfo typeInfo) {
- return new FieldType(true, toArrowType(typeInfo), null);
+ return new FieldType(false, toArrowType(typeInfo), null);
Review comment:
I think, even this change should come only if list or struct are
elements of other complex data types. We cannot make it global for all data
types.
##########
File path:
ql/src/java/org/apache/hadoop/hive/ql/io/arrow/ArrowColumnarBatchSerDe.java
##########
@@ -185,7 +186,7 @@ private static Field toField(String name, TypeInfo
typeInfo) {
final TypeInfo keyTypeInfo = mapTypeInfo.getMapKeyTypeInfo();
final TypeInfo valueTypeInfo = mapTypeInfo.getMapValueTypeInfo();
final StructTypeInfo mapStructTypeInfo = new StructTypeInfo();
- mapStructTypeInfo.setAllStructFieldNames(Lists.newArrayList("keys",
"values"));
+ mapStructTypeInfo.setAllStructFieldNames(Lists.newArrayList("key",
"value"));
Review comment:
Make sense.
##########
File path:
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniLlapArrow.java
##########
@@ -123,8 +123,8 @@ public static void afterTest() {
return new LlapArrowRowInputFormat(Long.MAX_VALUE);
}
- // Currently MAP type is not supported. Add it back when Arrow 1.0 is
released.
- // See: SPARK-21187
+ // Currently, loading from a text file gives errors with Map dataType.
+ // This needs to be fixed when adding support for non-ORC writes (text and
parquet) for the llap-ext-client.
Review comment:
This comment doesn't make sense here as this test is not specific to
llap-ext-client. Instead mention the Hive JIRA which would solve this issue.
##########
File path:
itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcWithMiniLlapArrow.java
##########
@@ -123,8 +123,8 @@ public static void afterTest() {
return new LlapArrowRowInputFormat(Long.MAX_VALUE);
}
- // Currently MAP type is not supported. Add it back when Arrow 1.0 is
released.
- // See: SPARK-21187
+ // Currently, loading from a text file gives errors with Map dataType.
+ // This needs to be fixed when adding support for non-ORC writes (text and
parquet) for the llap-ext-client.
Review comment:
Ideally, anything that works for ORC should work for non-ORC as well.
Pls enable the tests on Map data type and see if it works after Arrow upgrade.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
