Skip to content

Commit 2ea87fe

Browse files
tdcmeehantdcmeehan
committed
Always evaluate built-in functions using the presto.default namespace
If the user has no specified a custom expression optimizer, expressions should be presumed to be in the presto.default namespace (which consists exclusively of Java functions).
1 parent 08b21a5 commit 2ea87fe

File tree

2 files changed

+96
-2
lines changed

2 files changed

+96
-2
lines changed

‎presto-main-base/src/main/java/com/facebook/presto/sql/relational/RowExpressionOptimizer.java

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,24 +13,33 @@
1313
*/
1414
package com.facebook.presto.sql.relational;
1515

16+
import com.facebook.presto.common.CatalogSchemaName;
17+
import com.facebook.presto.common.QualifiedObjectName;
1618
import com.facebook.presto.metadata.FunctionAndTypeManager;
1719
import com.facebook.presto.metadata.Metadata;
1820
import com.facebook.presto.spi.ConnectorSession;
21+
import com.facebook.presto.spi.relation.CallExpression;
1922
import com.facebook.presto.spi.relation.ExpressionOptimizer;
2023
import com.facebook.presto.spi.relation.RowExpression;
24+
import com.facebook.presto.spi.relation.RowExpressionVisitor;
2125
import com.facebook.presto.spi.relation.VariableReferenceExpression;
26+
import com.facebook.presto.sql.analyzer.TypeSignatureProvider;
2227
import com.facebook.presto.sql.planner.RowExpressionInterpreter;
2328

2429
import java.util.function.Function;
2530

31+
import static com.facebook.presto.metadata.BuiltInTypeAndFunctionNamespaceManager.JAVA_BUILTIN_NAMESPACE;
2632
import static com.facebook.presto.spi.relation.ExpressionOptimizer.Level.OPTIMIZED;
2733
import static com.facebook.presto.sql.planner.LiteralEncoder.toRowExpression;
34+
import static com.google.common.collect.ImmutableList.toImmutableList;
2835
import static java.util.Objects.requireNonNull;
2936

