Bug #8952
Maximum function call inside a dynamic query is broken
0%
History
#1 Updated by Artur Școlnic about 2 years ago
define temp-table tt
field number as integer.
create tt.
assign tt.number = 1.
FUNCTION foo RETURNS INTEGER (INPUT nr AS INTEGER):
RETURN (2 * nr).
END FUNCTION.
DEFINE VARIABLE qh AS HANDLE.
CREATE QUERY qh.
qh:SET-BUFFERS(BUFFER tt:HANDLE).
qh:QUERY-PREPARE("
for each tt where dynamic-function('foo',maximum(1, 2, 3, 4)) = true
").
qh:QUERY-OPEN.
Converting and running test cases like this one results in
java.lang.NullPointerException.#2 Updated by Artur Școlnic about 2 years ago
- Status changed from New to WIP
- Assignee set to Artur Școlnic
The issue is derived from #8912-86.
#3 Updated by Artur Școlnic about 2 years ago
The stack trace is
at com.goldencode.p2j.persist.RuntimeJastInterpreter.fixupParameters(RuntimeJastInterpreter.java:1058)
at com.goldencode.p2j.persist.RuntimeJastInterpreter.callStaticMethod(RuntimeJastInterpreter.java:1282)
at com.goldencode.p2j.persist.RuntimeJastInterpreter.evalExpression(RuntimeJastInterpreter.java:1489)
at com.goldencode.p2j.persist.RuntimeJastInterpreter.collectParameters(RuntimeJastInterpreter.java:864)
at com.goldencode.p2j.persist.RuntimeJastInterpreter.callStaticMethod(RuntimeJastInterpreter.java:1190)
at com.goldencode.p2j.persist.RuntimeJastInterpreter.evalExpression(RuntimeJastInterpreter.java:1489)
at com.goldencode.p2j.persist.RuntimeJastInterpreter.collectParameters(RuntimeJastInterpreter.java:864)
at com.goldencode.p2j.persist.RuntimeJastInterpreter.callCtor(RuntimeJastInterpreter.java:789)
at com.goldencode.p2j.persist.RuntimeJastInterpreter.evalExpression(RuntimeJastInterpreter.java:1322)
at com.goldencode.p2j.persist.RuntimeJastInterpreter.collectParameters(RuntimeJastInterpreter.java:864)
at com.goldencode.p2j.persist.RuntimeJastInterpreter.callStaticMethod(RuntimeJastInterpreter.java:1190)
at com.goldencode.p2j.persist.RuntimeJastInterpreter.evalExpression(RuntimeJastInterpreter.java:1489)
at com.goldencode.p2j.persist.RuntimeJastInterpreter.access$200(RuntimeJastInterpreter.java:173)
at com.goldencode.p2j.persist.RuntimeJastInterpreter$P2JQueryParameter.resolve(RuntimeJastInterpreter.java:2722)
at com.goldencode.p2j.persist.AbstractQuery.lambda$preprocessSubstitutionArguments$1(AbstractQuery.java:561)
at com.goldencode.p2j.util.ErrorManager.silentWorker(ErrorManager.java:4148)
at com.goldencode.p2j.util.ErrorManager.access$2500(ErrorManager.java:415)
at com.goldencode.p2j.util.ErrorManager$ErrorHelper.silent(ErrorManager.java:4463)
at com.goldencode.p2j.persist.AbstractQuery.preprocessSubstitutionArguments(AbstractQuery.java:574)
at com.goldencode.p2j.persist.QueryComponent.<init>(QueryComponent.java:237)
at com.goldencode.p2j.persist.AdaptiveComponent.<init>(AdaptiveComponent.java:199)
at com.goldencode.p2j.persist.AdaptiveQuery.addComponent(AdaptiveQuery.java:1143)
at com.goldencode.p2j.persist.AdaptiveQuery.initialize(AdaptiveQuery.java:722)
at com.goldencode.p2j.persist.AdaptiveQuery.initialize(AdaptiveQuery.java:513)
at com.goldencode.p2j.persist.PreselectQuery.initialize(PreselectQuery.java:1084)
at com.goldencode.p2j.persist.PreselectQueryMethodAccess.invoke(Unknown Source)
at com.goldencode.p2j.util.Utils.invoke(Utils.java:1696)
at com.goldencode.p2j.persist.RuntimeJastInterpreter.callMethod(RuntimeJastInterpreter.java:1008)
at com.goldencode.p2j.persist.RuntimeJastInterpreter.execMethod(RuntimeJastInterpreter.java:672)
at com.goldencode.p2j.persist.RuntimeJastInterpreter.lambda$interpret$0(RuntimeJastInterpreter.java:431)
at com.goldencode.p2j.jmx.NanoTimer.timer(NanoTimer.java:131)
at com.goldencode.p2j.persist.RuntimeJastInterpreter.interpret(RuntimeJastInterpreter.java:431)
at com.goldencode.p2j.persist.DynamicQueryHelper$2.onQueryOpen(DynamicQueryHelper.java:666)
at com.goldencode.p2j.persist.AbstractQuery.notifyQueryOpenListeners(AbstractQuery.java:1039)
at com.goldencode.p2j.persist.QueryWrapper.queryOpen(QueryWrapper.java:4519)
at com.goldencode.dataset.Start.lambda$execute$0(Start.java:39)
at com.goldencode.p2j.util.Block.body(Block.java:636)
at com.goldencode.p2j.util.BlockManager.processBody(BlockManager.java:9252)
at com.goldencode.p2j.util.BlockManager.topLevelBlock(BlockManager.java:8873)
at com.goldencode.p2j.util.BlockManager.externalProcedure(BlockManager.java:651)
at com.goldencode.p2j.util.BlockManager.externalProcedure(BlockManager.java:624)
at com.goldencode.dataset.Start.execute(Start.java:31)
at sun.reflect.GeneratedMethodAccessor8.invoke(Unknown Source)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.lang.reflect.Method.invoke(Method.java:498)
at com.goldencode.p2j.util.Utils.invoke(Utils.java:1774)
at com.goldencode.p2j.main.StandardServer$MainInvoker.execute(StandardServer.java:2616)
at com.goldencode.p2j.main.StandardServer.invoke(StandardServer.java:2018)
at com.goldencode.p2j.main.StandardServer.invoke(StandardServer.java:1940)
at com.goldencode.p2j.main.StandardServer.standardEntry(StandardServer.java:672)
at com.goldencode.p2j.main.StandardServerMethodAccess.invoke(Unknown Source)
at com.goldencode.p2j.util.MethodInvoker.invoke(MethodInvoker.java:156)
at com.goldencode.p2j.net.Dispatcher.processInbound(Dispatcher.java:807)
at com.goldencode.p2j.net.Conversation.block(Conversation.java:422)
at com.goldencode.p2j.net.Conversation.run(Conversation.java:235)
at java.lang.Thread.run(Thread.java:750)
#5 Updated by Ovidiu Maxiniuc about 2 years ago
This NPE is cause by lack of support for variable parameter list method in FWD. The list of current arguments is compared with the list of declared parameter of the method. The latter is actual a single array so the method resolution fails, returning null for the matching method.
#6 Updated by Artur Școlnic about 2 years ago
- The function call does not to be dynamic, a static call of the function
maximuminside a dynamic query is enough to reproduce the issue. - The first time after the server startup, the error is not thrown, every following time, it is.
- The issue can be observed when using
maximumandminimum, but not when using functions likeabsorsqrt. - The number of parameters or their type does not affect the behavior.
- The string manipulation functions do run correctly.
So far it seems like the issue is only with maximum and minimum.
#7 Updated by Artur Școlnic about 2 years ago
Ovidiu Maxiniuc wrote:
This NPE is cause by lack of support for variable parameter list method in FWD. The list of current arguments is compared with the list of declared parameter of the method. The latter is actual a single array so the method resolution fails, returning
nullfor the matching method.
I tried using 2 parameters, it still fails, but on the first run it does not.
#8 Updated by Artur Școlnic about 2 years ago
- Subject changed from Dynamic function call inside a dynamic query is broken to Maximum function call inside a dynamic query is broken
#9 Updated by Artur Școlnic about 2 years ago
Ovidiu Maxiniuc wrote:
This NPE is cause by lack of support for variable parameter list method in FWD. The list of current arguments is compared with the list of declared parameter of the method. The latter is actual a single array so the method resolution fails, returning
nullfor the matching method.
I may be wrong, but isn't BaseDataType maximum(BaseDataType... args) exactly for this type of situation? It is in DynamicOps.
#10 Updated by Ovidiu Maxiniuc about 2 years ago
Try with this patch:
--- a/src/com/goldencode/expr/Function.java
+++ b/src/com/goldencode/expr/Function.java (date 1720620668066)
@@ -394,17 +394,13 @@
* @return <code>true</code> if the signatures match as specified above,
* else <code>false</code>.
*/
- public static boolean matchSignature(Class[] sig,
- Class[] match,
- int minArgs,
- boolean strict)
+ public static boolean matchSignature(Class[] sig, Class[] match, int minArgs, boolean strict)
{
int len1 = (sig == null ? 0 : sig.length);
int len2 = (match == null ? 0 : match.length);
- // If supporting var-args, last parameter is the variable argument
- // object array, so we don't want to check the last parameter in the
- // loop below.
+ // if supporting var-args, last parameter is the variable argument object array, so we don't want to
+ // check the last parameter in the loop below.
if (minArgs >= 0)
{
len1--;
@@ -417,8 +413,44 @@
}
else if (len1 != len2)
{
- // For non var-arg functions, a mismatched number of args is right
- // out.
+ if (len2 >= len1 && len1 > 0 && sig[len1 - 1].isArray())
+ {
+ // try to match using vararg protocol: first [len1-1] normally, then expand the len1-th
+ for (int i = 0; i < len1 - 1; i++)
+ {
+ Class next = match[i];
+ if (next == null)
+ {
+ next = UnknownType.class;
+ }
+
+ if (!isAssignmentCompatible(sig[i], next, strict))
+ {
+ return false;
+ }
+ }
+
+ // try to match the rest of the [match] with the base type of the array
+ Class baseType = sig[len1 - 1].getComponentType();
+ for (int i = len1 - 1; i < len2 ;i++)
+ {
+ Class next = match[i];
+ if (next == null)
+ {
+ next = UnknownType.class;
+ }
+
+ if (!isAssignmentCompatible(baseType, next, strict))
+ {
+ return false;
+ }
+ }
+
+ // success we matched all arguments
+ return true;
+ }
+
+ // for non var-arg functions, a mismatched number of args is right out
return false;
}
It should work, but there are some calls from (dynamic, probably static, too) conversion (grafts with var-args) which expect this method to fail LE: return false, although the parameters seem OK. I have not investigated deeper. Ultimately we will add a parameter for dynamic evaluation (preferred) or copy the code to runtime interpreter.
#11 Updated by Ovidiu Maxiniuc about 2 years ago
Artur Școlnic wrote:
I may be wrong, but isn't
BaseDataType maximum(BaseDataType... args)exactly for this type of situation? It is inDynamicOps.
It would be the same. As with int64.maximum, it also use var-args and the old signature-matching algorithm is not working correctly.
I do not know why the conversion calls expects this method to fail. In fact, the existing implementation tries a workaround (see minArgs parameter). But even if when minArgs >= 0, it completely skips the last len2-len1 arguments, causing it to be incomplete beside being incorrect.
#12 Updated by Artur Școlnic about 2 years ago
I tried porting these changes, but the conversion fails. What branch is this from?
#13 Updated by Ovidiu Maxiniuc about 2 years ago
I probably have some additional changes in my working environment. Pasting the big code chunk before return false; at line 422 should do it. As I noted, the conversions (both static and dynamic) fail. Calls to this method expect it to return false, although I checked and, based on the provided signature it's a match:
| signature | class java.lang.String | interface com.goldencode.ast.Aast | long | class [Ljava.lang.Object; |
|---|---|---|---|---|
| arguments | class java.lang.String | null | long | class java.lang.String, class java.lang.String, class java.lang.String, class java.lang.String, class java.lang.String, class java.lang.String |
Probably, the null argument on 2nd position is the cause. One of the request comes from:
ref = tw.graft("class_def", null, javaRootId, "classname", classname, "pkgname", pkgname, "access", "public")and the method is:public com.goldencode.ast.Aast com.goldencode.p2j.pattern.TemplateWorker$Template.graft(java.lang.String,com.goldencode.ast.Aast,long,java.lang.Object[]) throws com.goldencode.ast.AstException#14 Updated by Artur Școlnic about 2 years ago
I am trying to port the matchSignature method to RuntimeJastInterpreter, but UnknownType is package-private.
#15 Updated by Artur Școlnic about 2 years ago
I created a new UnknownType class in persist package. The conversion was successful, but at runtime I get the error
Interpreter exception: Failed to detect the method to be called in [METHOD_CALL: initialize].
#16 Updated by Ovidiu Maxiniuc about 2 years ago
No, make the com.goldencode.expr.UnknownType public. It is needed by Function.isAssignmentCompatible().
Also, please add the missing H section.
#17 Updated by Artur Școlnic about 2 years ago
Ovidiu Maxiniuc wrote:
No, make the
com.goldencode.expr.UnknownTypepublic. It is needed byFunction.isAssignmentCompatible().
Also, please add the missing H section.
I got the same error as in #8952-3. Like I mentioned initially, on the first run, there is no error, is that something worth looking into?
#18 Updated by Ovidiu Maxiniuc about 2 years ago
It works just fine for me with following changes:
--- a/src/com/goldencode/expr/UnknownType.java
+++ b/src/com/goldencode/expr/UnknownType.java (date 1720703613791)
@@ -53,6 +53,6 @@
package com.goldencode.expr;
-final class UnknownType
+public final class UnknownType
{
}
--- a/src/com/goldencode/p2j/persist/RuntimeJastInterpreter.java
+++ b/src/com/goldencode/p2j/persist/RuntimeJastInterpreter.java (date 1720708238294)
@@ -1808,7 +1808,7 @@
for (Constructor<?> ctor : aClass.getDeclaredConstructors())
{
Class<?>[] required = ctor.getParameterTypes();
- if (Function.matchSignature(required, signature, -1, true))
+ if (matchSignature(required, signature, -1, true))
{
if (ret != null)
{
@@ -1917,7 +1917,7 @@
Class<?>[] required = meth.getParameterTypes();
// try strict checking first
- if (Function.matchSignature(required, signature, minargs, true))
+ if (matchSignature(required, signature, minargs, true))
{
strictCheck.add(meth);
ret = meth;
@@ -1951,7 +1951,7 @@
Class<?>[] required = meth.getParameterTypes();
// now try non-strict checking
- if (Function.matchSignature(required, signature, minargs, false))
+ if (matchSignature(required, signature, minargs, false))
{
// TODO: this is not correct we should refine the search the same way java does
// Ex: if the sig is an Integer and there are two methods: Long & Double
@@ -3198,4 +3198,118 @@
super(interpreter, lambdaNode);
}
}
+
+ /**
+ * Compare the specified argument signature against the specified match signature to determine whether
+ * <code>match</code> is compatible with <code>sig</code>. They match if they contain the same number of
+ * minimum required parameters and for each index <code>i</code>, either:
+ * <ul>
+ * <li><code>match[i]</code> is compatible with <code>sig[i]</code>; or
+ * <li><code>match[i] == null</code>.
+ * </ul>
+ * Types are tested for assignment compatibility rather than equality. Assignment compatibility may be
+ * <em>strict</em> or <em>lenient</em>:
+ * <ul>
+ * <li>A "strict" signature match: each required parameter type must be assignable from the provided
+ * parameter type, OR they must be the same primitive types or at least represent a compatible
+ * primitive/wrapper pair (e.g., <code>int</code> and <code>java.lang.Integer</code>).
+ * <li>A "lenient" signature match: each required parameter type must be assignable from the provided
+ * parameter type, OR they must be compatible boolean types (primitive/wrapper combination OK), OR
+ * they must be any combination of numeric primitive and/or wrapper types.
+ * </ul>
+ *
+ * @param sig
+ * Array of argument types which define the baseline signature.
+ * @param match
+ * Array of classes which indicate a parameter signature we are trying to match;
+ * <code>null</code>s are permitted.
+ * @param minArgs
+ * Minimum arguments required by the baseline signature. If negative, it is assumed var-arg lists
+ * are not to be considered.
+ * @param strict
+ * Whether to enforce a strict match or not.
+ *
+ * @return <code>true</code> if the signatures match as specified above,
+ * else <code>false</code>.
+ */
+ public static boolean matchSignature(Class[] sig, Class[] match, int minArgs, boolean strict)
+ {
+ int len1 = (sig == null ? 0 : sig.length);
+ int len2 = (match == null ? 0 : match.length);
+
+ // if supporting var-args, last parameter is the variable argument object array, so we don't want to
+ // check the last parameter in the loop below.
+ if (minArgs >= 0)
+ {
+ len1--;
+
+ // Check for too few parameters.
+ if (len2 < minArgs)
+ {
+ return false;
+ }
+ }
+ else if (len1 != len2)
+ {
+ if (len2 >= len1 && len1 > 0 && sig[len1 - 1].isArray())
+ {
+ // try to match using vararg protocol: first [len1-1] normally, then expand the len1-th
+ for (int i = 0; i < len1 - 1; i++)
+ {
+ Class next = match[i];
+ if (next == null)
+ {
+ next = UnknownType.class;
+ }
+
+ if (!Function.isAssignmentCompatible(sig[i], next, strict))
+ {
+ return false;
+ }
+ }
+
+ // try to match the rest of the [match] with the base type of the array
+ Class baseType = sig[len1 - 1].getComponentType();
+ for (int i = len1 - 1; i < len2 ;i++)
+ {
+ Class next = match[i];
+ if (next == null)
+ {
+ next = UnknownType.class;
+ }
+
+ if (!Function.isAssignmentCompatible(baseType, next, strict))
+ {
+ return false;
+ }
+ }
+
+ // success, we matched all arguments
+ return true;
+ }
+
+ // for non var-arg functions, a mismatched number of args is right out
+ return false;
+ }
+
+ // Compare each parameter in this func with the parameter at the same position in target. A null
+ // parameter in target always matches. Otherwise, parameters must be compatible, as defined by type map.
+ // For functions which support variable argument lists, we must match only the required arguments.
+ // Otherwise, we must match all the arguments.
+ for (int i = 0; i < len1; i++)
+ {
+ Class next = match[i];
+ if (next == null)
+ {
+ next = UnknownType.class;
+ }
+
+ if (!Function.isAssignmentCompatible(sig[i], next, strict))
+ {
+ return false;
+ }
+ }
+
+ return true;
+ }
}
The patch from #8952-10 was reverted, that method is only called from conversion.
This method can be simplified, the minargs is actually not used. I am also not very pleased it is still dependant on Function.isAssignmentCompatible and UnknownType.
#19 Updated by Eric Faulhaber about 2 years ago
- Assignee changed from Artur Școlnic to Ovidiu Maxiniuc