Github user arina-ielchiieva commented on a diff in the pull request:
https://github.com/apache/drill/pull/777#discussion_r105628751
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/FunctionImplementationRegistry.java
---
@@ -160,7 +168,7 @@ public DrillFuncHolder
findDrillFunction(FunctionResolver functionResolver, Func
FunctionResolver exactResolver =
FunctionResolverFactory.getExactResolver(functionCall);
DrillFuncHolder holder = exactResolver.getBestMatch(functions,
functionCall);
- if (holder == null) {
+ if (holder == null && useDynamicUdfs) {
--- End diff --
1. Since you have mentioned I remembered one more issue with
FunctionImplementationRegistry, it can access only system options, so using
`ExecConstants.USE_DYNAMIC_UDFS` won't work properly since it can be set at
session level as well. I guess using bootsrap option you introduced is OK for
now. Regarding your suggestion to have single option OFF, READ_ONLY and ON to
handle the various cases (I love this idea!), we can try to implement this the
scope of MVCC (I'll add this point to the document).
2. Even with boostrap option we need to update `findDrillFunction` to use
provided function resolver when dynamic udfs are turned off (more details in my
first comment). For example, `findDrillFunction` should can be re-written the
following way (please optimize if needed):
```java
public DrillFuncHolder findDrillFunction(FunctionResolver functionResolver,
FunctionCall functionCall) {
AtomicLong version = new AtomicLong();
String newFunctionName = functionReplacement(functionCall);
List<DrillFuncHolder> functions =
localFunctionRegistry.getMethods(newFunctionName, version);
if (!useDynamicUdfs) {
return functionResolver.getBestMatch(functions, functionCall);
}
FunctionResolver exactResolver =
FunctionResolverFactory.getExactResolver(functionCall);
DrillFuncHolder holder = exactResolver.getBestMatch(functions,
functionCall);
if (holder == null) {
syncWithRemoteRegistry(version.get());
List<DrillFuncHolder> updatedFunctions =
localFunctionRegistry.getMethods(newFunctionName, version);
holder = functionResolver.getBestMatch(updatedFunctions,
functionCall);
}
return holder;
}
```
3. Also changes should be done in `findExactMatchingDrillFunction` method
to take into account boostrap option as well. For example (please optimize if
needed):
```java
public DrillFuncHolder findExactMatchingDrillFunction(String name,
List<MajorType> argTypes, MajorType returnType) {
if (useDynamicUdfs) {
return findExactMatchingDrillFunction(name, argTypes, returnType,
true);
}
return findExactMatchingDrillFunction(name, argTypes, returnType,
false);
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
