Skip to content

feat(optimizer): Unwrap casts in comparison expressions #26326

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

feat(optimizer): Unwrap casts in comparison expressions #26326

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions presto-common/src/main/java/com/facebook/presto/common/type/AbstractVarcharType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@
import com.facebook.presto.common.block.BlockBuilder;
import com.facebook.presto.common.function.SqlFunctionProperties;
import io.airlift.slice.Slice;
import io.airlift.slice.SliceUtf8;
import io.airlift.slice.Slices;

import java.util.Objects;
import java.util.Optional;

import static java.lang.Character.MAX_CODE_POINT;

public class AbstractVarcharType
extends AbstractVariableWidthType
Expand Down Expand Up @@ -106,6 +110,27 @@ public int compareTo(Block leftBlock, int leftPosition, Block rightBlock, int ri
return leftBlock.compareTo(leftPosition, 0, leftLength, rightBlock, rightPosition, 0, rightLength);
}

@Override
public Optional<Range> getRange()
{
if (length > 100) {
// The max/min values may be materialized in the plan, so we don't want them to be too large.
// Range comparison against large values are usually nonsensical, too, so no need to support them
// beyond a certain size. They specific choice above is arbitrary and can be adjusted if needed.
return Optional.empty();
}

int codePointSize = SliceUtf8.lengthOfCodePoint(MAX_CODE_POINT);

Slice max = Slices.allocate(codePointSize * length);
int position = 0;
for (int i = 0; i < length; i++) {
position += SliceUtf8.setCodePointAt(MAX_CODE_POINT, max, position);
}

return Optional.of(new Range(Slices.EMPTY_SLICE, max));
}

@Override
public void appendTo(Block block, int position, BlockBuilder blockBuilder)
{
Expand Down
8 changes: 8 additions & 0 deletions presto-common/src/main/java/com/facebook/presto/common/type/BigintType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
import com.facebook.presto.common.block.Block;
import com.facebook.presto.common.function.SqlFunctionProperties;

import java.util.Optional;

import static com.facebook.presto.common.type.TypeSignature.parseTypeSignature;

public final class BigintType
Expand Down Expand Up @@ -50,4 +52,10 @@ public int hashCode()
{
return getClass().hashCode();
}

@Override
public Optional<Range> getRange()
{
return Optional.of(new Range(Long.MIN_VALUE, Long.MAX_VALUE));
}
}
10 changes: 10 additions & 0 deletions presto-common/src/main/java/com/facebook/presto/common/type/DoubleType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@
import com.facebook.presto.common.block.UncheckedBlock;
import com.facebook.presto.common.function.SqlFunctionProperties;

import java.util.Optional;

import static com.facebook.presto.common.type.TypeSignature.parseTypeSignature;
import static com.facebook.presto.common.type.TypeUtils.doubleCompare;
import static com.facebook.presto.common.type.TypeUtils.doubleEquals;
Expand Down Expand Up @@ -170,4 +172,12 @@ public int hashCode()
{
return getClass().hashCode();
}

@Override
public Optional<Range> getRange()
{
// The range for double is undefined because NaN is a special value that
// is *not* in any reasonable definition of a range for this type.
return Optional.empty();
}
Comment on lines +177 to +182
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually wonder if this is true. NaN should be treated as the highest value by convention.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tdcmeehan Thanks for the review.
You're right that Nan is treated as the largest value in Presto comparisons and ordering. However, returning NaN as the maximum value of a range would not make sense here. In this specific rule, we unwrap a CAST when it's compared to a constant, and the comparison can be further optimized if the constant exceeds the maximum value of the source type. Since NaN is the maximum value when the source type is real or double, no constant can be greater than it, so empty is directly returned in getRange to skip this check.

}
8 changes: 8 additions & 0 deletions presto-common/src/main/java/com/facebook/presto/common/type/IntegerType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import com.facebook.presto.common.block.BlockBuilder;
import com.facebook.presto.common.function.SqlFunctionProperties;

