Skip to content

Commit

Permalink
Add block/position convention to all distinct from operators
Browse files Browse the repository at this point in the history 
This commit adds a second implementation to distinct from operator of all
types. This 2nd implementation uses block/position convention. As a
result, for certain types, it's no longer necessary to extract the value
out of the Block in order to invoke "distinct from" operator. This is
expected to resolve performance issue previously introduced in
baad26e (which was work around with a
hack).
  • Loading branch information
@gerlou @haozhun
gerlou authored and haozhun committed Dec 17, 2018
1 parent 786ccbd commit b845cd6
Show file tree
Hide file tree
Showing 27 changed files with 867 additions and 287 deletions.
21 changes: 21 additions & 0 deletions presto-main/src/main/java/com/facebook/presto/metadata/FunctionRegistry.java
View file
Expand Up @@ -490,29 +490,48 @@ public FunctionRegistry(TypeManager typeManager, BlockEncodingSerde blockEncodin
.scalars(JsonFunctions.class)
.scalars(ColorFunctions.class)
.scalars(ColorOperators.class)
.scalar(ColorOperators.ColorDistinctFromOperator.class)
.scalars(HyperLogLogFunctions.class)
.scalars(QuantileDigestFunctions.class)
.scalars(UnknownOperators.class)
.scalar(UnknownOperators.UnknownDistinctFromOperator.class)
.scalars(BooleanOperators.class)
.scalar(BooleanOperators.BooleanDistinctFromOperator.class)
.scalars(BigintOperators.class)
.scalar(BigintOperators.BigintDistinctFromOperator.class)
.scalars(IntegerOperators.class)
.scalar(IntegerOperators.IntegerDistinctFromOperator.class)
.scalars(SmallintOperators.class)
.scalar(SmallintOperators.SmallintDistinctFromOperator.class)
.scalars(TinyintOperators.class)
.scalar(TinyintOperators.TinyintDistinctFromOperator.class)
.scalars(DoubleOperators.class)
.scalar(DoubleOperators.DoubleDistinctFromOperator.class)
.scalars(RealOperators.class)
.scalar(RealOperators.RealDistinctFromOperator.class)
.scalars(VarcharOperators.class)
.scalar(VarcharOperators.VarcharDistinctFromOperator.class)
.scalars(VarbinaryOperators.class)
.scalar(VarbinaryOperators.VarbinaryDistinctFromOperator.class)
.scalars(DateOperators.class)
.scalar(DateOperators.DateDistinctFromOperator.class)
.scalars(TimeOperators.class)
.scalar(TimeOperators.TimeDistinctFromOperator.class)
.scalars(TimestampOperators.class)
.scalar(TimestampOperators.TimestampDistinctFromOperator.class)
.scalars(IntervalDayTimeOperators.class)
.scalar(IntervalDayTimeOperators.IntervalDayTimeDistinctFromOperator.class)
.scalars(IntervalYearMonthOperators.class)
.scalar(IntervalYearMonthOperators.IntervalYearMonthDistinctFromOperator.class)
.scalars(TimeWithTimeZoneOperators.class)
.scalar(TimeWithTimeZoneOperators.TimeWithTimeZoneDistinctFromOperator.class)
.scalars(TimestampWithTimeZoneOperators.class)
.scalar(TimestampWithTimeZoneOperators.TimestampWithTimeZoneDistinctFromOperator.class)
.scalars(DateTimeOperators.class)
.scalars(HyperLogLogOperators.class)
.scalars(QuantileDigestOperators.class)
.scalars(IpAddressOperators.class)
.scalar(IpAddressOperators.IpAddressDistinctFromOperator.class)
.scalars(LikeFunctions.class)
.scalars(ArrayFunctions.class)
.scalars(HmacFunctions.class)
Expand All @@ -523,10 +542,12 @@ public FunctionRegistry(TypeManager typeManager, BlockEncodingSerde blockEncodin
.scalar(ArrayPositionFunction.class)
.scalars(CombineHashFunction.class)
.scalars(JsonOperators.class)
.scalar(JsonOperators.JsonDistinctFromOperator.class)
.scalars(FailureFunction.class)
.scalars(JoniRegexpCasts.class)
.scalars(CharacterStringCasts.class)
.scalars(CharOperators.class)
.scalar(CharOperators.CharDistinctFromOperator.class)
.scalar(DecimalOperators.Negation.class)
.scalar(DecimalOperators.HashCode.class)
.scalar(DecimalOperators.Indeterminate.class)
Expand Down
53 changes: 33 additions & 20 deletions presto-main/src/main/java/com/facebook/presto/operator/scalar/ArrayDistinctFromOperator.java
View file
Expand Up @@ -14,6 +14,8 @@
*/

import com.facebook.presto.spi.block.Block;
import com.facebook.presto.spi.function.BlockIndex;
import com.facebook.presto.spi.function.BlockPosition;
import com.facebook.presto.spi.function.Convention;
import com.facebook.presto.spi.function.IsNull;
import com.facebook.presto.spi.function.OperatorDependency;
Expand All @@ -25,12 +27,10 @@

import java.lang.invoke.MethodHandle;

import static com.facebook.presto.spi.function.InvocationConvention.InvocationArgumentConvention.NULL_FLAG;
import static com.facebook.presto.spi.function.InvocationConvention.InvocationArgumentConvention.BLOCK_POSITION;
import static com.facebook.presto.spi.function.InvocationConvention.InvocationReturnConvention.FAIL_ON_NULL;
import static com.facebook.presto.spi.function.OperatorType.IS_DISTINCT_FROM;
import static com.facebook.presto.spi.type.TypeUtils.readNativeValue;
import static com.facebook.presto.util.Failures.internalError;
import static com.google.common.base.Defaults.defaultValue;

@ScalarOperator(IS_DISTINCT_FROM)
public final class ArrayDistinctFromOperator
Expand All @@ -44,8 +44,8 @@ public static boolean isDistinctFrom(
operator = IS_DISTINCT_FROM,
returnType = StandardTypes.BOOLEAN,
argumentTypes = {"E", "E"},
convention = @Convention(arguments = {NULL_FLAG, NULL_FLAG}, result = FAIL_ON_NULL)) MethodHandle function,
@TypeParameter("E") Type type,
convention = @Convention(arguments = {BLOCK_POSITION, BLOCK_POSITION}, result = FAIL_ON_NULL)) MethodHandle function,
@TypeParameter("array(E)") Type type,
@SqlType("array(E)") Block left,
@IsNull boolean leftNull,
@SqlType("array(E)") Block right,
Expand All @@ -61,22 +61,12 @@ public static boolean isDistinctFrom(
return true;
}
for (int i = 0; i < left.getPositionCount(); i++) {
Object leftValue = readNativeValue(type, left, i);
boolean leftValueNull = leftValue == null;
if (leftValueNull) {
leftValue = defaultValue(type.getJavaType());
}
Object rightValue = readNativeValue(type, right, i);
boolean rightValueNull = rightValue == null;
if (rightValueNull) {
rightValue = defaultValue(type.getJavaType());
}
try {
if ((boolean) function.invoke(
leftValue,
leftValueNull,
rightValue,
rightValueNull)) {
if ((boolean) function.invokeExact(
left,
i,
right,
i)) {
return true;
}
}
Expand All @@ -86,4 +76,27 @@ public static boolean isDistinctFrom(
}
return false;
}

@TypeParameter("E")
@SqlType(StandardTypes.BOOLEAN)
public static boolean isDistinctFrom(
@OperatorDependency(
operator = IS_DISTINCT_FROM,
returnType = StandardTypes.BOOLEAN,
argumentTypes = {"E", "E"},
convention = @Convention(arguments = {BLOCK_POSITION, BLOCK_POSITION}, result = FAIL_ON_NULL)) MethodHandle elementIsDistinctFrom,
@TypeParameter("array(E)") Type type,
@BlockPosition @SqlType(value = "array(E)", nativeContainerType = Block.class) Block left,
@BlockIndex int leftPosition,
@BlockPosition @SqlType(value = "array(E)", nativeContainerType = Block.class) Block right,
@BlockIndex int rightPosition)
{
return isDistinctFrom(
elementIsDistinctFrom,
type,
(Block) type.getObject(left, leftPosition),
left.isNull(leftPosition),
(Block) type.getObject(right, rightPosition),
right.isNull(rightPosition));
}
}
41 changes: 34 additions & 7 deletions presto-main/src/main/java/com/facebook/presto/operator/scalar/JsonOperators.java
View file
Expand Up @@ -15,6 +15,9 @@

import com.facebook.presto.spi.ConnectorSession;
import com.facebook.presto.spi.PrestoException;
import com.facebook.presto.spi.block.Block;
import com.facebook.presto.spi.function.BlockIndex;
import com.facebook.presto.spi.function.BlockPosition;
import com.facebook.presto.spi.function.IsNull;
import com.facebook.presto.spi.function.LiteralParameters;
import com.facebook.presto.spi.function.ScalarOperator;
Expand Down Expand Up @@ -391,15 +394,39 @@ public static Boolean notEqual(@SqlType(JSON) Slice leftJson, @SqlType(JSON) Sli
}

@ScalarOperator(IS_DISTINCT_FROM)
@SqlType(BOOLEAN)
public static boolean isDistinctFrom(@SqlType(JSON) Slice leftJson, @IsNull boolean leftNull, @SqlType(JSON) Slice rightJson, @IsNull boolean rightNull)
public static class JsonDistinctFromOperator
{
if (leftNull != rightNull) {
return true;
@SqlType(BOOLEAN)
public static boolean isDistinctFrom(@SqlType(JSON) Slice leftJson, @IsNull boolean leftNull, @SqlType(JSON) Slice rightJson, @IsNull boolean rightNull)
{
if (leftNull != rightNull) {
return true;
}
if (leftNull) {
return false;
}
return notEqual(leftJson, rightJson);
}
if (leftNull) {
return false;

@SqlType(BOOLEAN)
public static boolean isDistinctFrom(
@BlockPosition @SqlType(value = JSON, nativeContainerType = Slice.class) Block left,
@BlockIndex int leftPosition,
@BlockPosition @SqlType(value = JSON, nativeContainerType = Slice.class) Block right,
@BlockIndex int rightPosition)
{
if (left.isNull(leftPosition) != right.isNull(rightPosition)) {
return true;
}
if (left.isNull(leftPosition)) {
return false;
}
int leftLength = left.getSliceLength(leftPosition);
int rightLength = right.getSliceLength(rightPosition);
if (leftLength != rightLength) {
return true;
}
return !left.equals(leftPosition, 0, right, rightPosition, 0, leftLength);
}
return notEqual(leftJson, rightJson);
}
}
52 changes: 31 additions & 21 deletions presto-main/src/main/java/com/facebook/presto/operator/scalar/MapDistinctFromOperator.java
View file
Expand Up @@ -14,20 +14,23 @@
*/

import com.facebook.presto.spi.block.Block;
import com.facebook.presto.spi.function.BlockIndex;
import com.facebook.presto.spi.function.BlockPosition;
import com.facebook.presto.spi.function.Convention;
import com.facebook.presto.spi.function.IsNull;
import com.facebook.presto.spi.function.OperatorDependency;
import com.facebook.presto.spi.function.ScalarOperator;
import com.facebook.presto.spi.function.SqlType;
import com.facebook.presto.spi.function.TypeParameter;
import com.facebook.presto.spi.type.MapType;
import com.facebook.presto.spi.type.StandardTypes;
import com.facebook.presto.spi.type.Type;

import java.lang.invoke.MethodHandle;

import static com.facebook.presto.spi.function.OperatorType.EQUAL;
import static com.facebook.presto.spi.function.OperatorType.HASH_CODE;
import static com.facebook.presto.spi.function.InvocationConvention.InvocationArgumentConvention.BLOCK_POSITION;
import static com.facebook.presto.spi.function.InvocationConvention.InvocationReturnConvention.FAIL_ON_NULL;
import static com.facebook.presto.spi.function.OperatorType.IS_DISTINCT_FROM;
import static com.facebook.presto.spi.type.TypeUtils.readNativeValue;

@ScalarOperator(IS_DISTINCT_FROM)
public final class MapDistinctFromOperator
Expand All @@ -38,14 +41,9 @@ private MapDistinctFromOperator() {}
@TypeParameter("V")
@SqlType(StandardTypes.BOOLEAN)
public static boolean isDistinctFrom(
@OperatorDependency(operator = EQUAL, returnType = StandardTypes.BOOLEAN, argumentTypes = {"K", "K"})
MethodHandle keyEqualsFunction,
@OperatorDependency(operator = HASH_CODE, returnType = StandardTypes.BIGINT, argumentTypes = {"K"})
MethodHandle keyHashcodeFunction,
@OperatorDependency(operator = IS_DISTINCT_FROM, returnType = StandardTypes.BOOLEAN, argumentTypes = {"V", "V"})
@OperatorDependency(operator = IS_DISTINCT_FROM, returnType = StandardTypes.BOOLEAN, argumentTypes = {"V", "V"}, convention = @Convention(arguments = {BLOCK_POSITION, BLOCK_POSITION}, result = FAIL_ON_NULL))
MethodHandle valueDistinctFromFunction,
@TypeParameter("K") Type keyType,
@TypeParameter("V") Type valueType,
@TypeParameter("map(K, V)") Type mapType,
@SqlType("map(K,V)") Block leftMapBlock,
@IsNull boolean leftMapNull,
@SqlType("map(K,V)") Block rightMapBlock,
Expand All @@ -59,18 +57,30 @@ public static boolean isDistinctFrom(
}
// Note that we compare to NOT distinct here and so negate the result.
return !MapGenericEquality.genericEqual(
keyType,
((MapType) mapType).getKeyType(),
leftMapBlock,
rightMapBlock,
(leftMapIndex, rightMapIndex) -> {
Object leftValue = readNativeValue(valueType, leftMapBlock, leftMapIndex);
Object rightValue = readNativeValue(valueType, rightMapBlock, rightMapIndex);
boolean leftNull = leftValue == null;
boolean rightNull = rightValue == null;
if (leftNull || rightNull) {
return leftNull == rightNull;
}
return !(boolean) valueDistinctFromFunction.invoke(leftValue, leftNull, rightValue, rightNull);
});
(leftMapIndex, rightMapIndex) -> !(boolean) valueDistinctFromFunction.invokeExact(leftMapBlock, leftMapIndex, rightMapBlock, rightMapIndex));
}

@TypeParameter("K")
@TypeParameter("V")
@SqlType(StandardTypes.BOOLEAN)
public static boolean isDistinctFrom(
@OperatorDependency(operator = IS_DISTINCT_FROM, returnType = StandardTypes.BOOLEAN, argumentTypes = {"V", "V"}, convention = @Convention(arguments = {BLOCK_POSITION, BLOCK_POSITION}, result = FAIL_ON_NULL))
MethodHandle valueDistinctFromFunction,
@TypeParameter("map(K, V)") Type mapType,
@BlockPosition @SqlType(value = "map(K,V)", nativeContainerType = Block.class) Block left,
@BlockIndex int leftPosition,
@BlockPosition @SqlType(value = "map(K,V)", nativeContainerType = Block.class) Block right,
@BlockIndex int rightPosition)
{
return isDistinctFrom(
valueDistinctFromFunction,
mapType,
(Block) mapType.getObject(left, leftPosition),
left.isNull(leftPosition),
(Block) mapType.getObject(right, rightPosition),
right.isNull(rightPosition));
}
}
30 changes: 25 additions & 5 deletions presto-main/src/main/java/com/facebook/presto/operator/scalar/RowDistinctFromOperator.java
View file
Expand Up @@ -18,6 +18,7 @@
import com.facebook.presto.metadata.FunctionRegistry;
import com.facebook.presto.metadata.Signature;
import com.facebook.presto.metadata.SqlOperator;
import com.facebook.presto.operator.scalar.ScalarFunctionImplementation.ScalarImplementationChoice;
import com.facebook.presto.spi.block.Block;
import com.facebook.presto.spi.function.InvocationConvention;
import com.facebook.presto.spi.type.StandardTypes;
Expand All @@ -31,6 +32,7 @@

