[BEAM-2303] Support SpecificData in AvroCoder

[Amar3tto]

Added SpecificDatumReader and SpecificDatumWriter initialization in AvroCoder constructor.

---

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

• Choose reviewer(s) and mention them in a comment (R: @username).
• Format the pull request title like [BEAM-XXX] Fixes bug in ApproximateQuantiles, where you replace BEAM-XXX with the appropriate JIRA issue, if applicable. This will automatically link the pull request to the issue.
• Update CHANGES.md with noteworthy changes.
• If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

Post-Commit Tests Status (on master branch)

Lang	SDK	ULR	Dataflow	Flink	Samza	Spark	Twister2
Go		---	---		---		---
Java
Python		---			---		---
XLang		---			---		---

Pre-Commit Tests Status (on master branch)

---	Java	Python	Go	Website	Whitespace	Typescript
Non-portable
Portable	---		---	---	---	---

See .test-infra/jenkins/README for trigger phrase, status and link of all Jenkins jobs.

GitHub Actions Tests Status (on master branch)

See CI.md for more information about GitHub Actions CI.

[Amar3tto]

Run Java PreCommit

[KhaninArtur]

LGTM - seems pretty straight forward 🙂

[pabloem]

@Amar3tto@KhaninArtur I guess this is ready to be reviewed by a committer?

[KhaninArtur]

@Amar3tto@KhaninArtur I guess this is ready to be reviewed by a committer?

@pabloem, you're right, it's ready

[pabloem]

r: @iemejia@amaliujia

[pabloem]

@Amar3tto can you open the PR to not be a draft PR anymore?

[iemejia]

CC @RyanSkraba mind to take a look at this one since you know SpecificData pretty well

[RyanSkraba]

LGTM, thanks for including me in the review!

[RyanSkraba]

This is fun: if the AvroCoder is instantiated as AvroCoder<MySpecificRecord> it will behave differently than AvroCoder<GenericRecord>, even though MySpecificRecord is a GenericRecord. I can see this being a gotcha for some developers, but I think it's the right thing to do and I don't see an alternative.

[iemejia]

LGTM

I am really curious in the consequence of this (becuase of @RyanSkraba comment) when mapping from Avro <-> Beam Row. The PR seems right so we can merge it. In case that you can do a follow up testing conversions from SpecificData I will be glad to see how it behaves.

[clairemcginty]

Hi @iemejia / @pabloem / @Amar3tto . this PR created some hidden bugs for us upgrading from Beam 2.29.0 to 2.30.0. It changes the default CharSequence representation in decoded Avro string fields. When using ReflectDatum{Reader,Writer}, CharSequences are backed by default by Strings [1]. This switch to SpecificDatum{Reader,Writer} means that, unless the Avro field property java-class is set to java.lang.String for all String fields, the CharSequences are backed by default now by org.apache.avro.util.Utf8s [2]. a lot of our users were relying on the default representation being Strings and are now seeing runtime errors in pipelines. Finally, Utf8s aren't serializable so there's no default Coder implementation for them, so users would have to convert them to Java strings anyway if they wanted to do a GBK operation on an Avro field, for example. I created a quick Gist to demonstrate the problem: [3].

Is this something I could bring to the ***@ or ****@ mailing list? Let me know what you think.

[lukecwik]

It would be good start a thread on ***@***.***

…

On Thu, Jul 15, 2021 at 9:23 AM Claire McGinty ***@***.***> wrote: Hi @iemejia <https://github.com/iemejia> / @pabloem <https://github.com/pabloem> / @Amar3tto <https://github.com/Amar3tto> . this PR created some hidden bugs for us upgrading from Beam 2.29.0 to 2.30.0. It changes the default CharSequence representation in decoded Avro string fields. When using ReflectDatum{Reader,Writer}, CharSequences are backed by default by Strings [1] <https://github.com/apache/avro/blob/release-1.8.2/lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumReader.java#L229>. This switch to SpecificDatum{Reader,Writer} means that, unless the Avro field property java-class is set to java.lang.String for all String fields, the CharSequences are backed by default now by org.apache.avro.util.Utf8s [2] <https://github.com/apache/avro/blob/release-1.8.2/lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java#L408>. a lot of our users were relying on the default representation being Strings and are now seeing runtime errors in pipelines. Finally, Utf8s aren't serializable so there's no default Coder implementation for them, so users would have to convert them to Java strings anyway if they wanted to do a GBK operation on an Avro field, for example. I created a quick Gist to demonstrate the problem: [3] <https://gist.github.com/clairemcginty/97ee6b33c0b5633d5d42d29b1d057d85>. Is this something I could bring to the dev@ or user@ mailing list? Let me know what you think. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#14410 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACM4V3GYE4CQIN4BOMLPDVLTX4DR3ANCNFSM42I2LRHQ> .