msokolov commented on code in PR #16423:
URL: https://github.com/apache/lucene/pull/16423#discussion_r3657330799
##########
lucene/expressions/src/java/org/apache/lucene/expressions/js/JavascriptCompiler.java:
##########
@@ -155,10 +162,35 @@ public static Expression compile(String sourceText)
throws ParseException {
* @param functions map of String names to {@link MethodHandle}s
* @return A new compiled expression
* @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have
correct signature
+ * @throws IllegalStateException if the resulting expression class fails to
link (e.g.,
+ * complexity)
*/
public static Expression compile(String sourceText, Map<String,
MethodHandle> functions)
throws ParseException {
- return compile(sourceText, functions, false);
+ return compile(sourceText, functions, DEFAULT_MAX_NESTING_DEPTH);
+ }
+
+ /**
+ * Compiles the given expression with the supplied custom functions using a
custom maximum nesting
+ * depth.
+ *
+ * <p>Functions must be {@code public static}, return {@code double} and can
take from zero to 256
+ * {@code double} parameters.
+ *
+ * @param sourceText The expression to compile
+ * @param functions map of String names to {@link MethodHandle}s
+ * @param maxNestingDepth the maximum depth of nesting (function calls,
precedence)
+ * @return A new compiled expression
+ * @throws ParseException on failure to compile
+ * @throws IllegalArgumentException if any of the functions does not have
correct signature
+ * @throws IllegalStateException if the resulting expression class fails to
link (e.g.,
+ * complexity)
+ */
+ public static Expression compile(
+ String sourceText, Map<String, MethodHandle> functions, int
maxNestingDepth)
Review Comment:
hm, I did not realize a single expression with lots of + would be included.
You know at one time we attempted to use Lucene's expression language to encode
machine-learned decision trees that had thousands and thousands of nodes.
Ultimately we concluded this was bad idea and implemented a custom handling
function,, but the the reason was not StackOverflow, but some internal limit in
the JDK on byte code. I'm forgetting exactly what was limited - maybe the total
size of byte code, number of instructions, or number of identifiers? Anyway
there can be cases with very large expressions, so I get your point that 250
may not be enough. Still, this feels something like BooleanQuery's 1024 limit
or vector search's 1024-dimension limit - can we pick a large default (maybe
1024 is our magic number)? But I don't fundamentally object to exposing the
config parameter, just wonder if anyone would use it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
