connectors icon indicating copy to clipboard operation
connectors copied to clipboard

Add 'equivalent' for type comparison

Open kination opened this issue 3 years ago • 5 comments

Currently, comparison expressions fail with IllegalArgumentException for decimal type arguments that don’t have equal precision/scale. This should be valid (and is in Spark SQL). This impacts IN and all the binary comparison operators.

This change is to add equivalent method, to only be used internally by our code to compare data types.

Refer https://github.com/delta-io/connectors/issues/222 for detail.

kination avatar Aug 24 '22 03:08 kination

Please add a description explaining the need for this.

tdas avatar Aug 25 '22 14:08 tdas

Also, is this how Spark compares types?

tdas avatar Aug 25 '22 14:08 tdas

@tdas @allisonport-db thanks for review. Add some updates.

kination avatar Aug 29 '22 04:08 kination

Looks good usage wise. Can you add tests for decimals of the same value with different scales? These should be equal.

Can you also make sure the tests you've added fail without the fix? I think one test is comparing decimals of the same scale I don't think this is testing anything new compared to our existing tests.

allisonport-db avatar Aug 31 '22 23:08 allisonport-db

@allisonport-db I've updated few more test. Please check.

Also, could you give me some advice to fix on Failing because of negative scalastyle result issue in PR check?

kination avatar Sep 06 '22 08:09 kination

@allisonport-db updated, but wonder why build has been failed...

kination avatar Sep 27 '22 07:09 kination

Can you sign off your latest commit as detailed at the bottom of this page? https://github.com/delta-io/delta/blob/master/CONTRIBUTING.md

allisonport-db avatar Oct 07 '22 07:10 allisonport-db

@allisonport-db updated. Is sign-off need for every PR?

kination avatar Oct 11 '22 07:10 kination

Yes, I also added it to your PR description. Thanks, I'm merging this

allisonport-db avatar Oct 12 '22 02:10 allisonport-db