Ma77Ball commented on code in PR #57346:
URL: https://github.com/apache/spark/pull/57346#discussion_r3619324802


##########
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##########
@@ -4465,6 +4465,15 @@ object SQLConf {
       .booleanConf
       .createWithDefault(false)
 
+  val WINDOW_MONOTONIC_DEQUE_ENABLED =
+    buildConf("spark.sql.window.monotonicDeque.enabled")
+      .withBindingPolicy(ConfigBindingPolicy.NOT_APPLICABLE)
+      .doc("Use monotonic deques for sliding min/max aggregate window frames. 
" +
+        "This provides O(N) complexity instead of O(N * W) or O(N log W).")
+      .version("5.0.0")
+      .booleanConf
+      .createWithDefault(true)

Review Comment:
   This new optimization defaults to on, but the sibling 
`spark.sql.window.segmentTree.enabled` (same area, added in 4.2.0) defaults to 
off. Turning a brand-new execution path on by default for all sliding MIN/MAX 
window queries on its first release is risky; consider defaulting to false to 
match and let it bake before flipping on. Fix:
   
   ```suggestion
         .createWithDefault(false)
   ```



##########
sql/core/src/main/scala/org/apache/spark/sql/execution/window/WindowEvaluatorFactoryBase.scala:
##########
@@ -281,7 +283,19 @@ trait WindowEvaluatorFactoryBase {
 
           // Shrinking Frame.
           case ("AGGREGATE", frameType, lower, UnboundedFollowing, _) =>
-            if (eligibleForSegTree(functions, aggFilters, frameType, conf)) {
+            val isMinMaxOnly = conf.windowMonotonicDequeEnabled &&
+              functions.nonEmpty && functions.forall {
+                case _: Min | _: Max => true
+                case _ => false
+              } && aggFilters.forall(_.isEmpty)

Review Comment:
   The `isMinMaxOnly` guard is duplicated in the Moving Frame case below. Might 
be better to add it to a small local helper so the two copies don't drift. 



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


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

Reply via email to