[BEAM-8146,BEAM-8204,BEAM-8205] Add equals and hashCode to SchemaCoder and RowCoder

[TheNeuralBit]

• Adds a typeDescriptor parameter to SchemaCoder, and plumbs through a descriptor everywhere instances are created (3bafcb0).
• Modifies GetterBasedSchemaProvider so that the toRow/fromRow functions it creates have a meaningful equals and hashCode function (d415795).
• Adds an equals function to SchemaCoder that checks schema, typeDescriptor, fromRow, and toRow.
• Adds some tests for the above features
  • Each sub-class of GetterBasedSchemaProvider has at least one test checking that toRow and fromRow are equal after serialization (36a6014).
  • Adds SchemaCoderTest which checks some instances of SchemaCoder with CoderProperties.{coderSerializable, coderConsistentWithEquals} (ab18bd5).
• This change resolves BEAM-8204 and BEAM-8205, so the PR also re-enables AvroSchemaTest.testAvroPipelineGroupBy for Flink and Apex (17c2646).

Post-Commit Tests Status (on master branch)

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

Pre-Commit Tests Status (on master branch)

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

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

[TheNeuralBit]

Run Flink ValidatesRunner

[TheNeuralBit]

Run Apex ValidatesRunner

[TheNeuralBit]

Run Flink ValidatesRunner

[TheNeuralBit]

Run Apex ValidatesRunner

[TheNeuralBit]

Run Apex ValidatesRunner

[TheNeuralBit]

Run Flink ValidatesRunner

[TheNeuralBit]

R: @reuvenlax

I know you said Flink/Apex shouldn't be relying on coder equality and that should be the real fix for BEAM-8204 and BEAM-8205, but I think this is helpful for testing anyway.

[TheNeuralBit]

Run Flink ValidatesRunner

[TheNeuralBit]

Run Apex ValidatesRunner

[reuvenlax]

is this ready for review?

[TheNeuralBit]

Yep!

[kennknowles]

Thing to watch out for is update (in)compatibility. It happened once when a contribution made a good refactor.

[kennknowles]

Since it is already a CustomCoder hence not really update compatible, this is g2g after you resolve the rebase.

[TheNeuralBit]

Thanks @kennknowles, I resolved the merge conflicts so I think this is ready.

It may be good to get a review from @reuvenlax before merging though, at least an approval of the high-level approach. Plumbing through the type descriptor and changing the GetterBasedSchemaProvider interface is a pretty substantial change. The latter one will also cause a small conflict with #8690.

[kennknowles]

Interesting. If toRow and fromRow are equal, is it possible for the type descriptor to be different? (other than, I suppose, loss of generics reflection information due to erasure)

[TheNeuralBit]

That's a good point, I don't think so. Initially I added the type descriptor as something we can verify as a proxy for toRow/fromRow but now that I've made the functions created by our SchemaProvider implementations comparable, that check is redundant. Do you think I should back out the type descriptor changes to make this simpler? It's mostly its own commit so it wouldn't be too hard.

[kennknowles]

I would say keep as-is. Being able to implement getEncodedTypeDescriptor adds value on its own.

[kennknowles]

Given how long you've been waiting on this, I'm inclined to merge. Last chance to comment @reuvenlax ?

[TheNeuralBit]

+1 on merging @kennknowles