import java.util.Optional;

import static com.facebook.presto.common.type.TypeSignature.parseTypeSignature;
import static java.lang.String.format;

Expand Down Expand Up @@ -54,6 +56,12 @@ else if (value < Integer.MIN_VALUE) {
blockBuilder.writeInt((int) value).closeEntry();
}

@Override
public Optional<Range> getRange()
{
return Optional.of(new Range((long) Integer.MIN_VALUE, (long) Integer.MAX_VALUE));
}

@Override
@SuppressWarnings("EqualsWhichDoesntCheckParameterClass")
public boolean equals(Object other)
Expand Down
10 changes: 10 additions & 0 deletions presto-common/src/main/java/com/facebook/presto/common/type/RealType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import com.facebook.presto.common.block.BlockBuilder;
import com.facebook.presto.common.function.SqlFunctionProperties;

import java.util.Optional;

import static com.facebook.presto.common.type.TypeSignature.parseTypeSignature;
import static com.facebook.presto.common.type.TypeUtils.realCompare;
import static com.facebook.presto.common.type.TypeUtils.realEquals;
Expand Down Expand Up @@ -106,4 +108,12 @@ public int hashCode()
{
return getClass().hashCode();
}

@Override
public Optional<Range> getRange()
{
// The range for real is undefined because NaN is a special value that
// is *not* in any reasonable definition of a range for this type.
return Optional.empty();
}
}
8 changes: 8 additions & 0 deletions presto-common/src/main/java/com/facebook/presto/common/type/SmallintType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import com.facebook.presto.common.block.UncheckedBlock;
import com.facebook.presto.common.function.SqlFunctionProperties;

import java.util.Optional;

import static com.facebook.presto.common.type.TypeSignature.parseTypeSignature;
import static java.lang.Long.rotateLeft;
import static java.lang.String.format;
Expand Down Expand Up @@ -116,6 +118,12 @@ public int compareTo(Block leftBlock, int leftPosition, Block rightBlock, int ri
return Short.compare(leftValue, rightValue);
}

@Override
public Optional<Range> getRange()
{
return Optional.of(new Range((long) Short.MIN_VALUE, (long) Short.MAX_VALUE));
}

