Skip to content

Commit

Permalink
Revert "Add source location info to RowExpression"
Browse files Browse the repository at this point in the history
This reverts commit 0406fbe.
  • Loading branch information
rongrong authored and Rongrong Zhong committed Nov 3, 2021
1 parent b6b6234 commit 547a068
Show file tree
Hide file tree
Showing 120 changed files with 345 additions and 835 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public static List<DruidAggregationColumnNode> computeAggregationNodes(Aggregati
// This block handles the case when a distinct aggregation is present in addition to another aggregation function.
// E.g. `SELECT count(distinct COL_A), sum(COL_B) FROM myTable` to Druid as `SELECT distinctCount(COL_A), sum(COL_B) FROM myTable`
if (aggregation.getCall().getDisplayName().equalsIgnoreCase(COUNT_FUNCTION_NAME) && aggregation.getMask().get().getName().equalsIgnoreCase(aggregation.getArguments().get(0) + DISTINCT_MASK)) {
nodeBuilder.add(new AggregationFunctionColumnNode(outputColumn, new CallExpression(aggregation.getCall().getSourceLocation(), DRUID_COUNT_DISTINCT_FUNCTION_NAME, aggregation.getCall().getFunctionHandle(), aggregation.getCall().getType(), aggregation.getCall().getArguments())));
nodeBuilder.add(new AggregationFunctionColumnNode(outputColumn, new CallExpression(DRUID_COUNT_DISTINCT_FUNCTION_NAME, aggregation.getCall().getFunctionHandle(), aggregation.getCall().getType(), aggregation.getCall().getArguments())));
continue;
}
// Druid doesn't support push down aggregation functions other than count on top of distinct function.
Expand Down Expand Up @@ -135,7 +135,6 @@ private static boolean handlePushDownSingleDistinctCount(ImmutableList.Builder<D
new AggregationFunctionColumnNode(
outputColumn,
new CallExpression(
aggregation.getCall().getSourceLocation(),
DRUID_COUNT_DISTINCT_FUNCTION_NAME,
aggregation.getFunctionHandle(),
aggregation.getCall().getType(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -445,13 +445,12 @@ private RowExpression negateComparison(CallExpression expression)
{
OperatorType newOperator = negate(getOperator(expression).orElse(null));
if (newOperator == null) {
return new CallExpression(expression.getSourceLocation(), "NOT", functionResolution.notFunction(), BOOLEAN, singletonList(expression));
return new CallExpression("NOT", functionResolution.notFunction(), BOOLEAN, singletonList(expression));
}
checkArgument(expression.getArguments().size() == 2, "Comparison expression must have exactly two arguments");
RowExpression left = expression.getArguments().get(0).accept(this, null);
RowExpression right = expression.getArguments().get(1).accept(this, null);
return new CallExpression(
left.getSourceLocation(),
newOperator.getOperator(),
functionResolution.comparisonFunction(newOperator, left.getType(), right.getType()),
BOOLEAN,
Expand Down Expand Up @@ -773,7 +772,7 @@ private Optional<OperatorType> getOperator(RowExpression expression)

private RowExpression notCallExpression(RowExpression argument)
{
return new CallExpression(argument.getSourceLocation(), "not", functionResolution.notFunction(), BOOLEAN, singletonList(argument));
return new CallExpression("not", functionResolution.notFunction(), BOOLEAN, singletonList(argument));
}

private static OperatorType negate(OperatorType operator)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public RowExpression visitCall(CallExpression call, Context<C> context)
List<RowExpression> arguments = rewrite(call.getArguments(), context);

if (!sameElements(call.getArguments(), arguments)) {
return new CallExpression(call.getSourceLocation(), call.getDisplayName(), call.getFunctionHandle(), call.getType(), arguments);
return new CallExpression(call.getDisplayName(), call.getFunctionHandle(), call.getType(), arguments);
}
return call;
}
Expand Down Expand Up @@ -132,7 +132,7 @@ public RowExpression visitLambda(LambdaDefinitionExpression lambda, Context<C> c

RowExpression body = rewrite(lambda.getBody(), context.get());
if (body != lambda.getBody()) {
return new LambdaDefinitionExpression(lambda.getSourceLocation(), lambda.getArgumentTypes(), lambda.getArguments(), body);
return new LambdaDefinitionExpression(lambda.getArgumentTypes(), lambda.getArguments(), body);
}

return lambda;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,20 +206,20 @@ private RowExpression toRowExpression(Subfield subfield, List<Type> types)
if (pathElement instanceof Subfield.LongSubscript) {
Type indexType = baseType instanceof MapType ? ((MapType) baseType).getKeyType() : BIGINT;
FunctionHandle functionHandle = functionResolution.subscriptFunction(baseType, indexType);
ConstantExpression index = new ConstantExpression(base.getSourceLocation(), ((Subfield.LongSubscript) pathElement).getIndex(), indexType);
return new CallExpression(base.getSourceLocation(), SUBSCRIPT.name(), functionHandle, types.get(types.size() - 1), ImmutableList.of(base, index));
ConstantExpression index = new ConstantExpression(((Subfield.LongSubscript) pathElement).getIndex(), indexType);
return new CallExpression(SUBSCRIPT.name(), functionHandle, types.get(types.size() - 1), ImmutableList.of(base, index));
}

if (pathElement instanceof Subfield.StringSubscript) {
Type indexType = ((MapType) baseType).getKeyType();
FunctionHandle functionHandle = functionResolution.subscriptFunction(baseType, indexType);
ConstantExpression index = new ConstantExpression(base.getSourceLocation(), Slices.utf8Slice(((Subfield.StringSubscript) pathElement).getIndex()), indexType);
return new CallExpression(base.getSourceLocation(), SUBSCRIPT.name(), functionHandle, types.get(types.size() - 1), ImmutableList.of(base, index));
ConstantExpression index = new ConstantExpression(Slices.utf8Slice(((Subfield.StringSubscript) pathElement).getIndex()), indexType);
return new CallExpression(SUBSCRIPT.name(), functionHandle, types.get(types.size() - 1), ImmutableList.of(base, index));
}

if (pathElement instanceof Subfield.NestedField) {
Subfield.NestedField nestedField = (Subfield.NestedField) pathElement;
return new SpecialFormExpression(base.getSourceLocation(), DEREFERENCE, types.get(types.size() - 1), base, new ConstantExpression(base.getSourceLocation(), getFieldIndex((RowType) baseType, nestedField.getName()), INTEGER));
return new SpecialFormExpression(DEREFERENCE, types.get(types.size() - 1), base, new ConstantExpression(getFieldIndex((RowType) baseType, nestedField.getName()), INTEGER));
}

verify(false, "Unexpected path element: " + pathElement);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ private void assertSameCanonicalLeafSubPlan(Session session, String sql2, String
.map(Optional::get)
.collect(Collectors.toList());
assertEquals(leafCanonicalPlans.size(), 2);
assertEquals(objectMapper.writeValueAsString(leafCanonicalPlans.get(0)).replaceAll("\"sourceLocation\":\\{[^\\}]*\\}", ""), objectMapper.writeValueAsString(leafCanonicalPlans.get(1)).replaceAll("\"sourceLocation\":\\{[^\\}]*\\}", ""));
assertEquals(objectMapper.writeValueAsString(leafCanonicalPlans.get(0)), objectMapper.writeValueAsString(leafCanonicalPlans.get(1)));
}

private void assertDifferentCanonicalLeafSubPlan(Session session, String sql1, String sql2)
Expand All @@ -195,6 +195,6 @@ private void assertDifferentCanonicalLeafSubPlan(Session session, String sql1, S
Optional<CanonicalPlanFragment> canonicalPlan2 = generateCanonicalPlan(fragment2.getRoot(), fragment2.getPartitioningScheme());
assertTrue(canonicalPlan1.isPresent());
assertTrue(canonicalPlan2.isPresent());
assertNotEquals(objectMapper.writeValueAsString(canonicalPlan1).replaceAll("\"sourceLocation\":\\{[^\\}]*\\}", ""), objectMapper.writeValueAsString(canonicalPlan2).replaceAll("\"sourceLocation\":\\{[^\\}]*\\}", ""));
assertNotEquals(objectMapper.writeValueAsString(canonicalPlan1), objectMapper.writeValueAsString(canonicalPlan2));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public void testExplainOfCreateTableAs()
{
String query = "CREATE TABLE copy_orders AS SELECT * FROM orders";
MaterializedResult result = computeActual("EXPLAIN " + query);
assertEquals(getOnlyElement(result.getOnlyColumnAsSet()), getExplainPlan("EXPLAIN ", query, LOGICAL));
assertEquals(getOnlyElement(result.getOnlyColumnAsSet()), getExplainPlan(query, LOGICAL));
}

// Hive specific tests should normally go in TestHiveIntegrationSmokeTest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ public void testExplainOfCreateTableAs()
{
String query = "CREATE TABLE copy_orders AS SELECT * FROM orders";
MaterializedResult result = computeActual("EXPLAIN " + query);
assertEquals(getOnlyElement(result.getOnlyColumnAsSet()), getExplainPlan("EXPLAIN ", query, LOGICAL));
assertEquals(getOnlyElement(result.getOnlyColumnAsSet()), getExplainPlan(query, LOGICAL));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,6 @@ public void testExplainOfCreateTableAs()
{
String query = "CREATE TABLE copy_orders AS SELECT * FROM orders";
MaterializedResult result = computeActual("EXPLAIN " + query);
assertEquals(getOnlyElement(result.getOnlyColumnAsSet()), getExplainPlan("EXPLAIN ", query, LOGICAL));
assertEquals(getOnlyElement(result.getOnlyColumnAsSet()), getExplainPlan(query, LOGICAL));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,6 @@ public void testExplainOfCreateTableAs()
{
String query = "CREATE TABLE copy_orders AS SELECT * FROM orders";
MaterializedResult result = computeActual("EXPLAIN " + query);
assertEquals(getOnlyElement(result.getOnlyColumnAsSet()), getExplainPlan("EXPLAIN ", query, LOGICAL));
assertEquals(getOnlyElement(result.getOnlyColumnAsSet()), getExplainPlan(query, LOGICAL));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@
import static com.facebook.presto.spi.relation.ExpressionOptimizer.Level.OPTIMIZED;
import static com.facebook.presto.spi.relation.SpecialFormExpression.Form.IS_NULL;
import static com.facebook.presto.sql.ExpressionUtils.and;
import static com.facebook.presto.sql.analyzer.ExpressionTreeUtils.getSourceLocation;
import static com.facebook.presto.sql.analyzer.TypeSignatureProvider.fromTypes;
import static com.facebook.presto.sql.relational.Expressions.call;
import static com.facebook.presto.sql.relational.Expressions.constantNull;
Expand Down Expand Up @@ -172,7 +171,7 @@ private RowExpression simplifyExpression(ConnectorSession session, RowExpression
// Expression evaluates to SQL null, which in Filter is equivalent to false. This assumes the expression is a top-level expression (eg. not in NOT).
value = false;
}
return LiteralEncoder.toRowExpression(predicate.getSourceLocation(), value, BOOLEAN);
return LiteralEncoder.toRowExpression(value, BOOLEAN);
}

private Map<NodeRef<Expression>, Type> getExpressionTypes(Session session, Expression expression, TypeProvider types)
Expand Down Expand Up @@ -483,7 +482,7 @@ private VariableStatsEstimate getExpressionStats(Expression expression)
private VariableReferenceExpression toVariable(Expression expression)
{
checkArgument(expression instanceof SymbolReference, "Unexpected expression: %s", expression);
return new VariableReferenceExpression(getSourceLocation(expression), ((SymbolReference) expression).getName(), types.get(expression));
return new VariableReferenceExpression(((SymbolReference) expression).getName(), types.get(expression));
}
}

Expand Down Expand Up @@ -564,13 +563,13 @@ public PlanNodeStatsEstimate visitCall(CallExpression node, Void context)
if (!(left instanceof VariableReferenceExpression) && right instanceof VariableReferenceExpression) {
// normalize so that variable is on the left
OperatorType flippedOperator = flip(operatorType);
return process(call(node.getSourceLocation(), flippedOperator.name(), metadata.getFunctionAndTypeManager().resolveOperator(flippedOperator, fromTypes(right.getType(), left.getType())), BOOLEAN, right, left));
return process(call(flippedOperator.name(), metadata.getFunctionAndTypeManager().resolveOperator(flippedOperator, fromTypes(right.getType(), left.getType())), BOOLEAN, right, left));
}

if (left instanceof ConstantExpression) {
// normalize so that literal is on the right
OperatorType flippedOperator = flip(operatorType);
return process(call(node.getSourceLocation(), flippedOperator.name(), metadata.getFunctionAndTypeManager().resolveOperator(flippedOperator, fromTypes(right.getType(), left.getType())), BOOLEAN, right, left));
return process(call(flippedOperator.name(), metadata.getFunctionAndTypeManager().resolveOperator(flippedOperator, fromTypes(right.getType(), left.getType())), BOOLEAN, right, left));
}

if (left instanceof VariableReferenceExpression && left.equals(right)) {
Expand All @@ -582,7 +581,7 @@ public PlanNodeStatsEstimate visitCall(CallExpression node, Void context)
if (right instanceof ConstantExpression) {
Object rightValue = ((ConstantExpression) right).getValue();
if (rightValue == null) {
return visitConstant(constantNull(right.getSourceLocation(), BOOLEAN), null);
return visitConstant(constantNull(BOOLEAN), null);
}
OptionalDouble literal = toStatsRepresentation(metadata.getFunctionAndTypeManager(), session, right.getType(), rightValue);
return estimateExpressionToLiteralComparison(input, leftStats, leftVariable, literal, getComparisonOperator(operatorType));
Expand Down Expand Up @@ -634,14 +633,12 @@ public PlanNodeStatsEstimate visitCall(CallExpression node, Void context)

VariableStatsEstimate valueStats = input.getVariableStatistics((VariableReferenceExpression) value);
RowExpression lowerBound = call(
value.getSourceLocation(),
OperatorType.GREATER_THAN_OR_EQUAL.name(),
metadata.getFunctionAndTypeManager().resolveOperator(OperatorType.GREATER_THAN_OR_EQUAL, fromTypes(value.getType(), min.getType())),
BOOLEAN,
value,
min);
RowExpression upperBound = call(
value.getSourceLocation(),
OperatorType.LESS_THAN_OR_EQUAL.name(),
metadata.getFunctionAndTypeManager().resolveOperator(OperatorType.LESS_THAN_OR_EQUAL, fromTypes(value.getType(), max.getType())),
BOOLEAN,
Expand Down Expand Up @@ -740,7 +737,7 @@ private PlanNodeStatsEstimate estimateLogicalOr(RowExpression left, RowExpressio
private PlanNodeStatsEstimate estimateIn(RowExpression value, List<RowExpression> candidates)
{
ImmutableList<PlanNodeStatsEstimate> equalityEstimates = candidates.stream()
.map(inValue -> process(call(value.getSourceLocation(), OperatorType.EQUAL.name(), metadata.getFunctionAndTypeManager().resolveOperator(OperatorType.EQUAL, fromTypes(value.getType(), inValue.getType())), BOOLEAN, value, inValue)))
.map(inValue -> process(call(OperatorType.EQUAL.name(), metadata.getFunctionAndTypeManager().resolveOperator(OperatorType.EQUAL, fromTypes(value.getType(), inValue.getType())), BOOLEAN, value, inValue)))
.collect(toImmutableList());

if (equalityEstimates.stream().anyMatch(PlanNodeStatsEstimate::isOutputRowCountUnknown)) {
Expand Down Expand Up @@ -794,12 +791,12 @@ private PlanNodeStatsEstimate estimateIsNull(RowExpression expression)

private RowExpression isNull(RowExpression expression)
{
return new SpecialFormExpression(expression.getSourceLocation(), IS_NULL, BOOLEAN, expression);
return new SpecialFormExpression(IS_NULL, BOOLEAN, expression);
}

private RowExpression not(RowExpression expression)
{
return call(expression.getSourceLocation(), "not", functionResolution.notFunction(), expression.getType(), expression);
return call("not", functionResolution.notFunction(), expression.getType(), expression);
}

private ComparisonExpression.Operator getComparisonOperator(OperatorType operator)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import static com.facebook.presto.cost.FilterStatsCalculator.UNKNOWN_FILTER_COEFFICIENT;
import static com.facebook.presto.cost.VariableStatsEstimate.buildFrom;
import static com.facebook.presto.sql.ExpressionUtils.extractConjuncts;
import static com.facebook.presto.sql.analyzer.ExpressionTreeUtils.getNodeLocation;
import static com.facebook.presto.sql.planner.plan.Patterns.join;
import static com.facebook.presto.sql.relational.OriginalExpressionUtils.castToExpression;
import static com.facebook.presto.sql.relational.OriginalExpressionUtils.isExpression;
Expand Down Expand Up @@ -223,7 +222,7 @@ private PlanNodeStatsEstimate filterByEquiJoinClauses(
Session session,
TypeProvider types)
{
ComparisonExpression drivingPredicate = new ComparisonExpression(EQUAL, new SymbolReference(getNodeLocation(drivingClause.getLeft().getSourceLocation()), drivingClause.getLeft().getName()), new SymbolReference(getNodeLocation(drivingClause.getRight().getSourceLocation()), drivingClause.getRight().getName()));
ComparisonExpression drivingPredicate = new ComparisonExpression(EQUAL, new SymbolReference(drivingClause.getLeft().getName()), new SymbolReference(drivingClause.getRight().getName()));
PlanNodeStatsEstimate filteredStats = filterStatsCalculator.filterStats(stats, drivingPredicate, session, types);
for (EquiJoinClause clause : remainingClauses) {
filteredStats = filterByAuxiliaryClause(filteredStats, clause);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
import static com.facebook.presto.cost.StatsUtil.toStatsRepresentation;
import static com.facebook.presto.spi.relation.ExpressionOptimizer.Level.OPTIMIZED;
import static com.facebook.presto.spi.relation.SpecialFormExpression.Form.COALESCE;
import static com.facebook.presto.sql.analyzer.ExpressionTreeUtils.getSourceLocation;
import static com.facebook.presto.sql.planner.LiteralInterpreter.evaluate;
import static com.facebook.presto.sql.relational.Expressions.isNull;
import static com.facebook.presto.util.MoreMath.max;
Expand Down Expand Up @@ -343,7 +342,7 @@ protected VariableStatsEstimate visitNode(Node node, Void context)
@Override
protected VariableStatsEstimate visitSymbolReference(SymbolReference node, Void context)
{
return input.getVariableStatistics(new VariableReferenceExpression(getSourceLocation(node), node.getName(), types.get(node)));
return input.getVariableStatistics(new VariableReferenceExpression(node.getName(), types.get(node)));
}

@Override
Expand Down
Loading

0 comments on commit 547a068

Please sign in to comment.