3037
public final class RowExpressionOptimizer
3138
implements ExpressionOptimizer
3239
{
3340
private final FunctionAndTypeManager functionAndTypeManager;
41+
private final CatalogSchemaName defaultNamespace;
42+
private final RowExpressionVisitor<RowExpression, Void> namespaceNormalizer;
3443

3544
public RowExpressionOptimizer(Metadata metadata)
3645
{
@@ -39,22 +48,78 @@ public RowExpressionOptimizer(Metadata metadata)
3948

4049
public RowExpressionOptimizer(FunctionAndTypeManager functionAndTypeManager)
4150
{
51+
this.defaultNamespace = requireNonNull(functionAndTypeManager, "functionMetadataManager is null").getDefaultNamespace();
4252
this.functionAndTypeManager = requireNonNull(functionAndTypeManager, "functionMetadataManager is null");
53+
this.namespaceNormalizer = !JAVA_BUILTIN_NAMESPACE.equals(defaultNamespace) ? new BuiltInFunctionNamespaceOverride() : new IdentityRowExpressionVisitor();
4354
}
4455

4556
@Override
4657
public RowExpression optimize(RowExpression rowExpression, Level level, ConnectorSession session)
4758
{
4859
if (level.ordinal() <= OPTIMIZED.ordinal()) {
49-
return toRowExpression(rowExpression.getSourceLocation(), new RowExpressionInterpreter(rowExpression, functionAndTypeManager, session, level).optimize(), rowExpression.getType());
60+
RowExpressionInterpreter rowExpressionInterpreter = new RowExpressionInterpreter(
61+
rowExpression.accept(namespaceNormalizer, null),
62+
functionAndTypeManager,
63+
session,
64+
level);
65+
return toRowExpression(rowExpression.getSourceLocation(), rowExpressionInterpreter.optimize(), rowExpression.getType());
5066
}
5167
throw new IllegalArgumentException("Not supported optimization level: " + level);
5268
}
5369

5470
@Override
5571
public RowExpression optimize(RowExpression expression, Level level, ConnectorSession session, Function<VariableReferenceExpression, Object> variableResolver)
5672
{
57-
RowExpressionInterpreter interpreter = new RowExpressionInterpreter(expression, functionAndTypeManager, session, level);
73+
RowExpressionInterpreter interpreter = new RowExpressionInterpreter(
74+
expression.accept(namespaceNormalizer, null),
75+
functionAndTypeManager,
76+
session,
77+
level);
5878
return toRowExpression(expression.getSourceLocation(), interpreter.optimize(variableResolver::apply), expression.getType());
5979
}
80+
81+
/**
82+
* TODO: GIANT HACK
83+
* This class is a hack and should eventually be removed. It is used to ensure consistent constant folding behavior when the built-in
84+
* function namespace has been switched (for example, to native.default. in the case of native functions). This will no longer be needed
85+
* when the native sidecar is capable of providing its own expression optimizer.
86+
*/
87+
private class BuiltInFunctionNamespaceOverride
88+
implements RowExpressionVisitor<RowExpression, Void>
89+
{
90+
@Override
91+
public RowExpression visitExpression(RowExpression expression, Void context)
92+
{
93+
return expression;
94+
}
95+
96+
@Override
97+
public RowExpression visitCall(CallExpression call, Void context)
98+
{
99+
if (call.getFunctionHandle().getCatalogSchemaName().equals(defaultNamespace)) {
100+
call = new CallExpression(
101+
call.getSourceLocation(),
102+
call.getDisplayName(),
103+
functionAndTypeManager.lookupFunction(
104+
QualifiedObjectName.valueOf(JAVA_BUILTIN_NAMESPACE, call.getDisplayName()),
105+
call.getArguments().stream()
106+
.map(RowExpression::getType)
107+
.map(x -> new TypeSignatureProvider(x.getTypeSignature()))
108+
.collect(toImmutableList())),
109+
call.getType(),
110+
call.getArguments());
111+
}
112+
return call;
113+
}
114+
}
115+
116+
private class IdentityRowExpressionVisitor
117+
implements RowExpressionVisitor<RowExpression, Void>
118+
{
119+
@Override
120+
public RowExpression visitExpression(RowExpression expression, Void context)
121+
{
122+
return expression;
123+
}
124+
}
60125
}

‎presto-main-base/src/test/java/com/facebook/presto/sql/relational/TestRowExpressionOptimizer.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,20 @@
1313
*/
1414
package com.facebook.presto.sql.relational;
1515

16+
import com.facebook.presto.common.QualifiedObjectName;
1617
import com.facebook.presto.common.block.IntArrayBlock;
1718
import com.facebook.presto.common.type.ArrayType;
1819
import com.facebook.presto.common.type.RowType;
20+
import com.facebook.presto.metadata.BuiltInFunctionHandle;
1921
import com.facebook.presto.metadata.FunctionAndTypeManager;
2022
import com.facebook.presto.metadata.MetadataManager;
2123
import com.facebook.presto.spi.function.FunctionHandle;
24+
import com.facebook.presto.spi.function.Signature;
2225
import com.facebook.presto.spi.relation.CallExpression;
2326
import com.facebook.presto.spi.relation.ConstantExpression;
2427
import com.facebook.presto.spi.relation.RowExpression;
2528
import com.facebook.presto.spi.relation.SpecialFormExpression;
29+
import com.facebook.presto.sql.analyzer.FunctionsConfig;
2630
import com.google.common.collect.ImmutableList;
2731
import org.testng.annotations.AfterClass;
2832
import org.testng.annotations.BeforeClass;
@@ -34,6 +38,7 @@
3438
import static com.facebook.presto.common.function.OperatorType.EQUAL;
3539
import static com.facebook.presto.common.type.BigintType.BIGINT;
3640
import static com.facebook.presto.common.type.BooleanType.BOOLEAN;
41+
import static com.facebook.presto.common.type.DoubleType.DOUBLE;
3742
import static com.facebook.presto.common.type.IntegerType.INTEGER;
3843
import static com.facebook.presto.common.type.JsonType.JSON;
3944
import static com.facebook.presto.common.type.TypeSignature.parseTypeSignature;
@@ -43,6 +48,7 @@
4348
import static com.facebook.presto.metadata.CastType.JSON_TO_MAP_CAST;
4449
import static com.facebook.presto.metadata.CastType.JSON_TO_ROW_CAST;
4550
import static com.facebook.presto.metadata.FunctionAndTypeManager.createTestFunctionAndTypeManager;
51+
import static com.facebook.presto.spi.function.FunctionKind.SCALAR;
4652
import static com.facebook.presto.spi.relation.ExpressionOptimizer.Level.OPTIMIZED;
4753
import static com.facebook.presto.spi.relation.SpecialFormExpression.Form.IF;
4854
import static com.facebook.presto.sql.analyzer.TypeSignatureProvider.fromTypes;
@@ -52,7 +58,9 @@
5258
import static com.facebook.presto.testing.TestingConnectorSession.SESSION;
5359
import static com.facebook.presto.util.StructuralTestUtil.mapType;
5460
import static io.airlift.slice.Slices.utf8Slice;
61+
import static java.lang.String.format;
5562
import static org.testng.Assert.assertEquals;
63+
import static org.testng.Assert.assertThrows;
5664

5765
public class TestRowExpressionOptimizer
5866
{
@@ -134,6 +142,27 @@ public void testCastWithJsonParseOptimization()
134142
call(JSON_TO_ROW_CAST.name(), functionAndTypeManager.lookupCast(JSON_TO_ROW_CAST, VARCHAR, functionAndTypeManager.getType(parseTypeSignature("row(varchar,bigint)"))), RowType.anonymous(ImmutableList.of(VARCHAR, BIGINT)), field(1, VARCHAR)));
135143
}
136144

145+
@Test
146+
public void testDefaultExpressionOptimizerUsesJavaNamespaceForBuiltInFunctions()
147+
{
148+
String nativePrefix = "native.default";
149+
MetadataManager metadata = MetadataManager.createTestMetadataManager(new FunctionsConfig().setDefaultNamespacePrefix(nativePrefix));
150+
RowExpressionOptimizer nativeOptimizer = new RowExpressionOptimizer(metadata);
151+
RowExpression simpleAddition = call(
152+
"sqrt",
153+
new BuiltInFunctionHandle(
154+
new Signature(
155+
QualifiedObjectName.valueOf(format("%s.sqrt", nativePrefix)),
156+
SCALAR,
157+
BIGINT.getTypeSignature(),
158+
ImmutableList.of(BIGINT.getTypeSignature()))),
159+
DOUBLE,
160+
ImmutableList.of(
161+
constant(4L, BIGINT)));
162+
assertEquals(nativeOptimizer.optimize(simpleAddition, OPTIMIZED, SESSION), constant(2.0, DOUBLE));
163+
assertThrows(IllegalArgumentException.class, () -> optimizer.optimize(simpleAddition, OPTIMIZED, SESSION));
164+
}
165+
137166
private static RowExpression ifExpression(RowExpression condition, long trueValue, long falseValue)
138167
{
139168
return new SpecialFormExpression(IF, BIGINT, ImmutableList.of(condition, constant(trueValue, BIGINT), constant(falseValue, BIGINT)));

0 commit comments

Comments
 (0)