AVRO-3235: [java] Add regression test for enum resolution across namespaces - #3952
Open
iemejia wants to merge 2 commits into
Open
AVRO-3235: [java] Add regression test for enum resolution across namespaces#3952iemejia wants to merge 2 commits into
iemejia wants to merge 2 commits into
Conversation
…e conversions Every explicit IConvertible.ToXxx(IFormatProvider) implementation on AvroDecimal called Convert.ToXxx(this, provider). Because 'this' is an IConvertible, Convert.ToXxx(IConvertible, provider) calls back into the same IConvertible.ToXxx(provider) method, recursing until a StackOverflowException (which is uncatchable and crashes the process). Reproducer: Convert.ToByte(new AvroDecimal(0)). Route the 12 numeric/boolean conversions through IConvertible.ToType(type, provider) (the one method that performs the real conversion via Convert.ChangeType on the underlying decimal), and make ToString(provider) use the public ToString() override. Adds a test exercising all conversions.
…spaces Enums (and their enclosing records) that share the same simple name but live in different namespaces must still resolve, so data versioned by namespace stays backward/forward compatible. This behaviour is already correct in the resolver (enum matching uses the simple name), but there was no dedicated regression test for the cross-namespace case reported in AVRO-3235. Adds resolveEnumAcrossNamespaces: asserts the enum action is an EnumAdjust (not NAMES_DONT_MATCH) and does a full v1 -> v2 record round-trip where the reader adds a defaulted field, confirming the enum resolves and the default is inserted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
AVRO-3235 reported that schema evolution crashed when a record/enum kept the same simple name but changed namespace across versions (a common versioning pattern). The underlying behaviour has since been fixed — the resolver matches enums by their simple name (
Resolver.EnumAdjust.resolveusesgetName()), so cross-namespace enums resolve correctly. But there was no dedicated regression test for that case.I reproduced the original scenario on current
mainand confirmed it works (v1 record read by a v2 reader: the enum resolves and the added field takes its default). This PR locks that in.Test
Adds
TestResolver.resolveEnumAcrossNamespaces:EnumAdjust(not aNAMES_DONT_MATCHerror) fortest.v1.Status→test.v2.Status;test.v1.Simple→ reader recordtest.v2.Simplewith an extra defaulteddescriptionfield, asserting the enum resolves and the default is inserted.This guards against a regression, which is especially relevant given the related tension around name-vs-fullname matching (AVRO-3703).
Closes AVRO-3235.
JIRA: https://issues.apache.org/jira/browse/AVRO-3235