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

Incompatible changes with records in unions are not detected #253

Closed
steven-aerts opened this issue Nov 19, 2015 · 8 comments
Closed

Incompatible changes with records in unions are not detected #253

steven-aerts opened this issue Nov 19, 2015 · 8 comments

Comments

@steven-aerts
Copy link

@steven-aerts steven-aerts commented Nov 19, 2015

I think I found an issue with the schema compatibility checking of avro schema's.
An incompatible record evolution wrapped in a union is ignored by the registry.
This is clearly incompatible according to the avro schema evolution rules.
It is also flagged as incompatible when I use checkReaderWriterCompatibility(reader, writer) from avro.

I created a unit test which demostrats this issue:

@Test
public void testUnionCompatibility() {
  Schema schema1 = SchemaBuilder.unionOf()
          .record("myrecord")
          .fields()
              .name("f1").type().stringType().noDefault()
          .endRecord()
          .endUnion();

  Schema schema2 = SchemaBuilder.unionOf()
          .record("myrecord")
          .fields()
              .name("f1").type().stringType().noDefault()
              .name("f2").type().stringType().stringDefault("")
          .endRecord()
          .endUnion();

  Schema schema3 = SchemaBuilder.unionOf()
          .record("myrecord")
          .fields()
              .name("f1").type().stringType().noDefault()
              .name("f2").type().stringType().noDefault()
          .endRecord()
          .endUnion();

  assertEquals(SchemaCompatibility.SchemaCompatibilityType.COMPATIBLE, SchemaCompatibility.checkReaderWriterCompatibility(schema2, schema1).getType());
  AvroCompatibilityChecker backwardChecker = AvroCompatibilityChecker.BACKWARD_CHECKER;
  assertTrue("adding a field with default is a backward compatible change",
          backwardChecker.isCompatible(schema2, schema1));

  assertEquals(SchemaCompatibility.SchemaCompatibilityType.INCOMPATIBLE, SchemaCompatibility.checkReaderWriterCompatibility(schema3, schema1).getType());
  assertFalse("adding a field w/o default is not a backward compatible change",
          backwardChecker.isCompatible(schema3, schema1));
}
@steven-aerts

This comment has been minimized.

Copy link
Author

@steven-aerts steven-aerts commented Nov 23, 2015

Looking at the code again I bumped into a related problem with the current code.

The following assert should fail:

assertFalse("changing field name is not a backward compatible change", backwardChecker.isCompatible(schema4, schema1));

as schema1 and schema4 are compatible, because schema4 contains an alias for the renamed field f1.
However the current code and its test claims that it is not.

@martinboyleie

This comment has been minimized.

Copy link

@martinboyleie martinboyleie commented Mar 22, 2016

Also seeing the same issue. Think it needs to be resolved in the Avro library. Logged https://issues.apache.org/jira/browse/AVRO-1815

@aludwiko

This comment has been minimized.

Copy link

@aludwiko aludwiko commented Nov 25, 2016

@steven-aerts #253 (comment) you are right about this, please check: #453

@steven-aerts

This comment has been minimized.

Copy link
Author

@steven-aerts steven-aerts commented Nov 26, 2016

I agree, I should have created a separate issue for this.

@skliarpawlo

This comment has been minimized.

Copy link

@skliarpawlo skliarpawlo commented Mar 12, 2017

Why such a quite severe bug didn't get attention yet? Does this have any workarounds? I still see the PR to https://issues.apache.org/jira/browse/AVRO-1815 in 'Open' state

@mausch

This comment has been minimized.

Copy link

@mausch mausch commented Sep 18, 2019

AVRO-1815 has been resolved since Nov 2017.
Perhaps Schema Registry needs to update Avro to fix this?

@maximann

This comment has been minimized.

Copy link

@maximann maximann commented Oct 28, 2019

+1, this is quite severe and needs to be addressed in schema registry as well.

@rayokota

This comment has been minimized.

Copy link
Member

@rayokota rayokota commented Oct 28, 2019

Avro has been upgraded to 1.9.1 by #1228

@rayokota rayokota closed this Oct 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants
You can’t perform that action at this time.