maheshrajus commented on code in PR #5856:
URL: https://github.com/apache/hive/pull/5856#discussion_r2152715258


##########
ql/src/java/org/apache/hadoop/hive/ql/metadata/PartitionTree.java:
##########
@@ -258,21 +254,20 @@ List<Partition> getPartitionsByFilter(final String 
filter) throws MetaException
       return new ArrayList<>(parts.values());
     }
     List<Partition> result = new ArrayList<>();
-    ScriptEngine se = new ScriptEngineManager().getEngineByName("JavaScript");
-    if (se == null) {
-      LOG.error("JavaScript script engine is not found, therefore partition 
filtering "
-          + "for temporary tables is disabled.");
-      return result;
-    }
-    for (Map.Entry<String, Partition> entry : parts.entrySet()) {
-      se.put("partitionName", entry.getKey());
-      se.put("values", entry.getValue().getValues());
-      try {
-        if ((Boolean)se.eval(filter)) {
-          result.add(entry.getValue());
+    try (GraalJSScriptEngine se = GraalJSScriptEngine.create(null,
+            Context.newBuilder().allowExperimentalOptions(true)
+                    .option("js.nashorn-compat", "true")
+                    .allowAllAccess(true))) {

Review Comment:
   @okumin thanks for the review.
   i checked and debug the above code. so here we are setting 
_builder.allowAllAccess(true)_ in the method 
**updateForNashornCompatibilityMode()**. But this method call when 
contextConfig is null. But in our current case we are setting flags related to 
nashorn compatibility("js.nashorn-compat") in Context config and calling method 
GraalJSScriptEngine.create(). So we will not hit the null case(see code below) 
and the method(updateForNashornCompatibilityMode) will not call. So here 
setting "**allowAllAccess(true)**" in Context builder is ok in my opinion. 
Please let me know your opinion on same.  thank you !
   
   ```
           if (contextConfig == null) {      --> in our current case this will 
not be NULL.
               contextConfigToUse = Context.newBuilder(new 
String[]{"js"}).allowExperimentalOptions(true);
               contextConfigToUse.option("js.syntax-extensions", "true");
               contextConfigToUse.option("js.load", "true");
               contextConfigToUse.option("js.print", "true");
               contextConfigToUse.option("js.global-arguments", "true");
               contextConfigToUse.option("js.charset", "UTF-8");
               if (NASHORN_COMPATIBILITY_MODE) {
                   updateForNashornCompatibilityMode(contextConfigToUse);
               } else if 
(Boolean.getBoolean("graaljs.insecure-scriptengine-access")) {
                   updateForScriptEngineAccessibility(contextConfigToUse);
               }
           }
   ```
   ```
       private static void updateForNashornCompatibilityMode(Context.Builder 
builder) {
           builder.allowAllAccess(true);
           builder.allowHostAccess(NASHORN_HOST_ACCESS);
           builder.useSystemExit(true);
       }
   ```
   
   
   
   



-- 
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: gitbox-unsubscr...@hive.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org

Reply via email to