Prevent StructLikeWrapper#equals from throwing an exception

[alexjo2144]

Fixes: #5064

[rdblue]

This will check nested field docs. Is that the intended behavior? I think you want something more like sameType, possibly also ignoring nullability.

I should also note that I'm not sure that this is a good idea. StructLikeWrapper is not intended for comparing two arbitrary structs.

[rdblue]

@alexjo2144, should we update this to check structure and types, but not nullability and docs?

[findepi]

You can do this just before return comparator.compare(this.struct, that
if all you want is to "Prevent equals from throwing an exception"

however, making equals compare by type too looks like a good change as well.
this, however, should be called out in the commit message.

[alexjo2144]

however, making equals compare by type too looks like a good change as well.
this, however, should be called out in the commit message.

That's a good point, and probably the risky part of this change. Before, two StructLikeWrapper where both contained null struct fields were considered equals even if they were based off of different types. After this change, that is not true.

I should also note that I'm not sure that this is a good idea. StructLikeWrapper is not intended for comparing two arbitrary structs.

Based on the class description it sounded like the whole point of the Wrapper is to be able to compare StructLikes using equals. But we don't have to do this if you'd rather leave it alone

[rdblue]

Based on the class description it sounded like the whole point of the Wrapper is to be able to compare StructLikes using equals.

The point was to compare structs that have the same type. Normally, we try to do type checking up front because a type describes a large group of rows. Rather than checking each row's type, we want to check types and then check rows. That said, this should be a fast check if the types are identical.

[findepi]

That said, this should be a fast check if the types are identical.

Do you mean we should do type == that.type first before invoking the equals?

[rdblue]

Do you mean we should do type == that.type first before invoking the equals?

No, I'm saying that is the first check done in equals (usually) so hopefully if you're using this correctly -- that is, with the same schema for all records -- the extra check would be cheap.