You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by "RustedBones (via GitHub)" <gi...@apache.org> on 2024/12/18 11:55:37 UTC
[PR] [java] Fix avro bytes conversion for BQ storage [beam]
RustedBones opened a new pull request, #33412:
URL: https://github.com/apache/beam/pull/33412
Avro byte is represented with java ByteBuffer.
Current implementation expects a byte array. This fails in practice with a ClassCastException:
`class java.nio.HeapByteBuffer cannot be cast to class [B`
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [java] Fix avro bytes conversion for BQ storage [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33412:
URL: https://github.com/apache/beam/pull/33412#issuecomment-2562595911
Reminder, please take a look at this pr: @Abacn @damondouglas
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [java] Fix avro bytes conversion for BQ storage [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on PR #33412:
URL: https://github.com/apache/beam/pull/33412#issuecomment-2562857518
actually is it worth to do a fix similar to #26647 in case byte[] is also possible ?
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [java] Fix avro bytes conversion for BQ storage [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33412:
URL: https://github.com/apache/beam/pull/33412#issuecomment-2569133948
Reminder, please take a look at this pr: @Abacn @damondouglas
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [java] Fix avro bytes conversion for BQ storage [beam]
Posted by "razvanculea (via GitHub)" <gi...@apache.org>.
razvanculea commented on PR #33412:
URL: https://github.com/apache/beam/pull/33412#issuecomment-2607439690
@Abacn , @RustedBones , the change from bytes[] to ByteBuffer broke the [BigQueryIOLT](https://github.com/apache/beam/blob/master/it/google-cloud-platform/src/test/java/org/apache/beam/it/gcp/bigquery/BigQueryIOLT.java).java for the STORAGE_WRITE_API tests.
Should I also change the BigQueryIOLT to ByteBuffer to write ? (it's one line to change)
What about users already writing the old way, bytes[] with STORAGE_WRITE_API ? To update to the newest SDK post this change they'll need to wrap their bytes[] into ByteBuffer. (this is right, but was not needed before)
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [java] Fix avro bytes conversion for BQ storage [beam]
Posted by "RustedBones (via GitHub)" <gi...@apache.org>.
RustedBones commented on PR #33412:
URL: https://github.com/apache/beam/pull/33412#issuecomment-2607378978
avro record set with `byte[]` can't be serialized by the avro coder.
if user was setting a byte array on a field, this could only work when element is passed straight for conversion, which relies on non deterministic step fusion. Even the test framework should catch that because it tests that elements can be serialized at boundaries.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [java] Fix avro bytes conversion for BQ storage [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn merged PR #33412:
URL: https://github.com/apache/beam/pull/33412
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [java] Fix avro bytes conversion for BQ storage [beam]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #33412:
URL: https://github.com/apache/beam/pull/33412#issuecomment-2551287857
Assigning reviewers. If you would like to opt out of this review, comment `assign to next reviewer`:
R: @Abacn for label java.
R: @damondouglas for label io.
Available commands:
- `stop reviewer notifications` - opt out of the automated review tooling
- `remind me after tests pass` - tag the comment author after tests pass
- `waiting on author` - shift the attention set back to the author (any comment or push by the author will return the attention set to the reviewers)
The PR bot will only process comments in the main thread (not review comments).
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [java] Fix avro bytes conversion for BQ storage [beam]
Posted by "Abacn (via GitHub)" <gi...@apache.org>.
Abacn commented on PR #33412:
URL: https://github.com/apache/beam/pull/33412#issuecomment-2573305257
acknowledged. Thank you!
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [java] Fix avro bytes conversion for BQ storage [beam]
Posted by "RustedBones (via GitHub)" <gi...@apache.org>.
RustedBones commented on PR #33412:
URL: https://github.com/apache/beam/pull/33412#issuecomment-2572787157
That shouldn't be necessary. In `GenericRecords`, bytes MUST be of `ByteBuffer` type. See [here](https://github.com/apache/avro/blob/d26a3275bb2cc51337f9fb242ce93ed6ed8b199b/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumWriter.java#L400) for instance.
Byte arrays are only supported with avro reflect records which are not accepted by the beam API.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [java] Fix avro bytes conversion for BQ storage [beam]
Posted by "liferoad (via GitHub)" <gi...@apache.org>.
liferoad commented on PR #33412:
URL: https://github.com/apache/beam/pull/33412#issuecomment-2607362401
> actually is it worth to do a fix similar to #26647 in case byte[] is also possible ?
If this breaks the existing user's code, I think we should support both byte[] and ByteBuffer
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [java] Fix avro bytes conversion for BQ storage [beam]
Posted by "RustedBones (via GitHub)" <gi...@apache.org>.
RustedBones commented on PR #33412:
URL: https://github.com/apache/beam/pull/33412#issuecomment-2607466362
Ok, If the record comes from the format fuction, it bypasses serialization. User could have created broken records because that was the only way to support bytes. Can add back `bytes[]` in there even if not accepted by avro itself
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org