AVRO-4210: Fix BinaryData.compareBytes to treat bytes as unsigned

[jasonf20]

This is a regression fix introduced in 1.12.1

What is the purpose of the change

Fix a regression in BinaryData.compareBytes introduced here: https://github.com/apache/avro/pull/3126/files

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

Documentation

• Does this pull request introduce a new feature? no

[jasonf20]

cc: @martin-g @belugabehr

[Copilot]

Pull Request Overview

This PR fixes a regression in the BinaryData.compareBytes method introduced in version 1.12.1, where bytes were incorrectly being treated as signed instead of unsigned during comparison.

• Changed Arrays.compare to Arrays.compareUnsigned to ensure bytes are treated as unsigned values

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[martin-g]

Let's add some tests!

[jasonf20]

Let's add some tests!

Sure, added

[jasonf20]

I see that in the regression PR there are also usages of the method BinaryData.compareBytes that were replaced with direct calls to Arrays.compare. This PR doesn't address those, I don't know if comparing in those contexts needs to be unsigned or not, but the behaviour of those would have changed in the PR.

I noticed this issue since I use the BinaryData.compareBytes method directly. Do we need to revert the other changes as well?

[jasonf20]

@martin-g I added a second commit that re-uses BinaryData.compareBytes again to restore the original behaviour. Without this I believe users would get different ordering of their data after upgrading to 1.12.1.

It's probably important to merge and release this quickly before adoption of 1.12.1 is widespread. That we we don't end up changing the ordering and then changing the ordering back again.

[jasonf20]

Hi @belugabehr, any chance you can have a look at this?

@martin-g If @belugabehr isn't available can we move forward with merging. The change shouldn't be risky since it's simply reverting very recent changes.

[martin-g]

I'd prefer someone from the Java team to approve it first.

[RyanSkraba]

Hello, this looks like the right thing to do -- as a regression, it would be great to have a JIRA associated with it! I can create the JIRA, but if you'd like credit as reporter/fixer, let me know your JIRA id (and/or create one at https://selfserve.apache.org/jira-account.html)

[RyanSkraba]

I've verified that this restores the behaviour of compareBytes from 1.12.0 and LGTM.

[RyanSkraba]

Cherry-picked to branch-1.12.

[belugabehr]

Hello team, My sincere apologies for all the swirl on this. I've also been away on holiday recently - just getting caught up. It looks like the issue has been resolved, but I'll try to find time this week to ensure all loose ends are resolved. Again, very sorry for the oversight on this. I was too trusting of the unit tests and should have more carefully considered this change. Do note that it is indeed worthwhile to pursue this, just need to use the unsigned version of the API.

…

On Sun, Nov 23, 2025, 9:48 AM Ryan Skraba ***@***.***> wrote: *RyanSkraba* left a comment (apache/avro#3537) <#3537 (comment)> Hello, this looks like the right thing to do -- as a regression, it would be great to have a JIRA associated with it! I can create the JIRA, but if you'd like credit as reporter/fixer, let me know your JIRA id (and/or create one at https://selfserve.apache.org/jira-account.html) — Reply to this email directly, view it on GitHub <#3537 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC766E27NDS3ZTGKBDY5MGD36HCMDAVCNFSM6AAAAACKKV4G5WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTKNRYGAZTEOBQGA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[RyanSkraba]

I think the system is working well 😄 You made an improvement, somebody noticed a difference in behaviour and corrected it -- I'm grateful to both of you ❤️ and everybody wins!