Skip to content

[BEAM-5866] Override structuralValue in ListCoder and MapCoder#6862

Merged
robertwb merged 2 commits intoapache:masterfrom
kanterov:kanterov_structuralValue
Dec 10, 2018
Merged

[BEAM-5866] Override structuralValue in ListCoder and MapCoder#6862
robertwb merged 2 commits intoapache:masterfrom
kanterov:kanterov_structuralValue

Conversation

@kanterov
Copy link
Member

@kanterov kanterov commented Oct 28, 2018

  • use underlying coders in consistentWithEquals
  • allocate objects if needed instead of relying on serialization in structruralValue

Follow this checklist to help us incorporate your contribution quickly and easily:

  • 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.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

It will help us expedite review of your Pull Request if you tag someone (e.g. @username) to look at it.

Post-Commit Tests Status (on master branch)

Lang SDK Apex Dataflow Flink Gearpump Samza Spark
Go Build Status --- --- --- --- --- ---
Java Build Status Build Status Build Status Build Status Build Status Build Status Build Status Build Status
Python Build Status --- Build Status
Build Status
Build Status --- --- ---

List coders are consistent with equals if underlying element coders are.
Allocate objects in structuralValue only if needed.
Map coders are consistent with equals if underlying coders are.
Allocate objects in structuralValue only if needed.
@kanterov kanterov force-pushed the kanterov_structuralValue branch from b324ac1 to 8b4f60e Compare October 28, 2018 13:31
@kanterov kanterov mentioned this pull request Oct 28, 2018
2 tasks
@robertwb robertwb self-requested a review December 10, 2018 13:37
Copy link
Contributor

@robertwb robertwb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@robertwb robertwb merged commit 7a394cf into apache:master Dec 10, 2018
@kennknowles
Copy link
Member

kennknowles commented Dec 10, 2018

Wow, I'm actually shocked by this. I thought certainly that StructuredCoder already contained this logic but it certainly doesn't.

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.

3 participants