Skip to content

Commit

Permalink
fix: Only register inequality comparisons for orderable types (#11784)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #11784

Inequalities only work with types that are orderable pretty much by definition.

For maps in particular we allow users to call map1 < map2 but it will always throw because the map
type does not support inequalities unless null is treated as a value (which it is not in GenericView's
compare function which is what gets invoked).

This will address #11549 where inequalities on
maps can be generated in fuzzers and result in exceptions.

Reviewed By: xiaoxmeng

Differential Revision: D66909744

fbshipit-source-id: cc32dea19939a96a4a01afde050c5664fefb73b3
  • Loading branch information
Kevin Wilfong authored and facebook-github-bot committed Dec 9, 2024
1 parent 929affe commit 5739a6d
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,20 +52,22 @@ void registerComparisonFunctions(const std::string& prefix) {

registerNonSimdizableScalar<LtFunction, bool>({prefix + "lt"});
VELOX_REGISTER_VECTOR_FUNCTION(udf_simd_comparison_lt, prefix + "lt");
registerFunction<LtFunction, bool, Generic<T1>, Generic<T1>>({prefix + "lt"});
registerFunction<LtFunction, bool, Orderable<T1>, Orderable<T1>>(
{prefix + "lt"});

registerNonSimdizableScalar<GtFunction, bool>({prefix + "gt"});
VELOX_REGISTER_VECTOR_FUNCTION(udf_simd_comparison_gt, prefix + "gt");
registerFunction<GtFunction, bool, Generic<T1>, Generic<T1>>({prefix + "gt"});
registerFunction<GtFunction, bool, Orderable<T1>, Orderable<T1>>(
{prefix + "gt"});

registerNonSimdizableScalar<LteFunction, bool>({prefix + "lte"});
VELOX_REGISTER_VECTOR_FUNCTION(udf_simd_comparison_lte, prefix + "lte");
registerFunction<LteFunction, bool, Generic<T1>, Generic<T1>>(
registerFunction<LteFunction, bool, Orderable<T1>, Orderable<T1>>(
{prefix + "lte"});

registerNonSimdizableScalar<GteFunction, bool>({prefix + "gte"});
VELOX_REGISTER_VECTOR_FUNCTION(udf_simd_comparison_gte, prefix + "gte");
registerFunction<GteFunction, bool, Generic<T1>, Generic<T1>>(
registerFunction<GteFunction, bool, Orderable<T1>, Orderable<T1>>(
{prefix + "gte"});

registerFunction<DistinctFromFunction, bool, Generic<T1>, Generic<T1>>(
Expand Down
18 changes: 0 additions & 18 deletions velox/functions/prestosql/tests/ComparisonsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -724,24 +724,6 @@ TEST_F(ComparisonsTest, gtLtArray) {
testCompareArray<double>({NaN}, {NaN, 3.0}, expectedResults);
}

TEST_F(ComparisonsTest, gtLtMap) {
// Comparing maps is not supported for any types.
auto expectedException = "Map is not orderable type";

std::optional<std::vector<std::pair<int64_t, std::optional<int64_t>>>> map1 =
{{{1, 2}, {3, 4}}};
std::optional<std::vector<std::pair<int64_t, std::optional<int64_t>>>> map2 =
{{{1, 2}, {3, 4}}};
testCompareMap<int64_t>(map1, map2, expectedException);

static const auto NaN = std::numeric_limits<double>::quiet_NaN();
std::optional<std::vector<std::pair<double, std::optional<double>>>> map3 = {
{{1.0, 2.0}, {NaN, 4.0}}};
std::optional<std::vector<std::pair<double, std::optional<double>>>> map4 = {
{{1.0, 2.0}, {3, 4.0}}};
testCompareMap<double>(map3, map4, expectedException);
}

TEST_F(ComparisonsTest, distinctFrom) {
auto input = makeRowVector({
makeNullableFlatVector<int64_t>({3, 1, 1, std::nullopt, std::nullopt}),
Expand Down

0 comments on commit 5739a6d

Please sign in to comment.