fix(spanner): compare null elements in primitive-array Value.equals#13826
fix(spanner): compare null elements in primitive-array Value.equals#13826fornwall wants to merge 1 commit into
Conversation
The five primitive-array Value implementations (BoolArrayImpl, Int64ArrayImpl, Float32ArrayImpl, Float64ArrayImpl, PgOidArrayImpl) compared only the backing primitive array in valueEquals, ignoring the BitSet that tracks which elements are null. Null elements are stored as the primitive default in the backing array, so for example int64Array(asList(null, 1L)) compared equal to int64Array(asList(0L, 1L)) even though the two serialize to different protos and (since valueHash does include the null mask) have different hash codes. This breaks the equals/hashCode contract, use of Value in hash-based collections, and Mutation.equals-based mutation diffing. Move the null-mask comparison into a shared valueEquals override on PrimitiveArrayImpl, mirroring the existing valueHash/arrayHash split, and turn the subclass overrides into arrayEquals. Signed-off-by: Fredrik Fornwall <fredrik@fornwall.net>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request fixes an issue in Value.java where equality checks for primitive arrays did not correctly account for null elements. Since null elements are stored as primitive defaults in the backing array, the valueEquals method in PrimitiveArrayImpl was updated to compare both the nulls bitmask and the backing arrays via a new abstract arrayEquals method. Corresponding subclass implementations were updated, and comprehensive equality tests were added in ValueTest.java to verify correct behavior. There are no review comments, so I have no feedback to provide.
|
(@Spannerteam: When reviewing, consider whether this behavior change is reasonable, even if it is technically a bug fix. If the existing behavior has been present for a long time, then changing it could have an effect on existing applications.) |
2 participants
Make the five primitive-array
Valueimplementations (BoolArrayImpl,Int64ArrayImpl,Float32ArrayImpl,Float64ArrayImpl,PgOidArrayImpl)consider the
nullvalues inequals(), to avoid treating primitive defaults(
0,0.0,false) as equal tonullvalues.