dssysolyatin commented on code in PR #2977:
URL: https://github.com/apache/calcite/pull/2977#discussion_r1038206955


##########
core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableWindow.java:
##########
@@ -693,7 +696,7 @@ private static Pair<Expression, Expression> 
getPartitionIterator(
     }
     Expression multiMap_ =
         builder.append(
-            "multiMap", Expressions.new_(SortedMultiMap.class));
+            "multiMap", Expressions.new_(SortedMultiMap.class), false);

Review Comment:
   @libenchao @rubenada As I wrote above "multiMap" case works without this 
fix. 
   But it works only because expressions which create and fill multiMap are 
different. 
   I set optimize flag = false for "multiMap" only for safety, because it is 
really easy to make a mistake here. 
   I am not against optimizations, but in my opinion, this optimization does 
more harm than good. 
   
   ```
   Pseudo code for "multiMap" case
   <-- first EnumerableWindow -->
   multiMap = new SortedMultiMap() <-- luckely optimizer caches only this 
expression
   fillMultimap(multiMap)
   multiMap.clear()
   
   <-- second EnumerableWindow -->
   fillMap(multiMap)
   multiMap.clear()
   ```



-- 
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: commits-unsubscr...@calcite.apache.org

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

Reply via email to