import static com.facebook.presto.metadata.Signature.comparableWithVariadicBound;
import static com.facebook.presto.operator.scalar.ScalarFunctionImplementation.ArgumentProperty.valueTypeArgumentProperty;
import static com.facebook.presto.operator.scalar.ScalarFunctionImplementation.NullConvention.BLOCK_AND_POSITION;
import static com.facebook.presto.operator.scalar.ScalarFunctionImplementation.NullConvention.USE_NULL_FLAG;
import static com.facebook.presto.spi.function.InvocationConvention.InvocationArgumentConvention.NULL_FLAG;
import static com.facebook.presto.spi.function.OperatorType.IS_DISTINCT_FROM;
Expand All @@ -44,7 +46,8 @@ public class RowDistinctFromOperator
extends SqlOperator
{
public static final RowDistinctFromOperator ROW_DISTINCT_FROM = new RowDistinctFromOperator();
private static final MethodHandle METHOD_HANDLE = methodHandle(RowDistinctFromOperator.class, "isDistinctFrom", Type.class, List.class, Block.class, boolean.class, Block.class, boolean.class);
private static final MethodHandle METHOD_HANDLE_NULL_FLAG = methodHandle(RowDistinctFromOperator.class, "isDistinctFrom", Type.class, List.class, Block.class, boolean.class, Block.class, boolean.class);
private static final MethodHandle METHOD_HANDLE_BLOCK_POSITION = methodHandle(RowDistinctFromOperator.class, "isDistinctFrom", Type.class, List.class, Block.class, int.class, Block.class, int.class);

private RowDistinctFromOperator()
{
Expand All @@ -71,11 +74,17 @@ public ScalarFunctionImplementation specialize(BoundVariables boundVariables, in
argumentMethods.add(functionInvoker.methodHandle());
}
return new ScalarFunctionImplementation(
false,
ImmutableList.of(
valueTypeArgumentProperty(USE_NULL_FLAG),
valueTypeArgumentProperty(USE_NULL_FLAG)),
METHOD_HANDLE.bindTo(type).bindTo(argumentMethods.build()),
new ScalarImplementationChoice(
false,
ImmutableList.of(valueTypeArgumentProperty(USE_NULL_FLAG), valueTypeArgumentProperty(USE_NULL_FLAG)),
METHOD_HANDLE_NULL_FLAG.bindTo(type).bindTo(argumentMethods.build()),
Optional.empty()),
new ScalarImplementationChoice(
false,
ImmutableList.of(valueTypeArgumentProperty(BLOCK_AND_POSITION), valueTypeArgumentProperty(BLOCK_AND_POSITION)),
METHOD_HANDLE_BLOCK_POSITION.bindTo(type).bindTo(argumentMethods.build()),
Optional.empty())),
isDeterministic());
}

Expand Down Expand Up @@ -115,4 +124,15 @@ public static boolean isDistinctFrom(Type rowType, List<MethodHandle> argumentMe
}
return false;
}

public static boolean isDistinctFrom(Type rowType, List<MethodHandle> argumentMethods, Block leftRow, int leftPosition, Block rightRow, int rightPosition)
{
return isDistinctFrom(
rowType,
argumentMethods,
(Block) rowType.getObject(leftRow, leftPosition),
leftRow.isNull(leftPosition),
(Block) rowType.getObject(rightRow, rightPosition),
rightRow.isNull(rightPosition));
}
}

0 comments on commit b845cd6

Please sign in to comment.