Drop Usage of ensureGil from PythonDeephavenSession and Copy Current Scope

Integrations/src/main/java/io/deephaven/integrations/python/PythonDeephavenSession.java

@@ -45,7 +45,7 @@
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.function.Consumer;
-import java.util.function.Predicate;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 
 /**
@@ -254,8 +254,12 @@ protected Changes createDiff(PythonSnapshot from, PythonSnapshot to, RuntimeExce
     }
 
     @Override
-    protected Set<String> getVariableNames(Predicate<String> allowName) {
-        return PyLib.ensureGil(() -> scope.getKeys().filter(allowName).collect(Collectors.toUnmodifiableSet()));
+    protected Set<String> getVariableNames() {
+        try (final PyDictWrapper currScope = scope.currentScope().copy()) {
+            return currScope.keySet().stream()
+                    .map(scope::convertStringKey)
+                    .collect(Collectors.toSet());
+        }
     }
 
     @Override
@@ -299,19 +303,24 @@ protected synchronized Object setVariable(String name, @Nullable Object newValue
     }
 
     @Override
-    protected Map<String, Object> getAllValues(@NotNull final Predicate<Map.Entry<String, Object>> predicate) {
-        final HashMap<String, Object> result = PyLib.ensureGil(
-                () -> scope.getEntriesRaw().<Map.Entry<String, PyObject>>map(
-                        e -> new AbstractMap.SimpleImmutableEntry<>(scope.convertStringKey(e.getKey()), e.getValue()))
-                        .collect(HashMap::new, (map, entry) -> map.put(entry.getKey(), entry.getValue()),
-                                HashMap::putAll));
-
-        final Iterator<Map.Entry<String, Object>> iter = result.entrySet().iterator();
-        while (iter.hasNext()) {
-            final Map.Entry<String, Object> entry = iter.next();
-            entry.setValue(scope.convertValue((PyObject) entry.getValue()));
-            if (!predicate.test(entry)) {
-                iter.remove();
+    protected <T> Map<String, T> getAllValues(
+            @Nullable final Function<Object, T> valueMapper,
+            @NotNull final QueryScope.ParamFilter<T> filter) {
+        final Map<String, T> result = new HashMap<>();
+
+        try (final PyDictWrapper currScope = scope.currentScope().copy()) {
+            for (final Map.Entry<PyObject, PyObject> entry : currScope.entrySet()) {
+                final String name = scope.convertStringKey(entry.getKey());
+                Object value = scope.convertValue(entry.getValue());
+                if (valueMapper != null) {
+                    value = valueMapper.apply(value);
+                }
+
+                // noinspection unchecked
+                if (filter.accept(name, (T) value)) {
+                    // noinspection unchecked
+                    result.put(name, (T) value);
+                }
             }
         }
 

engine/context/src/main/java/io/deephaven/engine/context/EmptyQueryScope.java

@@ -7,10 +7,8 @@
 import org.jetbrains.annotations.NotNull;
 
 import java.lang.ref.WeakReference;
-import java.util.Collections;
-import java.util.Map;
-import java.util.Set;
-import java.util.function.Predicate;
+import java.util.*;
+import java.util.function.Function;
 import java.util.stream.Stream;
 
 public class EmptyQueryScope implements QueryScope {
@@ -20,7 +18,7 @@ private EmptyQueryScope() {}
 
     @Override
     public Set<String> getParamNames() {
-        return Collections.emptySet();
+        return new HashSet<>();
     }
 
     @Override
@@ -49,8 +47,13 @@ public <T> void putParam(String name, T value) {
     }
 
     @Override
-    public Map<String, Object> toMap(@NotNull Predicate<Map.Entry<String, Object>> predicate) {
-        return Collections.emptyMap();
+    public Map<String, Object> toMap(@NotNull ParamFilter<Object> filter) {
+        return new HashMap<>();
+    }
+
+    @Override
+    public <T> Map<String, T> toMap(@NotNull Function<Object, T> valueMapper, @NotNull ParamFilter<T> filter) {
+        return new HashMap<>();
     }
 
     @Override

engine/context/src/main/java/io/deephaven/engine/context/PoisonedQueryScope.java

@@ -10,7 +10,7 @@
 import java.lang.ref.WeakReference;
 import java.util.Map;
 import java.util.Set;
-import java.util.function.Predicate;
+import java.util.function.Function;
 import java.util.stream.Stream;
 
 public class PoisonedQueryScope implements QueryScope {
@@ -54,7 +54,12 @@ public <T> void putParam(String name, T value) {
     }
 
     @Override
-    public Map<String, Object> toMap(@NotNull Predicate<Map.Entry<String, Object>> predicate) {
+    public Map<String, Object> toMap(@NotNull ParamFilter<Object> filter) {
+        return fail();
+    }
+
+    @Override
+    public <T> Map<String, T> toMap(@NotNull Function<Object, T> valueMapper, @NotNull ParamFilter<T> filter) {
         return fail();
     }
 

engine/context/src/main/java/io/deephaven/engine/context/QueryScope.java

@@ -6,15 +6,14 @@
 import io.deephaven.engine.liveness.LivenessNode;
 import io.deephaven.base.log.LogOutput;
 import io.deephaven.base.log.LogOutputAppendable;
-import io.deephaven.util.annotations.FinalDefault;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
 import java.util.Collection;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
-import java.util.function.Predicate;
+import java.util.function.Function;
 
 /**
  * Variable scope used to resolve parameter values during query execution and to expose named objects to users. Objects
@@ -84,7 +83,7 @@ default QueryScopeParam<?>[] getParams(final Collection<String> names) throws Mi
     /**
      * Get all known scope variable names.
      *
-     * @return A collection of scope variable names.
+     * @return A caller-owned mutable collection of scope variable names.
      */
     Set<String> getParamNames();
 
