Skip to content

PARQUET-415: Fix ByteBuffer Binary serialization. - #305

Closed
rdblue wants to merge 1 commit into
apache:masterfrom
rdblue:PARQUET-415-fix-bytebuffer-binary-serialization
Closed

PARQUET-415: Fix ByteBuffer Binary serialization.#305
rdblue wants to merge 1 commit into
apache:masterfrom
rdblue:PARQUET-415-fix-bytebuffer-binary-serialization

Conversation

@rdblue

@rdblue rdblue commented Dec 22, 2015

Copy link
Copy Markdown
Contributor

This also adds a test to validate that serialization works for all
Binary objects that are already test cases.

@lw-lin

lw-lin commented Jan 27, 2016

Copy link
Copy Markdown
Contributor

+1, LGTM. Good to have testSerializable() for unit tests!

So, @rdblue , should we have more committers review on this, or can we get this committed? Cheers.

@rdblue

rdblue commented Jan 27, 2016

Copy link
Copy Markdown
Contributor Author

Thanks for reviewing, @proflin! We will need a committer to have a look as well, but it really helps to have more than one person review. Like we said in the sync-up today, we love it when non-committers help out with reviews because it makes everything go faster.

@lw-lin

lw-lin commented Jan 27, 2016

Copy link
Copy Markdown
Contributor

Sure. I'm familiar with the parquet-format/parquet-mr code base, and I would love to review more PRs. Things are going fast these days :)

BTW congratulations on your new journey at Netflix :-)

@julienledem

Copy link
Copy Markdown
Member

+1
Thanks for the review @proflin

This also adds a test to validate that serialization works for all
Binary objects that are already test cases.
@rdblue
rdblue force-pushed the PARQUET-415-fix-bytebuffer-binary-serialization branch from 65bdcf0 to 4e75d54 Compare February 3, 2016 19:53
@asfgit asfgit closed this in 0a711eb Feb 3, 2016
piyushnarang pushed a commit to piyushnarang/parquet-mr that referenced this pull request Jun 15, 2016
This also adds a test to validate that serialization works for all
Binary objects that are already test cases.

Author: Ryan Blue <blue@apache.org>

Closes apache#305 from rdblue/PARQUET-415-fix-bytebuffer-binary-serialization and squashes the following commits:

4e75d54 [Ryan Blue] PARQUET-415: Fix ByteBuffer Binary serialization.
rdblue added a commit to rdblue/parquet-mr that referenced this pull request Jan 6, 2017
This also adds a test to validate that serialization works for all
Binary objects that are already test cases.

Author: Ryan Blue <blue@apache.org>

Closes apache#305 from rdblue/PARQUET-415-fix-bytebuffer-binary-serialization and squashes the following commits:

4e75d54 [Ryan Blue] PARQUET-415: Fix ByteBuffer Binary serialization.
@jinxing64

Copy link
Copy Markdown

@rdblue
Thanks for fixing this problem, may I ask a question?

  1. What parquet version includes this change? is it 1.9.0?
  2. Without this change, what exception will be thrown? is it below?
Caused by: java.nio.BufferUnderflowException
        at java.nio.HeapByteBuffer.get(HeapByteBuffer.java:151)
        at java.nio.ByteBuffer.get(ByteBuffer.java:715)
        at org.apache.parquet.io.api.Binary$ByteBufferBackedBinary.getBytes(Binary.java:405)
        at org.apache.parquet.io.api.Binary$ByteBufferBackedBinary.getBytesUnsafe(Binary.java:414)
        at org.apache.parquet.io.api.Binary$ByteBufferBackedBinary.writeObject(Binary.java:484)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at java.io.ObjectStreamClass.invokeWriteObject(ObjectStreamClass.java:1028)
        at java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1496)
        at java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1432)
        at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1178)
        at java.io.ObjectOutputStream.defaultWriteFields(ObjectOutputStream.java:1548)
        at java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1509)
        at java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1432)
......

@rdblue

rdblue commented Apr 10, 2018

Copy link
Copy Markdown
Contributor Author

@jinxing64, this is fixed in 1.9.0. It could result in the trace you pasted, I think. I'm not sure if that's actually happening though.

@jinxing64

Copy link
Copy Markdown

@rdblue
Thanks a lot !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants