diff --git a/processing/src/main/java/org/apache/druid/query/filter/JavaScriptDimFilter.java b/processing/src/main/java/org/apache/druid/query/filter/JavaScriptDimFilter.java index 62a379a36a4..954ea5d2fa1 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/JavaScriptDimFilter.java +++ b/processing/src/main/java/org/apache/druid/query/filter/JavaScriptDimFilter.java @@ -36,6 +36,7 @@ import org.checkerframework.checker.nullness.qual.EnsuresNonNull; import org.checkerframework.checker.nullness.qual.MonotonicNonNull; import org.mozilla.javascript.Context; import org.mozilla.javascript.Function; +import org.mozilla.javascript.ScriptRuntime; import org.mozilla.javascript.ScriptableObject; import javax.annotation.Nullable; @@ -155,7 +156,8 @@ public class JavaScriptDimFilter extends AbstractOptimizableDimFilter implements * script compilation. */ @EnsuresNonNull("predicateFactory") - private JavaScriptPredicateFactory getPredicateFactory() + @VisibleForTesting + JavaScriptPredicateFactory getPredicateFactory() { // JavaScript configuration should be checked when it's actually used because someone might still want Druid // nodes to be able to deserialize JavaScript-based objects even though JavaScript is disabled. @@ -327,7 +329,14 @@ public class JavaScriptDimFilter extends AbstractOptimizableDimFilter implements if (extractionFn != null) { input = extractionFn.apply(input); } - return Context.toBoolean(fnApply.call(cx, scope, scope, new Object[]{input})); + Object fnResult = fnApply.call(cx, scope, scope, new Object[]{input}); + if (fnResult instanceof ScriptableObject) { + // Direct return js function result (like arr.includes) will return org.mozilla.javascript.NativeBoolean, + // Context.toBoolean always treat it as true, even if it is false. Convert it to java.lang.Boolean first to fix this mistake. + return Context.toBoolean(((ScriptableObject) fnResult).getDefaultValue(ScriptRuntime.BooleanClass)); + } else { + return Context.toBoolean(fnResult); + } } @Override diff --git a/processing/src/test/java/org/apache/druid/query/filter/JavaScriptDimFilterTest.java b/processing/src/test/java/org/apache/druid/query/filter/JavaScriptDimFilterTest.java index b9f29936136..0bf34328d9a 100644 --- a/processing/src/test/java/org/apache/druid/query/filter/JavaScriptDimFilterTest.java +++ b/processing/src/test/java/org/apache/druid/query/filter/JavaScriptDimFilterTest.java @@ -113,4 +113,48 @@ public class JavaScriptDimFilterTest JavaScriptDimFilter javaScriptDimFilter = new JavaScriptDimFilter("dim", FN1, null, new JavaScriptConfig(false)); Assert.assertEquals(javaScriptDimFilter.getRequiredColumns(), Sets.newHashSet("dim")); } + + @Test + public void testPredicateFactoryApplyObject() + { + // test for return org.mozilla.javascript.NativeBoolean + JavaScriptDimFilter javaScriptDimFilter = new JavaScriptDimFilter( + "dim", + "function(id) { return ['123', '456'].includes(id); }", + null, + JavaScriptConfig.getEnabledInstance() + ); + Assert.assertTrue(javaScriptDimFilter.getPredicateFactory().applyObject("123")); + Assert.assertTrue(javaScriptDimFilter.getPredicateFactory().applyObject("456")); + Assert.assertFalse(javaScriptDimFilter.getPredicateFactory().applyObject("789")); + + // test for return java.lang.Boolean + JavaScriptDimFilter javaScriptDimFilter1 = new JavaScriptDimFilter( + "dim", + "function(id) { return ['123', '456'].includes(id) == true; }", + null, + JavaScriptConfig.getEnabledInstance() + ); + Assert.assertTrue(javaScriptDimFilter1.getPredicateFactory().applyObject("123")); + Assert.assertTrue(javaScriptDimFilter1.getPredicateFactory().applyObject("456")); + Assert.assertFalse(javaScriptDimFilter1.getPredicateFactory().applyObject("789")); + + // test for return other type + JavaScriptDimFilter javaScriptDimFilter2 = new JavaScriptDimFilter( + "dim", + "function(id) { return 'word'; }", + null, + JavaScriptConfig.getEnabledInstance() + ); + Assert.assertTrue(javaScriptDimFilter2.getPredicateFactory().applyObject("123")); + + // test for return null + JavaScriptDimFilter javaScriptDimFilter3 = new JavaScriptDimFilter( + "dim", + "function(id) { return null; }", + null, + JavaScriptConfig.getEnabledInstance() + ); + Assert.assertFalse(javaScriptDimFilter3.getPredicateFactory().applyObject("123")); + } }