@@ -133,25 +132,33 @@ default QueryScopeParam<?>[] getParams(final Collection<String> names) throws Mi
      */
     <T> void putParam(final String name, final T value);
 
+    @FunctionalInterface
+    interface ParamFilter<T> {
+        boolean accept(String name, T value);
+    }
+
     /**
      * Returns a mutable map with all objects in the scope. Callers may want to unwrap language-specific values using
-     * {@link #unwrapObject(Object)} before using them. This map is owned by the caller and may be mutated.
+     * {@link #unwrapObject(Object)} before using them. This returned map is owned by the caller.
      *
-     * @param predicate a predicate to filter the map entries
+     * @param filter a predicate to filter the map entries
      * @return a caller-owned mutable map with all known variables and their values.
      */
-    Map<String, Object> toMap(@NotNull Predicate<Map.Entry<String, Object>> predicate);
+    Map<String, Object> toMap(@NotNull ParamFilter<Object> filter);
 
     /**
-     * Returns a mutable map with all objects in the scope. Callers may want to unwrap language-specific values using
-     * {@link #unwrapObject(Object)} before using them. This map is owned by the caller and may be mutated.
+     * Returns a mutable map with all objects in the scope.
+     * <p>
+     * Callers may want to pass in a valueMapper of {@link #unwrapObject(Object)} which would unwrap values before
+     * filtering. The returned map is owned by the caller.
      *
+     * @param valueMapper a function to map the values
+     * @param filter a predicate to filter the map entries
      * @return a caller-owned mutable map with all known variables and their values.
+     * @param <T> the type of the mapped values
      */
