Project

General

Profile

Bug #8952

Maximum function call inside a dynamic query is broken

Added by Artur Școlnic about 2 years ago. Updated about 2 years ago.

Status:
WIP
Priority:
Normal
Target version:
-
Start date:
Due date:
% Done:

0%

billable:
No
vendor_id:
GCD
case_num:
version_reported:
version_resolved:
reviewer:
production:
No
env_name:
topics:

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

Investigating the issue led to several findings:
  • The function call does not to be dynamic, a static call of the function maximum inside 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 maximum and minimum, but not when using functions like abs or sqrt.
  • 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 null for 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 null for 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  &lt;code&gt;true&lt;/code&gt; if the signatures match as specified above,
     *          else &lt;code&gt;false&lt;/code&gt;.
     */
-   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 &gt;= 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 &gt;= len1 && len1 &gt; 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 &lt; 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 &lt; 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 in DynamicOps.

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.UnknownType public. It is needed by Function.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
+    * &lt;code&gt;match&lt;/code&gt; is compatible with &lt;code&gt;sig&lt;/code&gt;. They match if they contain the same number of
+    * minimum required parameters and for each index &lt;code&gt;i&lt;/code&gt;, either:
+    * &lt;ul&gt;
+    *    &lt;li&gt;&lt;code&gt;match[i]&lt;/code&gt; is compatible with &lt;code&gt;sig[i]&lt;/code&gt;; or
+    *    &lt;li&gt;&lt;code&gt;match[i] == null&lt;/code&gt;.
+    * &lt;/ul&gt;
+    * Types are tested for assignment compatibility rather than equality. Assignment compatibility may be
+    * &lt;em&gt;strict&lt;/em&gt; or &lt;em&gt;lenient&lt;/em&gt;:
+    * &lt;ul&gt;
+    *    &lt;li&gt;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., &lt;code&gt;int&lt;/code&gt; and &lt;code&gt;java.lang.Integer&lt;/code&gt;).
+    *    &lt;li&gt;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.
+    * &lt;/ul&gt;
+    *
+    * @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;
+    *          &lt;code&gt;null&lt;/code&gt;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  &lt;code&gt;true&lt;/code&gt; if the signatures match as specified above,
+    *          else &lt;code&gt;false&lt;/code&gt;.
+    */
+   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 &gt;= 0)
+      {
+         len1--;
+         
+         // Check for too few parameters.
+         if (len2 &lt; minArgs)
+         {
+            return false;
+         }
+      }
+      else if (len1 != len2)
+      {
+         if (len2 &gt;= len1 && len1 &gt; 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 &lt; 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 &lt; 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 &lt; 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

Also available in: Atom PDF