Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ public final class SystemSessionProperties
public static final String OPTIMIZE_FULL_OUTER_JOIN_WITH_COALESCE = "optimize_full_outer_join_with_coalesce";
public static final String INDEX_LOADER_TIMEOUT = "index_loader_timeout";
public static final String OPTIMIZED_REPARTITIONING_ENABLED = "optimized_repartitioning";
public static final String SIMPLIFY_ARRAY_OPERATIONS = "simplify_array_operations";

private final List<PropertyMetadata<?>> sessionProperties;

Expand Down Expand Up @@ -664,6 +665,11 @@ public SystemSessionProperties(
OPTIMIZED_REPARTITIONING_ENABLED,
"Experimental: Use optimized repartitioning",
featuresConfig.isOptimizedRepartitioningEnabled(),
false),
booleanProperty(
SIMPLIFY_ARRAY_OPERATIONS,
"Simplify and optimize array operations.",
true,
false));
}

Expand Down Expand Up @@ -1134,4 +1140,8 @@ public static boolean isOptimizedRepartitioningEnabled(Session session)
{
return session.getSystemProperty(OPTIMIZED_REPARTITIONING_ENABLED, Boolean.class);
}
public static boolean isSimplifyArrayOperations(Session session)
{
return session.getSystemProperty(SIMPLIFY_ARRAY_OPERATIONS, Boolean.class);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
import com.facebook.presto.sql.planner.iterative.rule.RemoveUnreferencedScalarLateralNodes;
import com.facebook.presto.sql.planner.iterative.rule.ReorderJoins;
import com.facebook.presto.sql.planner.iterative.rule.RewriteSpatialPartitioningAggregation;
import com.facebook.presto.sql.planner.iterative.rule.SimplifyArrayOperations;
import com.facebook.presto.sql.planner.iterative.rule.SimplifyCountOverConstant;
import com.facebook.presto.sql.planner.iterative.rule.SimplifyExpressions;
import com.facebook.presto.sql.planner.iterative.rule.SimplifyRowExpressions;
Expand Down Expand Up @@ -258,6 +259,11 @@ public PlanOptimizers(
PlanOptimizer rowExpressionPredicatePushDown = new StatsRecordingPlanOptimizer(optimizerStats, new RowExpressionPredicatePushDown(metadata, sqlParser));

builder.add(
new IterativeOptimizer(
ruleStats,
statsCalculator,
estimatedExchangesCostCalculator,
new SimplifyArrayOperations().rules()),
// Clean up all the sugar in expressions, e.g. AtTimeZone, must be run before all the other optimizers
new IterativeOptimizer(
ruleStats,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.facebook.presto.sql.planner.iterative.rule;

import static com.facebook.presto.sql.planner.iterative.rule.SimplifyArrayOperationsRewriter.simplifyArrayOperations;

public class SimplifyArrayOperations
extends ExpressionRewriteRuleSet
{
public SimplifyArrayOperations()
{
super((expression, context) -> simplifyArrayOperations(expression, context.getSession()));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,163 @@
/*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.facebook.presto.sql.planner.iterative.rule;

import com.facebook.presto.Session;
import com.facebook.presto.SystemSessionProperties;
import com.facebook.presto.sql.tree.Cast;
import com.facebook.presto.sql.tree.ComparisonExpression;
import com.facebook.presto.sql.tree.ComparisonExpression.Operator;
import com.facebook.presto.sql.tree.Expression;
import com.facebook.presto.sql.tree.ExpressionRewriter;
import com.facebook.presto.sql.tree.ExpressionTreeRewriter;
import com.facebook.presto.sql.tree.FunctionCall;
import com.facebook.presto.sql.tree.LambdaExpression;
import com.facebook.presto.sql.tree.Literal;
import com.facebook.presto.sql.tree.LongLiteral;
import com.facebook.presto.sql.tree.NodeLocation;
import com.facebook.presto.sql.tree.QualifiedName;
import com.google.common.collect.ImmutableList;

import java.util.Optional;

import static com.facebook.presto.sql.tree.ComparisonExpression.Operator.EQUAL;

public class SimplifyArrayOperationsRewriter
{
private SimplifyArrayOperationsRewriter() {}

public static Expression simplifyArrayOperations(Expression expression, Session session)
{
if (SystemSessionProperties.isSimplifyArrayOperations(session)) {
return ExpressionTreeRewriter.rewriteWith(new Visitor(), expression);
}

return expression;
}

private static class Visitor
extends ExpressionRewriter<Void>
{
private static boolean isConstant(Expression expression)
{
if (expression instanceof Cast) {
return isConstant(((Cast) expression).getExpression());
}

return expression instanceof Literal;
}

private static boolean isZero(Expression expression)

@wenleix wenleix Dec 27, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

cc @highker, @hellium01 : Is this the recommended way to see whether a expression will be evaluated into certain constant?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not really; we need to use interpreter and evaluate the expression. The return result of the eval should be a const with 0 as its value.

{
if (expression instanceof Cast) {
return isZero(((Cast) expression).getExpression());
}

return expression instanceof LongLiteral && ((LongLiteral) expression).getValue() == 0;
}

private static boolean isCardinalityOfFilter(Expression expression)
{
if (expression instanceof FunctionCall) {
FunctionCall functionCall = (FunctionCall) expression;
return functionCall.getName().toString().equals("cardinality") &&

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We used to use resolveFunction to get the function handle. But given now it requires transactions, maybe @rongrong can be a better PoC to comment on.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

resolveFunction now only takes an optional transaction ID. Since this is a static function you can just pass in Optional.empty().

functionCall.getArguments().size() == 1 &&
functionCall.getArguments().get(0) instanceof FunctionCall &&
((FunctionCall) (functionCall.getArguments().get(0))).getName().toString().equals("filter");
}

return false;
}

private static boolean isSimpleComaparison(ComparisonExpression comparisonExpression)
{
switch (comparisonExpression.getOperator()) {
case EQUAL:
case GREATER_THAN:
case LESS_THAN:
case NOT_EQUAL:
case GREATER_THAN_OR_EQUAL:
case LESS_THAN_OR_EQUAL:
return true;
}

return false;
}

private Expression simplifyCardinalityOfFilterComparedToZero(
NodeLocation location,
Expression array,
Operator operator,
LambdaExpression origLambda)
{
// Comparing to zero can be rewrtitten to none_match(= 0) or any_match(> 0)
return new FunctionCall(
location,
QualifiedName.of(operator == EQUAL ? "none_match" : "any_match"),
ImmutableList.of(array, origLambda));
}

private Expression simplifyCardinalityOfFilter(
FunctionCall cardinalityOfFilter,
Optional<Operator> operator,
Optional<Expression> rhs,
Void context,
ExpressionTreeRewriter<Void> treeRewriter)
{
FunctionCall filter = (FunctionCall) cardinalityOfFilter.getArguments().get(0);
Expression array = treeRewriter.defaultRewrite(filter.getArguments().get(0), context);
LambdaExpression origLambda = (LambdaExpression) filter.getArguments().get(1);
boolean isComparison = operator.isPresent() && rhs.isPresent();

if (isComparison && (operator.get() == Operator.EQUAL || operator.get() == Operator.GREATER_THAN) && isZero(rhs.get())) {
return simplifyCardinalityOfFilterComparedToZero(cardinalityOfFilter.getLocation().orElse(new NodeLocation(0, 0)), array, operator.get(), origLambda);
}

return null;
}

@Override
public Expression rewriteFunctionCall(FunctionCall functionCall, Void context, ExpressionTreeRewriter<Void> treeRewriter)
{
if (isCardinalityOfFilter(functionCall)) {
return simplifyCardinalityOfFilter(functionCall, Optional.empty(), Optional.empty(), context, treeRewriter);
}

return treeRewriter.defaultRewrite(functionCall, context);
}

@Override
public Expression rewriteComparisonExpression(ComparisonExpression comparisonExpression, Void context, ExpressionTreeRewriter<Void> treeRewriter)
{
Expression left = comparisonExpression.getLeft();
Expression right = comparisonExpression.getRight();
Operator operator = comparisonExpression.getOperator();

Expression rewritten = null;
if (isSimpleComaparison(comparisonExpression) && isCardinalityOfFilter(left) && isConstant(right)) {
rewritten = simplifyCardinalityOfFilter((FunctionCall) left, Optional.of(operator), Optional.of(right), context, treeRewriter);
}
else if (isSimpleComaparison(comparisonExpression) && isCardinalityOfFilter(right) && isConstant(left)) {
// If the left is a literal and right is cardinality, we simply normalize it to reverse the operation.
rewritten = simplifyCardinalityOfFilter((FunctionCall) left, Optional.of(operator.negate()), Optional.of(right), context, treeRewriter);
}

if (rewritten != null) {
return rewritten;
}

return treeRewriter.defaultRewrite(comparisonExpression, context);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1892,6 +1892,15 @@ private void assertArrayHashOperator(String inputArray, Type elementType, List<O
assertOperator(HASH_CODE, inputArray, BIGINT, arrayType.hash(arrayArrayBuilder.build(), 0));
}

@Test
public void CardinalityOfFilterSimplification()
{
assertFunction("cardinality(filter(ARRAY[3,4,6,9,10], x -> x % 2 = 0))", BIGINT, 3L);
assertFunction("cardinality(filter(ARRAY[3,4,6,9,10], x -> x % 3 = 0)) = 3", BOOLEAN, true);
assertFunction("cardinality(filter(ARRAY[3,4,6,9,10], x -> x % 2 = 0)) > 0", BOOLEAN, true);
assertFunction("cardinality(filter(ARRAY[3,4,6,9,10], x -> x > 2)) = 0", BOOLEAN, false);
}

private static SqlDate sqlDate(String dateString)
throws ParseException
{
Expand Down