Skip to content

Commit

Permalink
Resolve function using all registered function namespaces
Browse files Browse the repository at this point in the history 
We enforce explicit naming for dynamic function namespaces. All
unqualified function names will only be resolved against the built-in
static function namespace. While it is possible to define an ordering
(through SQL path or other means) and convention (best match / first
match), in reality when complicated namespaces are involved such
implicit resolution might hide errors and cause confusion.
  • Loading branch information
@rongrong
rongrong committed Sep 4, 2019
1 parent c359c21 commit 3379b4f
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 32 deletions.
7 changes: 7 additions & 0 deletions presto-main/src/main/java/com/facebook/presto/metadata/BuiltInFunctionHandle.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import com.facebook.presto.spi.function.FunctionHandle;
import com.facebook.presto.spi.function.Signature;
import com.facebook.presto.spi.relation.FullyQualifiedName;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;

Expand All @@ -40,6 +41,12 @@ public Signature getSignature()
return signature;
}

@Override
public FullyQualifiedName.Prefix getFunctionNamespace()
{
return signature.getName().getPrefix();
}

@Override
public boolean equals(Object o)
{
Expand Down
108 changes: 77 additions & 31 deletions presto-main/src/main/java/com/facebook/presto/metadata/FunctionManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import com.facebook.presto.operator.scalar.ScalarFunctionImplementation;
import com.facebook.presto.operator.window.WindowFunctionSupplier;
import com.facebook.presto.spi.PrestoException;
import com.facebook.presto.spi.QueryId;
import com.facebook.presto.spi.block.BlockEncodingSerde;
import com.facebook.presto.spi.function.FunctionHandle;
import com.facebook.presto.spi.function.FunctionKind;
Expand Down Expand Up @@ -93,6 +94,7 @@ public FunctionManager(TypeManager typeManager, BlockEncodingSerde blockEncoding
{
this.typeManager = requireNonNull(typeManager, "typeManager is null");
this.builtInFunctionNamespace = new BuiltInFunctionNamespaceManager(typeManager, blockEncodingSerde, featuresConfig, this);
this.functionNamespaces.put(DEFAULT_NAMESPACE, builtInFunctionNamespace);
this.functionInvokerProvider = new FunctionInvokerProvider(this);
this.handleResolver = handleResolver;
if (typeManager instanceof TypeRegistry) {
Expand All @@ -114,7 +116,6 @@ public void loadFunctionNamespaces(String functionNamespaceManagerName, List<Str
FunctionNamespaceManager manager = factory.create(properties);

for (String functionNamespacePrefix : functionNamespacePrefixes) {
FullyQualifiedName.Prefix prefix = FullyQualifiedName.Prefix.of(functionNamespacePrefix);
if (functionNamespaces.putIfAbsent(FullyQualifiedName.Prefix.of(functionNamespacePrefix), manager) != null) {
throw new IllegalArgumentException(format("Function namespace manager '%s' already registered to handle function namespace '%s'", factory.getName(), functionNamespacePrefix));
}
Expand Down Expand Up @@ -147,30 +148,43 @@ public List<SqlFunction> listFunctions()
}

/**
* Resolves a function using the SQL path, and implicit type coercions.
* Resolves a function using implicit type coercions. We enforce explicit naming for dynamic function namespaces.
* All unqualified function names will only be resolved against the built-in static function namespace. While it is
* possible to define an ordering (through SQL path or other means) and convention (best match / first match), in
* reality when complicated namespaces are involved such implicit resolution might hide errors and cause confusion.
*
* @throws PrestoException if there are no matches or multiple matches
*/
public FunctionHandle resolveFunction(Session session, QualifiedName name, List<TypeSignatureProvider> parameterTypes)
{
// TODO Actually use session
// Session will be used to provide information about the order of function namespaces to through resolving the function.
// This is likely to be in terms of SQL path. Currently we still don't have support multiple function namespaces, nor
// SQL path. As a result, session is not used here. We still add this to distinguish the two versions of resolveFunction
// while the refactoring is on-going.
FullyQualifiedName fullyQualifiedName;
if (!name.getPrefix().isPresent()) {
fullyQualifiedName = FullyQualifiedName.of(DEFAULT_NAMESPACE, name.getSuffix());
}
else {
fullyQualifiedName = FullyQualifiedName.of(name.getOriginalParts());
}

Optional<FunctionNamespaceManager> servingNamespaceManager = getServingNamespaceManager(fullyQualifiedName);
if (!servingNamespaceManager.isPresent()) {
throw new PrestoException(FUNCTION_NOT_FOUND, format("Cannot find function namespace for function %s", name));
}

QueryId queryId = session == null ? null : session.getQueryId();
Collection<SqlFunction> allCandidates = servingNamespaceManager.get().getCandidates(queryId, fullyQualifiedName);

try {
return lookupFunction(name.getSuffix(), parameterTypes);
return lookupFunction(servingNamespaceManager.get(), queryId, fullyQualifiedName, parameterTypes, allCandidates);
}
catch (PrestoException e) {
if (e.getErrorCode().getCode() != FUNCTION_NOT_FOUND.toErrorCode().getCode()) {
throw e;
}
}
FullyQualifiedName fullyQualifiedName = FullyQualifiedName.of(DEFAULT_NAMESPACE, name.getSuffix());
Collection<SqlFunction> allCandidates = builtInFunctionNamespace.getCandidates(null, fullyQualifiedName);

Optional<Signature> match = matchFunctionWithCoercion(allCandidates, parameterTypes);
if (match.isPresent()) {
return new BuiltInFunctionHandle(match.get());
return servingNamespaceManager.get().getFunctionHandle(queryId, match.get());
}

if (name.getSuffix().startsWith(MAGIC_LITERAL_FUNCTION_PREFIX)) {
Expand All @@ -186,13 +200,13 @@ public FunctionHandle resolveFunction(Session session, QualifiedName name, List<
return new BuiltInFunctionHandle(getMagicLiteralFunctionSignature(type));
}

throw new PrestoException(FUNCTION_NOT_FOUND, constructFunctionNotFoundErrorMessage(name.getSuffix(), parameterTypes, allCandidates));
throw new PrestoException(FUNCTION_NOT_FOUND, constructFunctionNotFoundErrorMessage(name.toString(), parameterTypes, allCandidates));
}

@Override
public FunctionMetadata getFunctionMetadata(FunctionHandle functionHandle)
{
return builtInFunctionNamespace.getFunctionMetadata(functionHandle);
return functionNamespaces.get(functionHandle.getFunctionNamespace()).getFunctionMetadata(functionHandle);
}

public WindowFunctionSupplier getWindowFunctionImplementation(FunctionHandle functionHandle)
Expand Down Expand Up @@ -251,13 +265,34 @@ public FunctionHandle lookupFunction(String name, List<TypeSignatureProvider> pa
{
FullyQualifiedName fullyQualifiedName = FullyQualifiedName.of(DEFAULT_NAMESPACE, name);
Collection<SqlFunction> allCandidates = builtInFunctionNamespace.getCandidates(null, fullyQualifiedName);
return lookupFunction(builtInFunctionNamespace, null, fullyQualifiedName, parameterTypes, allCandidates);
}

public FunctionHandle lookupCast(CastType castType, TypeSignature fromType, TypeSignature toType)
{
Signature signature = new Signature(castType.getCastName(), SCALAR, emptyList(), emptyList(), toType, singletonList(fromType), false);

try {
builtInFunctionNamespace.getScalarFunctionImplementation(signature);
}
catch (PrestoException e) {
if (castType.isOperatorType() && e.getErrorCode().getCode() == FUNCTION_IMPLEMENTATION_MISSING.toErrorCode().getCode()) {
throw new OperatorNotFoundException(toOperatorType(castType), ImmutableList.of(fromType), toType);
}
throw e;
}
return builtInFunctionNamespace.getFunctionHandle(null, signature);
}

private FunctionHandle lookupFunction(FunctionNamespaceManager functionNamespaceManager, QueryId queryId, FullyQualifiedName name, List<TypeSignatureProvider> parameterTypes, Collection<SqlFunction> allCandidates)
{
List<SqlFunction> exactCandidates = allCandidates.stream()
.filter(function -> function.getSignature().getTypeVariableConstraints().isEmpty())
.collect(Collectors.toList());

Optional<Signature> match = matchFunctionExact(exactCandidates, parameterTypes);
if (match.isPresent()) {
return new BuiltInFunctionHandle(match.get());
return functionNamespaceManager.getFunctionHandle(queryId, match.get());
}

List<SqlFunction> genericCandidates = allCandidates.stream()
Expand All @@ -266,26 +301,29 @@ public FunctionHandle lookupFunction(String name, List<TypeSignatureProvider> pa

match = matchFunctionExact(genericCandidates, parameterTypes);
if (match.isPresent()) {
return new BuiltInFunctionHandle(match.get());
return functionNamespaceManager.getFunctionHandle(queryId, match.get());
}

throw new PrestoException(FUNCTION_NOT_FOUND, constructFunctionNotFoundErrorMessage(name, parameterTypes, allCandidates));
throw new PrestoException(FUNCTION_NOT_FOUND, constructFunctionNotFoundErrorMessage(name.toString(), parameterTypes, allCandidates));
}

public FunctionHandle lookupCast(CastType castType, TypeSignature fromType, TypeSignature toType)
private Optional<FunctionNamespaceManager> getServingNamespaceManager(FullyQualifiedName name)
{
Signature signature = new Signature(castType.getCastName(), SCALAR, emptyList(), emptyList(), toType, singletonList(fromType), false);

try {
builtInFunctionNamespace.getScalarFunctionImplementation(signature);
FullyQualifiedName.Prefix functionPrefix = name.getPrefix();
if (functionPrefix.equals(DEFAULT_NAMESPACE)) {
return Optional.of(builtInFunctionNamespace);
}
catch (PrestoException e) {
if (castType.isOperatorType() && e.getErrorCode().getCode() == FUNCTION_IMPLEMENTATION_MISSING.toErrorCode().getCode()) {
throw new OperatorNotFoundException(toOperatorType(castType), ImmutableList.of(fromType), toType);

FullyQualifiedName.Prefix bestMatchNamespace = null;
FunctionNamespaceManager servingNamespaceManager = null;

for (Map.Entry<FullyQualifiedName.Prefix, FunctionNamespaceManager> functionNamespace : functionNamespaces.entrySet()) {
if (functionNamespace.getKey().contains(functionPrefix) && (bestMatchNamespace == null || bestMatchNamespace.contains(functionNamespace.getKey()))) {
bestMatchNamespace = functionNamespace.getKey();
servingNamespaceManager = functionNamespace.getValue();
}
throw e;
}
return builtInFunctionNamespace.getFunctionHandle(null, signature);
return Optional.ofNullable(servingNamespaceManager);
}

private String constructFunctionNotFoundErrorMessage(String name, List<TypeSignatureProvider> parameterTypes, Collection<SqlFunction> candidates)
Expand Down Expand Up @@ -351,7 +389,7 @@ private List<ApplicableFunction> identifyApplicableFunctions(Collection<SqlFunct
Optional<Signature> boundSignature = new SignatureBinder(typeManager, declaredSignature, allowCoercion)
.bind(actualParameters);
if (boundSignature.isPresent()) {
applicableFunctions.add(new ApplicableFunction(declaredSignature, boundSignature.get()));
applicableFunctions.add(new ApplicableFunction(declaredSignature, boundSignature.get(), function.isCalledOnNullInput()));
}
}
return applicableFunctions.build();
Expand Down Expand Up @@ -475,9 +513,10 @@ private boolean returnsNullOnGivenInputTypes(ApplicableFunction applicableFuncti
for (int i = 0; i < parameterTypes.size(); i++) {
Type parameterType = parameterTypes.get(i);
if (parameterType.equals(UNKNOWN)) {
// TODO: This still doesn't feel right. Need to understand function resolution logic better to know what's the right way.
BuiltInFunctionHandle functionHandle = new BuiltInFunctionHandle(boundSignature);
if (getFunctionMetadata(functionHandle).isCalledOnNullInput()) {
// The original implementation checks only whether the particular argument has @SqlNullable.
// However, RETURNS NULL ON NULL INPUT / CALLED ON NULL INPUT is a function level metadata according
// to SQL spec. So there is a loss of precision here.
if (applicableFunction.isCalledOnNullInput()) {
return false;
}
}
Expand Down Expand Up @@ -512,11 +551,13 @@ private static class ApplicableFunction
{
private final Signature declaredSignature;
private final Signature boundSignature;
private final boolean isCalledOnNullInput;

private ApplicableFunction(Signature declaredSignature, Signature boundSignature)
private ApplicableFunction(Signature declaredSignature, Signature boundSignature, boolean isCalledOnNullInput)
{
this.declaredSignature = declaredSignature;
this.boundSignature = boundSignature;
this.isCalledOnNullInput = isCalledOnNullInput;
}

public Signature getDeclaredSignature()
Expand All @@ -529,6 +570,11 @@ public Signature getBoundSignature()
return boundSignature;
}

public boolean isCalledOnNullInput()
{
return isCalledOnNullInput;
}

@Override
public String toString()
{
Expand Down
2 changes: 1 addition & 1 deletion presto-main/src/main/java/com/facebook/presto/server/PluginManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ public void installPlugin(Plugin plugin)
}

for (FunctionNamespaceManagerFactory functionNamespaceManagerFactory : plugin.getFunctionNamespaceManagerFactories()) {
log.info("Registering function namespace %s", functionNamespaceManagerFactory.getName());
log.info("Registering function namespace manager %s", functionNamespaceManagerFactory.getName());
metadata.getFunctionManager().addFunctionNamespaceFactory(functionNamespaceManagerFactory);
}

Expand Down
6 changes: 6 additions & 0 deletions ...-main/src/test/java/com/facebook/presto/sql/planner/TestRowExpressionVariableInliner.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

import com.facebook.presto.spi.function.FunctionHandle;
import com.facebook.presto.spi.relation.CallExpression;
import com.facebook.presto.spi.relation.FullyQualifiedName;
import com.facebook.presto.spi.relation.LambdaDefinitionExpression;
import com.facebook.presto.spi.relation.VariableReferenceExpression;
import com.google.common.collect.ImmutableList;
Expand All @@ -29,6 +30,11 @@ public class TestRowExpressionVariableInliner
private static class TestFunctionHandle
implements FunctionHandle
{
@Override
public FullyQualifiedName.Prefix getFunctionNamespace()
{
return FullyQualifiedName.of("a.b.c").getPrefix();
}
}

private static final FunctionHandle TEST_FUNCTION = new TestFunctionHandle();
Expand Down
2 changes: 2 additions & 0 deletions presto-spi/src/main/java/com/facebook/presto/spi/function/FunctionHandle.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package com.facebook.presto.spi.function;

import com.facebook.presto.spi.api.Experimental;
import com.facebook.presto.spi.relation.FullyQualifiedName;

/**
* FunctionHandle is a unique handle to identify the function implementation from namespaces.
Expand All @@ -22,4 +23,5 @@
@Experimental
public interface FunctionHandle
{
FullyQualifiedName.Prefix getFunctionNamespace();
}
1 change: 1 addition & 0 deletions presto-spi/src/main/java/com/facebook/presto/spi/function/FunctionNamespaceManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public interface FunctionNamespaceManager
* is deleted. Multiple calls of this function with the same parameters should return the same FunctionHandle.
* queryId serves as a transaction ID before proper support for transaction is introduced.
* TODO Support transaction in function namespaces
* @return FunctionHandle or null if the namespace manager does not manage any function with the given signature.
*/
FunctionHandle getFunctionHandle(QueryId queryId, Signature signature);

Expand Down
16 changes: 16 additions & 0 deletions presto-spi/src/main/java/com/facebook/presto/spi/relation/FullyQualifiedName.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,25 @@ private Prefix(List<String> parts)
public static Prefix of(String dottedName)
{
String[] parts = dottedName.split("\\.");
if (parts.length < 2) {
throw new IllegalArgumentException("Prefix should be in the form of a.b(.c...) with at least 1 dot");
}
return new Prefix(asList(parts));
}

public boolean contains(Prefix other)
{
if (parts.size() > other.parts.size()) {
return false;
}
for (int i = 0; i < parts.size(); i++) {
if (!parts.get(i).equals(other.parts.get(i))) {
return false;
}
}
return true;
}

@Override
public String toString()
{
Expand Down

0 comments on commit 3379b4f

Please sign in to comment.