-    @FinalDefault
-    default Map<String, Object> toMap() {
-        return toMap(entry -> true);
-    }
+    <T> Map<String, T> toMap(
+            @NotNull Function<Object, T> valueMapper, @NotNull ParamFilter<T> filter);
 
     /**
      * Removes any wrapping that exists on a scope param object so that clients can fetch them. Defaults to returning
@@ -167,7 +174,7 @@ default Object unwrapObject(@Nullable Object object) {
     @Override
     default LogOutput append(@NotNull final LogOutput logOutput) {
         logOutput.append('{');
-        for (final Map.Entry<String, Object> param : toMap().entrySet()) {
+        for (final Map.Entry<String, Object> param : toMap((name, value) -> true).entrySet()) {
             logOutput.nl().append(param.getKey()).append("=");
             Object paramValue = param.getValue();
             if (paramValue == this) {

engine/context/src/main/java/io/deephaven/engine/context/StandaloneQueryScope.java

@@ -1,30 +1,30 @@
 package io.deephaven.engine.context;
 
-import io.deephaven.api.util.NameValidator;
 import io.deephaven.engine.liveness.LivenessArtifact;
 import io.deephaven.engine.liveness.LivenessReferent;
 import io.deephaven.engine.updategraph.DynamicNode;
 import io.deephaven.hash.KeyedObjectHashMap;
 import io.deephaven.hash.KeyedObjectKey;
-import org.apache.commons.lang3.tuple.ImmutablePair;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.Set;
-import java.util.function.Predicate;
+import java.util.function.Function;
 
 /**
  * Map-based implementation, extending LivenessArtifact to manage the objects passed into it.
  */
 public class StandaloneQueryScope extends LivenessArtifact implements QueryScope {
 
-    private final KeyedObjectHashMap<String, ValueRetriever<?>> valueRetrievers =
+    private final Map<String, ValueRetriever<?>> valueRetrievers =
             new KeyedObjectHashMap<>(new ValueRetrieverNameKey());
 
     @Override
     public Set<String> getParamNames() {
-        return valueRetrievers.keySet();
+        return new HashSet<>(valueRetrievers.keySet());
     }
 
     @Override
@@ -64,7 +64,6 @@ public <T> T readParamValue(final String name, final T defaultValue) {
 
     @Override
     public <T> void putParam(final String name, final T value) {
-        NameValidator.validateQueryParameterName(name);
         if (value instanceof LivenessReferent && DynamicNode.notDynamicOrIsRefreshing(value)) {
             manage((LivenessReferent) value);
         }
@@ -80,12 +79,37 @@ public <T> void putParam(final String name, final T value) {
     }
 
     @Override
-    public Map<String, Object> toMap(@NotNull final Predicate<Map.Entry<String, Object>> predicate) {
-        final HashMap<String, Object> result = new HashMap<>();
-        valueRetrievers.entrySet().stream()
-                .map(e -> ImmutablePair.of(e.getKey(), (Object) e.getValue().value))
-                .filter(predicate)
-                .forEach(e -> result.put(e.getKey(), e.getValue()));
+    public Map<String, Object> toMap(@NotNull final ParamFilter<Object> filter) {
+        return toMapInternal(null, filter);
+    }
+
+    @Override
+    public <T> Map<String, T> toMap(
+            @NotNull final Function<Object, T> valueMapper,
+            @NotNull final ParamFilter<T> filter) {
+        return toMapInternal(valueMapper, filter);
+    }
+
+    private <T> Map<String, T> toMapInternal(
+            @Nullable final Function<Object, T> valueMapper,
+            @NotNull final ParamFilter<T> filter) {
+        final Map<String, T> result = new HashMap<>();
+
+        for (final Map.Entry<String, ValueRetriever<?>> entry : valueRetrievers.entrySet()) {
+            final String name = entry.getKey();
+            final ValueRetriever<?> valueRetriever = entry.getValue();
+            Object value = valueRetriever.getValue();
+            if (valueMapper != null) {
+                value = valueMapper.apply(value);
+            }
+
+            // noinspection unchecked
+            if (filter.accept(name, (T) value)) {
+                // noinspection unchecked
+                result.put(name, (T) value);
+            }
+        }
+
         return result;
     }
 

engine/sql/src/main/java/io/deephaven/engine/sql/Sql.java

@@ -82,16 +82,9 @@ private static Scope scope(Map<String, Table> scope, Map<TicketTable, Table> out
     private static Map<String, Table> currentScriptSessionNamedTables() {
         // getVariables() is inefficient
         // See SQLTODO(catalog-reader-implementation)
-        QueryScope queryScope = ExecutionContext.getContext().getQueryScope();
-        final Map<String, Table> scope = queryScope
-                .toMap()
-                .entrySet()
-                .stream()
-                .map(e -> Map.entry(e.getKey(), queryScope.unwrapObject(e.getValue())))
-                .filter(e -> e.getValue() instanceof Table)
-                .collect(Collectors.toMap(Entry::getKey, e -> (Table) e.getValue()));
-
-        return scope;
+        final QueryScope queryScope = ExecutionContext.getContext().getQueryScope();
+        // noinspection unchecked,rawtypes
+        return (Map<String, Table>) (Map) queryScope.toMap(queryScope::unwrapObject, (n, t) -> t instanceof Table);
     }
 
     private static TableHeader adapt(TableDefinition tableDef) {

engine/table/src/main/java/io/deephaven/engine/table/impl/select/AbstractConditionFilter.java

@@ -95,7 +95,7 @@ public synchronized void init(TableDefinition tableDefinition) {
         try {
             final QueryScope queryScope = ExecutionContext.getContext().getQueryScope();
             final Map<String, Object> queryScopeVariables = queryScope.toMap(
-                    NameValidator.VALID_QUERY_PARAMETER_MAP_ENTRY_PREDICATE);
+                    (name, value) -> NameValidator.isValidQueryParameterName(name));
             for (Map.Entry<String, Object> param : queryScopeVariables.entrySet()) {
                 possibleVariables.put(param.getKey(), QueryScopeParamTypeUtil.getDeclaredClass(param.getValue()));
                 Type declaredType = QueryScopeParamTypeUtil.getDeclaredType(param.getValue());

engine/table/src/main/java/io/deephaven/engine/table/impl/select/codegen/FormulaAnalyzer.java

@@ -105,7 +105,7 @@ public static QueryLanguageParser.Result getCompiledFormula(Map<String, ColumnDe
 
         final ExecutionContext context = ExecutionContext.getContext();
         final Map<String, Object> queryScopeVariables = context.getQueryScope().toMap(
-                NameValidator.VALID_QUERY_PARAMETER_MAP_ENTRY_PREDICATE);
+                (name, value) -> NameValidator.isValidQueryParameterName(name));
         for (Map.Entry<String, Object> param : queryScopeVariables.entrySet()) {
             if (possibleVariables.containsKey(param.getKey())) {
                 // skip any existing matches