Split comparison into unordered first and last

trinodb · Oct 9, 2021 · 2e96304 · 2e96304
1 parent 81bdf7c
commit 2e96304
Show file tree

Hide file tree

Showing 65 changed files with 533 additions and 239 deletions.
diff --git a/core/trino-main/src/main/java/io/trino/metadata/FunctionRegistry.java b/core/trino-main/src/main/java/io/trino/metadata/FunctionRegistry.java
@@ -110,7 +110,8 @@
 import io.trino.operator.scalar.EmptyMapConstructor;
 import io.trino.operator.scalar.FailureFunction;
 import io.trino.operator.scalar.FormatNumberFunction;
-import io.trino.operator.scalar.GenericComparisonOperator;
+import io.trino.operator.scalar.GenericComparisonUnorderedFirstOperator;
+import io.trino.operator.scalar.GenericComparisonUnorderedLastOperator;
 import io.trino.operator.scalar.GenericDistinctFromOperator;
 import io.trino.operator.scalar.GenericEqualOperator;
 import io.trino.operator.scalar.GenericHashCodeOperator;
@@ -605,7 +606,8 @@ public FunctionRegistry(
                 .function(new GenericXxHash64Operator(typeOperators))
                 .function(new GenericDistinctFromOperator(typeOperators))
                 .function(new GenericIndeterminateOperator(typeOperators))
-                .function(new GenericComparisonOperator(typeOperators))
+                .function(new GenericComparisonUnorderedLastOperator(typeOperators))
+                .function(new GenericComparisonUnorderedFirstOperator(typeOperators))
                 .function(new GenericLessThanOperator(typeOperators))
                 .function(new GenericLessThanOrEqualOperator(typeOperators))
                 .function(new VersionFunction(nodeVersion))
@@ -766,7 +768,8 @@ public final synchronized void addFunctions(List<? extends SqlFunction> function
                     !(function instanceof GenericXxHash64Operator) &&
                     !(function instanceof GenericDistinctFromOperator) &&
                     !(function instanceof GenericIndeterminateOperator) &&
-                    !(function instanceof GenericComparisonOperator) &&
+                    !(function instanceof GenericComparisonUnorderedLastOperator) &&
+                    !(function instanceof GenericComparisonUnorderedFirstOperator) &&
                     !(function instanceof GenericLessThanOperator) &&
                     !(function instanceof GenericLessThanOrEqualOperator)) {
                 OperatorType operatorType = unmangleOperator(name);
@@ -775,7 +778,8 @@ public final synchronized void addFunctions(List<? extends SqlFunction> function
                                 operatorType != OperatorType.XX_HASH_64 &&
                                 operatorType != OperatorType.IS_DISTINCT_FROM &&
                                 operatorType != OperatorType.INDETERMINATE &&
-                                operatorType != OperatorType.COMPARISON &&
+                                operatorType != OperatorType.COMPARISON_UNORDERED_LAST &&
+                                operatorType != OperatorType.COMPARISON_UNORDERED_FIRST &&
                                 operatorType != OperatorType.LESS_THAN &&
                                 operatorType != OperatorType.LESS_THAN_OR_EQUAL,
                         "Can not register %s function: %s", operatorType, function.getFunctionMetadata().getSignature());

diff --git a/core/trino-main/src/main/java/io/trino/metadata/MetadataManager.java b/core/trino-main/src/main/java/io/trino/metadata/MetadataManager.java
@@ -182,7 +182,8 @@
 import static io.trino.spi.function.InvocationConvention.InvocationReturnConvention.FAIL_ON_NULL;
 import static io.trino.spi.function.InvocationConvention.InvocationReturnConvention.NULLABLE_RETURN;
 import static io.trino.spi.function.InvocationConvention.simpleConvention;
-import static io.trino.spi.function.OperatorType.COMPARISON;
+import static io.trino.spi.function.OperatorType.COMPARISON_UNORDERED_FIRST;
+import static io.trino.spi.function.OperatorType.COMPARISON_UNORDERED_LAST;
 import static io.trino.spi.function.OperatorType.EQUAL;
 import static io.trino.spi.function.OperatorType.HASH_CODE;
 import static io.trino.spi.function.OperatorType.INDETERMINATE;
@@ -2084,8 +2085,11 @@ public void verifyTypes()
                 }
             }
             if (type.isOrderable()) {
-                if (!hasComparisonMethod(type)) {
-                    missingOperators.put(type, COMPARISON);
+                if (!hasComparisonUnorderedLastMethod(type)) {
+                    missingOperators.put(type, COMPARISON_UNORDERED_LAST);
+                }
+                if (!hasComparisonUnorderedFirstMethod(type)) {
+                    missingOperators.put(type, COMPARISON_UNORDERED_FIRST);
                 }
                 if (!hasLessThanMethod(type)) {
                     missingOperators.put(type, LESS_THAN);
@@ -2163,10 +2167,21 @@ private boolean hasIndeterminateMethod(Type type)
         }
     }
 
-    private boolean hasComparisonMethod(Type type)
+    private boolean hasComparisonUnorderedLastMethod(Type type)
+    {
+        try {
+            typeOperators.getComparisonUnorderedLastOperator(type, simpleConvention(FAIL_ON_NULL, NEVER_NULL, NEVER_NULL));
+            return true;
+        }
+        catch (UnsupportedOperationException e) {
+            return false;
+        }
+    }
+
+    private boolean hasComparisonUnorderedFirstMethod(Type type)
     {
         try {
-            typeOperators.getComparisonOperator(type, simpleConvention(FAIL_ON_NULL, NEVER_NULL, NEVER_NULL));
+            typeOperators.getComparisonUnorderedFirstOperator(type, simpleConvention(FAIL_ON_NULL, NEVER_NULL, NEVER_NULL));
             return true;
         }
         catch (UnsupportedOperationException e) {

diff --git a/core/trino-main/src/main/java/io/trino/operator/DynamicFilterSourceOperator.java b/core/trino-main/src/main/java/io/trino/operator/DynamicFilterSourceOperator.java
@@ -187,7 +187,7 @@ private DynamicFilterSourceOperator(
             // Skipping DOUBLE and REAL in collectMinMaxValues to avoid dealing with NaN values
             if (minMaxCollectionLimit > 0 && type.isOrderable() && type != DOUBLE && type != REAL) {
                 minMaxChannelsBuilder.add(channelIndex);
-                minMaxComparisonsBuilder.add(blockTypeOperators.getComparisonOperator(type));
+                minMaxComparisonsBuilder.add(blockTypeOperators.getComparisonUnorderedLastOperator(type));
             }
             this.blockBuilders[channelIndex] = type.createBlockBuilder(null, EXPECTED_BLOCK_BUILDER_SIZE);
             this.valueSets[channelIndex] = createEqualityTypedSet(

diff --git a/core/trino-main/src/main/java/io/trino/operator/PagesIndex.java b/core/trino-main/src/main/java/io/trino/operator/PagesIndex.java
@@ -460,7 +460,7 @@ public PagesHashStrategy createPagesHashStrategy(List<Integer> joinChannels, Opt
     public PagesIndexComparator createChannelComparator(int leftChannel, int rightChannel)
     {
         checkArgument(types.get(leftChannel).equals(types.get(rightChannel)), "comparing channels of different types: %s and %s", types.get(leftChannel), types.get(rightChannel));
-        return new SimpleChannelComparator(leftChannel, rightChannel, blockTypeOperators.getComparisonOperator(types.get(leftChannel)));
+        return new SimpleChannelComparator(leftChannel, rightChannel, blockTypeOperators.getComparisonUnorderedLastOperator(types.get(leftChannel)));
     }
 
     public LookupSourceSupplier createLookupSourceSupplier(

diff --git a/core/trino-main/src/main/java/io/trino/operator/SimplePagesHashStrategy.java b/core/trino-main/src/main/java/io/trino/operator/SimplePagesHashStrategy.java
@@ -60,7 +60,7 @@ public SimplePagesHashStrategy(
     {
         this.types = ImmutableList.copyOf(requireNonNull(types, "types is null"));
         this.comparisonOperators = types.stream()
-                .map(type -> type.isOrderable() ? Optional.of(blockTypeOperators.getComparisonOperator(type)) : Optional.<BlockPositionComparison>empty())
+                .map(type -> type.isOrderable() ? Optional.of(blockTypeOperators.getComparisonUnorderedLastOperator(type)) : Optional.<BlockPositionComparison>empty())
                 .collect(toImmutableList());
         this.outputChannels = ImmutableList.copyOf(requireNonNull(outputChannels, "outputChannels is null"));
         this.channels = ImmutableList.copyOf(requireNonNull(channels, "channels is null"));

diff --git a/...o-main/src/main/java/io/trino/operator/aggregation/AbstractMinMaxAggregationFunction.java b/...o-main/src/main/java/io/trino/operator/aggregation/AbstractMinMaxAggregationFunction.java
@@ -56,9 +56,9 @@
 import static io.trino.spi.function.InvocationConvention.InvocationArgumentConvention.NEVER_NULL;
 import static io.trino.spi.function.InvocationConvention.InvocationReturnConvention.FAIL_ON_NULL;
 import static io.trino.spi.function.InvocationConvention.simpleConvention;
-import static io.trino.spi.function.OperatorType.COMPARISON;
 import static io.trino.util.Failures.internalError;
 import static io.trino.util.MinMaxCompare.getMinMaxCompare;
+import static io.trino.util.MinMaxCompare.getMinMaxCompareFunctionDependencies;
 import static io.trino.util.Reflection.methodHandle;
 
 public abstract class AbstractMinMaxAggregationFunction
@@ -106,9 +106,7 @@ protected AbstractMinMaxAggregationFunction(String name, boolean min, String des
     @Override
     public FunctionDependencyDeclaration getFunctionDependencies()
     {
-        return FunctionDependencyDeclaration.builder()
-                .addOperatorSignature(COMPARISON, ImmutableList.of(new TypeSignature("E"), new TypeSignature("E")))
-                .build();
+        return getMinMaxCompareFunctionDependencies(new TypeSignature("E"), min);
     }
 
     @Override

diff --git a/core/trino-main/src/main/java/io/trino/operator/aggregation/MaxNAggregationFunction.java b/core/trino-main/src/main/java/io/trino/operator/aggregation/MaxNAggregationFunction.java
@@ -15,8 +15,6 @@
 
 import io.trino.type.BlockTypeOperators;
 
-import static io.trino.util.MinMaxCompare.getMaxCompare;
-
 public class MaxNAggregationFunction
         extends AbstractMinMaxNAggregationFunction
 {
@@ -25,7 +23,7 @@ public class MaxNAggregationFunction
     public MaxNAggregationFunction(BlockTypeOperators blockTypeOperators)
     {
         super(NAME,
-                type -> getMaxCompare(blockTypeOperators, type),
+                blockTypeOperators::getComparisonUnorderedFirstOperator,
                 "Returns the maximum values of the argument");
     }
 }
diff --git a/core/trino-main/src/main/java/io/trino/operator/aggregation/MinNAggregationFunction.java b/core/trino-main/src/main/java/io/trino/operator/aggregation/MinNAggregationFunction.java
@@ -23,7 +23,7 @@ public class MinNAggregationFunction
     public MinNAggregationFunction(BlockTypeOperators blockTypeOperators)
     {
         super(NAME,
-                type -> blockTypeOperators.getComparisonOperator(type).reversed(),
+                type -> blockTypeOperators.getComparisonUnorderedFirstOperator(type).reversed(),
                 "Returns the minimum values of the argument");
     }
 }
diff --git a/core/trino-main/src/main/java/io/trino/operator/aggregation/minmaxby/AbstractMinMaxBy.java b/core/trino-main/src/main/java/io/trino/operator/aggregation/minmaxby/AbstractMinMaxBy.java
@@ -80,11 +80,11 @@
 import static io.trino.spi.function.InvocationConvention.InvocationArgumentConvention.NEVER_NULL;
 import static io.trino.spi.function.InvocationConvention.InvocationReturnConvention.FAIL_ON_NULL;
 import static io.trino.spi.function.InvocationConvention.simpleConvention;
-import static io.trino.spi.function.OperatorType.COMPARISON;
 import static io.trino.sql.gen.BytecodeUtils.loadConstant;
 import static io.trino.sql.gen.SqlTypeBytecodeExpression.constantType;
 import static io.trino.util.CompilerUtils.defineClass;
 import static io.trino.util.CompilerUtils.makeClassName;
+import static io.trino.util.MinMaxCompare.getMinMaxCompareFunctionDependencies;
 import static io.trino.util.Reflection.methodHandle;
 import static java.util.Arrays.stream;
 
@@ -120,9 +120,7 @@ protected AbstractMinMaxBy(boolean min, String description)
     @Override
     public FunctionDependencyDeclaration getFunctionDependencies()
     {
-        return FunctionDependencyDeclaration.builder()
-                .addOperatorSignature(COMPARISON, ImmutableList.of(new TypeSignature("K"), new TypeSignature("K")))
-                .build();
+        return getMinMaxCompareFunctionDependencies(new TypeSignature("K"), min);
     }
 
     @Override

diff --git a/...-main/src/main/java/io/trino/operator/aggregation/minmaxby/MaxByNAggregationFunction.java b/...-main/src/main/java/io/trino/operator/aggregation/minmaxby/MaxByNAggregationFunction.java
@@ -15,8 +15,6 @@
 
 import io.trino.type.BlockTypeOperators;
 
-import static io.trino.util.MinMaxCompare.getMaxCompare;
-
 public class MaxByNAggregationFunction
         extends AbstractMinMaxByNAggregationFunction
 {
@@ -25,7 +23,7 @@ public class MaxByNAggregationFunction
     public MaxByNAggregationFunction(BlockTypeOperators blockTypeOperators)
     {
         super(NAME,
-                type -> getMaxCompare(blockTypeOperators, type),
+                blockTypeOperators::getComparisonUnorderedFirstOperator,
                 "Returns the values of the first argument associated with the maximum values of the second argument");
     }
 }
diff --git a/...-main/src/main/java/io/trino/operator/aggregation/minmaxby/MinByNAggregationFunction.java b/...-main/src/main/java/io/trino/operator/aggregation/minmaxby/MinByNAggregationFunction.java
@@ -23,7 +23,7 @@ public class MinByNAggregationFunction
     public MinByNAggregationFunction(BlockTypeOperators blockTypeOperators)
     {
         super(NAME,
-                type -> blockTypeOperators.getComparisonOperator(type).reversed(),
+                type -> blockTypeOperators.getComparisonUnorderedLastOperator(type).reversed(),
                 "Returns the values of the first argument associated with the minimum values of the second argument");
     }
 }
diff --git a/core/trino-main/src/main/java/io/trino/operator/annotations/FunctionsParserHelper.java b/core/trino-main/src/main/java/io/trino/operator/annotations/FunctionsParserHelper.java
@@ -55,7 +55,8 @@
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
 import static com.google.common.collect.ImmutableSortedSet.toImmutableSortedSet;
 import static io.trino.operator.annotations.ImplementationDependency.isImplementationDependencyAnnotation;
-import static io.trino.spi.function.OperatorType.COMPARISON;
+import static io.trino.spi.function.OperatorType.COMPARISON_UNORDERED_FIRST;
+import static io.trino.spi.function.OperatorType.COMPARISON_UNORDERED_LAST;
 import static io.trino.spi.function.OperatorType.EQUAL;
 import static io.trino.spi.function.OperatorType.HASH_CODE;
 import static io.trino.spi.function.OperatorType.INDETERMINATE;
@@ -69,7 +70,7 @@
 public final class FunctionsParserHelper
 {
     private static final Set<OperatorType> COMPARABLE_TYPE_OPERATORS = ImmutableSet.of(EQUAL, HASH_CODE, XX_HASH_64, IS_DISTINCT_FROM, INDETERMINATE);
-    private static final Set<OperatorType> ORDERABLE_TYPE_OPERATORS = ImmutableSet.of(COMPARISON, LESS_THAN, LESS_THAN_OR_EQUAL);
+    private static final Set<OperatorType> ORDERABLE_TYPE_OPERATORS = ImmutableSet.of(COMPARISON_UNORDERED_LAST, COMPARISON_UNORDERED_FIRST, LESS_THAN, LESS_THAN_OR_EQUAL);
 
     private FunctionsParserHelper()
     {}

diff --git a/core/trino-main/src/main/java/io/trino/operator/scalar/AbstractGreatestLeast.java b/core/trino-main/src/main/java/io/trino/operator/scalar/AbstractGreatestLeast.java
@@ -61,12 +61,12 @@
 import static io.trino.spi.function.InvocationConvention.InvocationReturnConvention.FAIL_ON_NULL;
 import static io.trino.spi.function.InvocationConvention.InvocationReturnConvention.NULLABLE_RETURN;
 import static io.trino.spi.function.InvocationConvention.simpleConvention;
-import static io.trino.spi.function.OperatorType.COMPARISON;
 import static io.trino.sql.gen.Bootstrap.BOOTSTRAP_METHOD;
 import static io.trino.util.CompilerUtils.defineClass;
 import static io.trino.util.CompilerUtils.makeClassName;
 import static io.trino.util.Failures.checkCondition;
 import static io.trino.util.MinMaxCompare.getMinMaxCompare;
+import static io.trino.util.MinMaxCompare.getMinMaxCompareFunctionDependencies;
 import static io.trino.util.Reflection.methodHandle;
 import static java.lang.invoke.MethodType.methodType;
 import static java.util.Collections.nCopies;
@@ -99,9 +99,7 @@ protected AbstractGreatestLeast(boolean min, String description)
     @Override
     public FunctionDependencyDeclaration getFunctionDependencies()
     {
-        return FunctionDependencyDeclaration.builder()
-                .addOperatorSignature(COMPARISON, ImmutableList.of(new TypeSignature("E"), new TypeSignature("E")))
-                .build();
+        return getMinMaxCompareFunctionDependencies(new TypeSignature("E"), min);
     }
 
     @Override

diff --git a/core/trino-main/src/main/java/io/trino/operator/scalar/ArrayMaxFunction.java b/core/trino-main/src/main/java/io/trino/operator/scalar/ArrayMaxFunction.java
@@ -27,7 +27,7 @@
 
 import static io.trino.spi.function.InvocationConvention.InvocationArgumentConvention.BLOCK_POSITION;
 import static io.trino.spi.function.InvocationConvention.InvocationReturnConvention.FAIL_ON_NULL;
-import static io.trino.spi.function.OperatorType.COMPARISON;
+import static io.trino.spi.function.OperatorType.COMPARISON_UNORDERED_FIRST;
 import static io.trino.spi.type.DoubleType.DOUBLE;
 import static io.trino.spi.type.RealType.REAL;
 import static io.trino.util.Failures.internalError;
@@ -44,7 +44,7 @@ private ArrayMaxFunction() {}
     @SqlNullable
     public static Long longArrayMax(
             @OperatorDependency(
-                    operator = COMPARISON,
+                    operator = COMPARISON_UNORDERED_FIRST,
                     argumentTypes = {"T", "T"},
                     convention = @Convention(arguments = {BLOCK_POSITION, BLOCK_POSITION}, result = FAIL_ON_NULL)) MethodHandle compareMethodHandle,
             @TypeParameter("T") Type elementType,
@@ -62,7 +62,7 @@ public static Long longArrayMax(
     @SqlNullable
     public static Boolean booleanArrayMax(
             @OperatorDependency(
-                    operator = COMPARISON,
+                    operator = COMPARISON_UNORDERED_FIRST,
                     argumentTypes = {"T", "T"},
                     convention = @Convention(arguments = {BLOCK_POSITION, BLOCK_POSITION}, result = FAIL_ON_NULL)) MethodHandle compareMethodHandle,
             @TypeParameter("T") Type elementType,
@@ -80,7 +80,7 @@ public static Boolean booleanArrayMax(
     @SqlNullable
     public static Double doubleArrayMax(
             @OperatorDependency(
-                    operator = COMPARISON,
+                    operator = COMPARISON_UNORDERED_FIRST,
                     argumentTypes = {"T", "T"},
                     convention = @Convention(arguments = {BLOCK_POSITION, BLOCK_POSITION}, result = FAIL_ON_NULL)) MethodHandle compareMethodHandle,
             @TypeParameter("T") Type elementType,
@@ -98,7 +98,7 @@ public static Double doubleArrayMax(
     @SqlNullable
     public static Object objectArrayMax(
             @OperatorDependency(
-                    operator = COMPARISON,
+                    operator = COMPARISON_UNORDERED_FIRST,
                     argumentTypes = {"T", "T"},
                     convention = @Convention(arguments = {BLOCK_POSITION, BLOCK_POSITION}, result = FAIL_ON_NULL)) MethodHandle compareMethodHandle,
             @TypeParameter("T") Type elementType,

diff --git a/core/trino-main/src/main/java/io/trino/operator/scalar/ArrayMinFunction.java b/core/trino-main/src/main/java/io/trino/operator/scalar/ArrayMinFunction.java
@@ -27,7 +27,7 @@
 
 import static io.trino.spi.function.InvocationConvention.InvocationArgumentConvention.BLOCK_POSITION;
 import static io.trino.spi.function.InvocationConvention.InvocationReturnConvention.FAIL_ON_NULL;
-import static io.trino.spi.function.OperatorType.COMPARISON;
+import static io.trino.spi.function.OperatorType.COMPARISON_UNORDERED_LAST;
 import static io.trino.util.Failures.internalError;
 
 @ScalarFunction("array_min")
@@ -41,7 +41,7 @@ private ArrayMinFunction() {}
     @SqlNullable
     public static Long longArrayMin(
             @OperatorDependency(
-                    operator = COMPARISON,
+                    operator = COMPARISON_UNORDERED_LAST,
                     argumentTypes = {"T", "T"},
                     convention = @Convention(arguments = {BLOCK_POSITION, BLOCK_POSITION}, result = FAIL_ON_NULL)) MethodHandle compareMethodHandle,
             @TypeParameter("T") Type elementType,
@@ -59,7 +59,7 @@ public static Long longArrayMin(
     @SqlNullable
     public static Boolean booleanArrayMin(
             @OperatorDependency(
-                    operator = COMPARISON,
+                    operator = COMPARISON_UNORDERED_LAST,
                     argumentTypes = {"T", "T"},
                     convention = @Convention(arguments = {BLOCK_POSITION, BLOCK_POSITION}, result = FAIL_ON_NULL)) MethodHandle compareMethodHandle,
             @TypeParameter("T") Type elementType,
@@ -77,7 +77,7 @@ public static Boolean booleanArrayMin(
     @SqlNullable
     public static Double doubleArrayMin(
             @OperatorDependency(
-                    operator = COMPARISON,
+                    operator = COMPARISON_UNORDERED_LAST,
                     argumentTypes = {"T", "T"},
                     convention = @Convention(arguments = {BLOCK_POSITION, BLOCK_POSITION}, result = FAIL_ON_NULL)) MethodHandle compareMethodHandle,
             @TypeParameter("T") Type elementType,
@@ -95,7 +95,7 @@ public static Double doubleArrayMin(
     @SqlNullable
     public static Object objectArrayMin(
             @OperatorDependency(
-                    operator = COMPARISON,
+                    operator = COMPARISON_UNORDERED_LAST,
                     argumentTypes = {"T", "T"},
                     convention = @Convention(arguments = {BLOCK_POSITION, BLOCK_POSITION}, result = FAIL_ON_NULL)) MethodHandle compareMethodHandle,
             @TypeParameter("T") Type elementType,