@Override
public void appendTo(Block block, int position, BlockBuilder blockBuilder)
{
Expand Down
8 changes: 8 additions & 0 deletions presto-common/src/main/java/com/facebook/presto/common/type/TinyintType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import com.facebook.presto.common.block.UncheckedBlock;
import com.facebook.presto.common.function.SqlFunctionProperties;

import java.util.Optional;

import static com.facebook.presto.common.type.TypeSignature.parseTypeSignature;
import static java.lang.Long.rotateLeft;
import static java.lang.String.format;
Expand Down Expand Up @@ -116,6 +118,12 @@ public int compareTo(Block leftBlock, int leftPosition, Block rightBlock, int ri
return Byte.compare(leftValue, rightValue);
}

@Override
public Optional<Range> getRange()
{
return Optional.of(new Range((long) Byte.MIN_VALUE, (long) Byte.MAX_VALUE));
}

@Override
public void appendTo(Block block, int position, BlockBuilder blockBuilder)
{
Expand Down
35 changes: 35 additions & 0 deletions presto-common/src/main/java/com/facebook/presto/common/type/Type.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
import io.airlift.slice.Slice;

import java.util.List;
import java.util.Optional;

import static java.util.Objects.requireNonNull;

public interface Type
{
Expand Down Expand Up @@ -194,4 +197,36 @@ default boolean equalValuesAreIdentical()
* Compare the values in the specified block at the specified positions equal.
*/
int compareTo(Block leftBlock, int leftPosition, Block rightBlock, int rightPosition);

/**
* Return the range of possible values for this type, if available.
*
* The type of the values must match {@link #getJavaType}
*/
default Optional<Range> getRange()
{
return Optional.empty();
}

final class Range
{
private final Object min;
private final Object max;

public Range(Object min, Object max)
{
this.min = requireNonNull(min, "min is null");
this.max = requireNonNull(max, "max is null");
}

public Object getMin()
{
return min;
}

public Object getMax()
{
return max;
}
}
}
10 changes: 10 additions & 0 deletions presto-main-base/src/main/java/com/facebook/presto/SystemSessionProperties.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,7 @@ public final class SystemSessionProperties
public static final String UTILIZE_UNIQUE_PROPERTY_IN_QUERY_PLANNING = "utilize_unique_property_in_query_planning";
public static final String PUSHDOWN_SUBFIELDS_FOR_MAP_FUNCTIONS = "pushdown_subfields_for_map_functions";
public static final String MAX_SERIALIZABLE_OBJECT_SIZE = "max_serializable_object_size";
public static final String UNWRAP_CASTS = "unwrap_casts";

// TODO: Native execution related session properties that are temporarily put here. They will be relocated in the future.
public static final String NATIVE_AGGREGATION_SPILL_ALL = "native_aggregation_spill_all";
Expand Down Expand Up @@ -1993,6 +1994,10 @@ public SystemSessionProperties(
booleanProperty(ADD_DISTINCT_BELOW_SEMI_JOIN_BUILD,
"Add distinct aggregation below semi join build",
featuresConfig.isAddDistinctBelowSemiJoinBuild(),
false),
booleanProperty(UNWRAP_CASTS,
"Enable optimization to unwrap CAST expression",
featuresConfig.isUnwrapCasts(),
false));
}

Expand Down Expand Up @@ -3394,4 +3399,9 @@ public static long getMaxSerializableObjectSize(Session session)
{
return session.getSystemProperty(MAX_SERIALIZABLE_OBJECT_SIZE, Long.class);
}

public static boolean isUnwrapCasts(Session session)
{
return session.getSystemProperty(UNWRAP_CASTS, Boolean.class);
}
}
14 changes: 14 additions & 0 deletions presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/FeaturesConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,8 @@ public class FeaturesConfig

private boolean builtInSidecarFunctionsEnabled;

private boolean unwrapCasts = true;

public enum PartitioningPrecisionStrategy
{
// Let Presto decide when to repartition
Expand Down Expand Up @@ -3194,4 +3196,16 @@ public boolean isBuiltInSidecarFunctionsEnabled()
{
return this.builtInSidecarFunctionsEnabled;
}

public boolean isUnwrapCasts()
{
return unwrapCasts;
}

@Config("optimizer.unwrap-casts")
public FeaturesConfig setUnwrapCasts(boolean unwrapCasts)
{
this.unwrapCasts = unwrapCasts;
return this;
}
}
2 changes: 2 additions & 0 deletions presto-main-base/src/main/java/com/facebook/presto/sql/planner/PlanOptimizers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@
import com.facebook.presto.sql.planner.iterative.rule.TransformUncorrelatedInPredicateSubqueryToDistinctInnerJoin;
import com.facebook.presto.sql.planner.iterative.rule.TransformUncorrelatedInPredicateSubqueryToSemiJoin;
import com.facebook.presto.sql.planner.iterative.rule.TransformUncorrelatedLateralToJoin;
import com.facebook.presto.sql.planner.iterative.rule.UnwrapCastInComparison;
import com.facebook.presto.sql.planner.optimizations.AddExchanges;
import com.facebook.presto.sql.planner.optimizations.AddExchangesForSingleNodeExecution;
import com.facebook.presto.sql.planner.optimizations.AddLocalExchanges;
Expand Down Expand Up @@ -340,6 +341,7 @@ public PlanOptimizers(
estimatedExchangesCostCalculator,
ImmutableSet.<Rule<?>>builder()
.addAll(new SimplifyRowExpressions(metadata, expressionOptimizerManager).rules())
.addAll(new UnwrapCastInComparison(metadata, expressionOptimizerManager).rules())
.add(new PruneRedundantProjectionAssignments())
.build());

Expand Down
Loading
Loading