From 9e582372f85d6838067214de70bf6a6d6ef2b16c Mon Sep 17 00:00:00 2001 From: Piyush Narang Date: Fri, 17 Jun 2016 11:07:44 -0700 Subject: [PATCH 1/3] Proof of concept fixes --- .../apache/parquet/avro/AvroWriteSupport.java | 2 +- .../parquet/avro/TestReflectLogicalTypes.java | 2 +- .../org/apache/parquet/io/api/Binary.java | 47 +++++++++++-------- 3 files changed, 29 insertions(+), 22 deletions(-) diff --git a/parquet-avro/src/main/java/org/apache/parquet/avro/AvroWriteSupport.java b/parquet-avro/src/main/java/org/apache/parquet/avro/AvroWriteSupport.java index 7fcd88eb1b..59e978ddf5 100644 --- a/parquet-avro/src/main/java/org/apache/parquet/avro/AvroWriteSupport.java +++ b/parquet-avro/src/main/java/org/apache/parquet/avro/AvroWriteSupport.java @@ -364,7 +364,7 @@ private Binary fromAvroString(Object value) { Utf8 utf8 = (Utf8) value; return Binary.fromReusedByteArray(utf8.getBytes(), 0, utf8.getByteLength()); } - return Binary.fromString((CharSequence) value); + return Binary.fromString(value.toString()); } private static GenericData getDataModel(Configuration conf) { diff --git a/parquet-avro/src/test/java/org/apache/parquet/avro/TestReflectLogicalTypes.java b/parquet-avro/src/test/java/org/apache/parquet/avro/TestReflectLogicalTypes.java index 401e6987d4..c6986b477b 100644 --- a/parquet-avro/src/test/java/org/apache/parquet/avro/TestReflectLogicalTypes.java +++ b/parquet-avro/src/test/java/org/apache/parquet/avro/TestReflectLogicalTypes.java @@ -426,7 +426,7 @@ public void testWriteNullableUUID() throws IOException { read(REFLECT, nullableUuidStringSchema, test)); } - @Test(expected = ClassCastException.class) +// @Test(expected = ClassCastException.class) public void testWriteUUIDMissingLogicalType() throws IOException { Schema uuidSchema = SchemaBuilder.record(RecordWithUUID.class.getName()) .fields().requiredString("uuid").endRecord(); diff --git a/parquet-column/src/main/java/org/apache/parquet/io/api/Binary.java b/parquet-column/src/main/java/org/apache/parquet/io/api/Binary.java index 30787f0bbd..ab78e2019c 100644 --- a/parquet-column/src/main/java/org/apache/parquet/io/api/Binary.java +++ b/parquet-column/src/main/java/org/apache/parquet/io/api/Binary.java @@ -31,6 +31,7 @@ import java.nio.charset.StandardCharsets; import java.util.Arrays; +import org.apache.parquet.io.ParquetDecodingException; import org.apache.parquet.io.ParquetEncodingException; import static org.apache.parquet.bytes.BytesUtils.UTF8; @@ -213,8 +214,8 @@ public void writeTo(DataOutput out) throws IOException { } - private static class FromStringBinary extends ByteBufferBackedBinary { - public FromStringBinary(CharSequence value) { + private static class FromStringBinary extends ByteArrayBackedBinary { + public FromStringBinary(String value) { // reused is false, because we do not hold on to the buffer after // conversion, and nobody else has a handle to it super(encodeUTF8(value), false); @@ -233,10 +234,10 @@ protected CharsetEncoder initialValue() { } }; - private static ByteBuffer encodeUTF8(CharSequence value) { + private static byte[] encodeUTF8(String value) { try { - return ENCODER.get().encode(CharBuffer.wrap(value)); - } catch (CharacterCodingException e) { + return value.getBytes("UTF-8"); + } catch (UnsupportedEncodingException e) { throw new ParquetEncodingException("UTF-8 not supported.", e); } } @@ -386,16 +387,27 @@ public ByteBufferBackedBinary(ByteBuffer value, int offset, int length, boolean @Override public String toStringUsingUTF8() { - int limit = value.limit(); - value.limit(offset+length); - int position = value.position(); - value.position(offset); - // no corresponding interface to read a subset of a buffer, would have to slice it - // which creates another ByteBuffer object or do what is done here to adjust the - // limit/offset and set them back after - String ret = UTF8.decode(value).toString(); - value.limit(limit); - value.position(position); + String ret = null; + if (value.hasArray()) { + try { + byte [] bytes = getBytes(); + ret = new String(bytes, "UTF-8"); + } catch (UnsupportedEncodingException e) { + throw new ParquetDecodingException("UTF-8 not supported"); + } + } else { + int limit = value.limit(); + value.limit(offset+length); + int position = value.position(); + value.position(offset); + // no corresponding interface to read a subset of a buffer, would have to slice it + // which creates another ByteBuffer object or do what is done here to adjust the + // limit/offset and set them back after + ret = UTF8.decode(value).toString(); + value.limit(limit); + value.position(position); + } + return ret; } @@ -555,11 +567,6 @@ public static Binary fromByteBuffer(final ByteBuffer value) { } public static Binary fromString(String value) { - // this method is for binary backward-compatibility - return fromString((CharSequence) value); - } - - public static Binary fromString(CharSequence value) { return new FromStringBinary(value); } From 2d50c8cb5fe87d5d27112704e6794e3b696d4192 Mon Sep 17 00:00:00 2001 From: Piyush Narang Date: Thu, 23 Jun 2016 09:41:24 -0700 Subject: [PATCH 2/3] Update Binary approach --- .../src/main/java/org/apache/parquet/io/api/Binary.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/parquet-column/src/main/java/org/apache/parquet/io/api/Binary.java b/parquet-column/src/main/java/org/apache/parquet/io/api/Binary.java index ab78e2019c..c8ec5c1045 100644 --- a/parquet-column/src/main/java/org/apache/parquet/io/api/Binary.java +++ b/parquet-column/src/main/java/org/apache/parquet/io/api/Binary.java @@ -387,11 +387,10 @@ public ByteBufferBackedBinary(ByteBuffer value, int offset, int length, boolean @Override public String toStringUsingUTF8() { - String ret = null; + String ret; if (value.hasArray()) { try { - byte [] bytes = getBytes(); - ret = new String(bytes, "UTF-8"); + ret = new String(value.array(), value.arrayOffset() + offset, length, "UTF-8"); } catch (UnsupportedEncodingException e) { throw new ParquetDecodingException("UTF-8 not supported"); } From 43c5bddc0fcd7695ffcbde571e6f2a6a77cab842 Mon Sep 17 00:00:00 2001 From: Piyush Narang Date: Thu, 23 Jun 2016 13:51:18 -0700 Subject: [PATCH 3/3] Keep avro on char sequence --- .../apache/parquet/avro/AvroWriteSupport.java | 2 +- .../parquet/avro/TestReflectLogicalTypes.java | 2 +- .../org/apache/parquet/io/api/Binary.java | 47 ++++++++++++++----- 3 files changed, 38 insertions(+), 13 deletions(-) diff --git a/parquet-avro/src/main/java/org/apache/parquet/avro/AvroWriteSupport.java b/parquet-avro/src/main/java/org/apache/parquet/avro/AvroWriteSupport.java index 59e978ddf5..460565bb01 100644 --- a/parquet-avro/src/main/java/org/apache/parquet/avro/AvroWriteSupport.java +++ b/parquet-avro/src/main/java/org/apache/parquet/avro/AvroWriteSupport.java @@ -364,7 +364,7 @@ private Binary fromAvroString(Object value) { Utf8 utf8 = (Utf8) value; return Binary.fromReusedByteArray(utf8.getBytes(), 0, utf8.getByteLength()); } - return Binary.fromString(value.toString()); + return Binary.fromCharSequence((CharSequence) value); } private static GenericData getDataModel(Configuration conf) { diff --git a/parquet-avro/src/test/java/org/apache/parquet/avro/TestReflectLogicalTypes.java b/parquet-avro/src/test/java/org/apache/parquet/avro/TestReflectLogicalTypes.java index c6986b477b..401e6987d4 100644 --- a/parquet-avro/src/test/java/org/apache/parquet/avro/TestReflectLogicalTypes.java +++ b/parquet-avro/src/test/java/org/apache/parquet/avro/TestReflectLogicalTypes.java @@ -426,7 +426,7 @@ public void testWriteNullableUUID() throws IOException { read(REFLECT, nullableUuidStringSchema, test)); } -// @Test(expected = ClassCastException.class) + @Test(expected = ClassCastException.class) public void testWriteUUIDMissingLogicalType() throws IOException { Schema uuidSchema = SchemaBuilder.record(RecordWithUUID.class.getName()) .fields().requiredString("uuid").endRecord(); diff --git a/parquet-column/src/main/java/org/apache/parquet/io/api/Binary.java b/parquet-column/src/main/java/org/apache/parquet/io/api/Binary.java index c8ec5c1045..50b98c202e 100644 --- a/parquet-column/src/main/java/org/apache/parquet/io/api/Binary.java +++ b/parquet-column/src/main/java/org/apache/parquet/io/api/Binary.java @@ -214,7 +214,7 @@ public void writeTo(DataOutput out) throws IOException { } - private static class FromStringBinary extends ByteArrayBackedBinary { + private static class FromStringBinary extends ByteBufferBackedBinary { public FromStringBinary(String value) { // reused is false, because we do not hold on to the buffer after // conversion, and nobody else has a handle to it @@ -226,23 +226,44 @@ public String toString() { return "Binary{\"" + toStringUsingUTF8() + "\"}"; } - private static final ThreadLocal ENCODER = - new ThreadLocal() { - @Override - protected CharsetEncoder initialValue() { - return StandardCharsets.UTF_8.newEncoder(); - } - }; - - private static byte[] encodeUTF8(String value) { + private static ByteBuffer encodeUTF8(String value) { try { - return value.getBytes("UTF-8"); + return ByteBuffer.wrap(value.getBytes("UTF-8")); } catch (UnsupportedEncodingException e) { throw new ParquetEncodingException("UTF-8 not supported.", e); } } } + private static class FromCharSequenceBinary extends ByteBufferBackedBinary { + public FromCharSequenceBinary(CharSequence value) { + // reused is false, because we do not hold on to the buffer after + // conversion, and nobody else has a handle to it + super(encodeUTF8(value), false); + } + + @Override + public String toString() { + return "Binary{\"" + toStringUsingUTF8() + "\"}"; + } + + private static final ThreadLocal ENCODER = + new ThreadLocal() { + @Override + protected CharsetEncoder initialValue() { + return StandardCharsets.UTF_8.newEncoder(); + } + }; + + private static ByteBuffer encodeUTF8(CharSequence value) { + try { + return ENCODER.get().encode(CharBuffer.wrap(value)); + } catch (CharacterCodingException e) { + throw new ParquetEncodingException("UTF-8 not supported.", e); + } + } + } + public static Binary fromReusedByteArray(final byte[] value, final int offset, final int length) { return new ByteArraySliceBackedBinary(value, offset, length, true); } @@ -569,6 +590,10 @@ public static Binary fromString(String value) { return new FromStringBinary(value); } + public static Binary fromCharSequence(CharSequence value) { + return new FromCharSequenceBinary(value); + } + /** * @see {@link Arrays#hashCode(byte[])} * @param array