Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use correct reader schema in AvroBinaryInputStream #817

Merged
merged 1 commit into from
Jan 23, 2024

Conversation

mberndt123
Copy link
Contributor

@mberndt123 mberndt123 commented Jan 23, 2024

Hey there,

My coworker @maxstreese recently tried backward compatibility using avro4s, but it didn't work out. Here's his code:

import com.sksamuel.avro4s.*
import java.io.*

@AvroDoc("initial")
@AvroName("Data")
case class Initial(a: Int, b: String)

@AvroDoc("updated")
@AvroName("Data")
case class Updated(a: Int, c: String = "foo")


@main
def main(): Unit =
  val buffer = ByteArrayOutputStream()

  // producer
  val out = AvroOutputStream.binary[Initial].to(buffer).build()
  out.write(Initial(1, "x"))
  out.close()

  // consumer
  val in = new AvroBinaryInputStream[Updated](ByteArrayInputStream(buffer.toByteArray), AvroSchema[Initial])
  println(in.iterator.next)

Clearly this should work because removed fields don't affect backward compatibility and there is a default for the new field c. But it doesn't work because AvroBinaryInputStream doesn't supply the reader schema to the decoder.

This PR adds a new constructor for AvroBinaryInputStream that allows supplying a reader schema.
I also got rid of one of those ugly asInstanceOf calls while I was at it.

Here are some related issues that this PR doesn't fix:

  • I don't know how to fix the helper methods in the AvroInputStream object because that would break binary compatibility
  • AvroDataInputStream is probably suffering the same issue

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@sksamuel sksamuel merged commit 2beb8cb into sksamuel:master Jan 23, 2024
1 check passed
@sksamuel
Copy link
Owner

Nice!

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.

None yet

2 participants