jon-wei commented on a change in pull request #8019: multi-value string 
expression transformation fix
URL: https://github.com/apache/incubator-druid/pull/8019#discussion_r300196212
 
 

 ##########
 File path: core/src/main/java/org/apache/druid/math/expr/Expr.java
 ##########
 @@ -147,92 +167,157 @@ default String getIdentifierIfIdentifier()
    */
   class BindingDetails
   {
-    private final ImmutableSet<String> freeVariables;
-    private final ImmutableSet<String> scalarVariables;
-    private final ImmutableSet<String> arrayVariables;
+    private final ImmutableSet<IdentifierExpr> freeVariables;
+    private final ImmutableSet<IdentifierExpr> scalarVariables;
+    private final ImmutableSet<IdentifierExpr> arrayVariables;
 
     public BindingDetails()
     {
-      this(Collections.emptySet(), Collections.emptySet(), 
Collections.emptySet());
+      this(ImmutableSet.of(), ImmutableSet.of(), ImmutableSet.of());
     }
 
-    public BindingDetails(String identifier)
+    public BindingDetails(IdentifierExpr expr)
     {
-      this(ImmutableSet.of(identifier), Collections.emptySet(), 
Collections.emptySet());
+      this(ImmutableSet.of(expr), ImmutableSet.of(), ImmutableSet.of());
     }
 
-    public BindingDetails(Set<String> freeVariables, Set<String> 
scalarVariables, Set<String> arrayVariables)
+    public BindingDetails(
+        ImmutableSet<IdentifierExpr> freeVariables,
+        ImmutableSet<IdentifierExpr> scalarVariables,
+        ImmutableSet<IdentifierExpr> arrayVariables
+    )
     {
-      this.freeVariables = ImmutableSet.copyOf(freeVariables);
-      this.scalarVariables = ImmutableSet.copyOf(scalarVariables);
-      this.arrayVariables = ImmutableSet.copyOf(arrayVariables);
+      this.freeVariables = freeVariables;
+      this.scalarVariables = scalarVariables;
+      this.arrayVariables = arrayVariables;
     }
 
     /**
      * Get the list of required column inputs to evaluate an expression
      */
-    public ImmutableList<String> getRequiredColumns()
+    public List<String> getRequiredColumnsList()
     {
-      return ImmutableList.copyOf(freeVariables);
+      return new 
ArrayList<>(freeVariables.stream().map(IdentifierExpr::getIdentifierBindingIfIdentifier).collect(Collectors.toSet()));
+    }
+
+    /**
+     * Get the set of required column inputs to evaluate an expression
+     */
+    public Set<String> getRequiredColumns()
+    {
+      return 
freeVariables.stream().map(IdentifierExpr::getIdentifierBindingIfIdentifier).collect(Collectors.toSet());
+    }
+
+    /**
+     * Set of identifiers which are used with scalar operators and functions
 
 Review comment:
   Suggest adding more docs here about how these differ from the 
`get*Variables()` methods

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to