LakshSingla commented on code in PR #16800:
URL: https://github.com/apache/druid/pull/16800#discussion_r1750678153


##########
sql/src/test/java/org/apache/druid/sql/calcite/CalciteWindowQueryTest.java:
##########
@@ -73,6 +73,13 @@ public class CalciteWindowQueryTest extends 
BaseCalciteQueryTest
       QueryContexts.ENABLE_DEBUG, true
   );
 
+  private static final Map<String, Object> 
DEFAULT_QUERY_CONTEXT_WITH_SUBQUERY_BYTES =
+      ImmutableMap.<String, Object>builder()
+                  .putAll(DEFAULT_QUERY_CONTEXT)
+                  .put(QueryContexts.MAX_SUBQUERY_BYTES_KEY, "100000")
+                  .put(QueryContexts.MAX_SUBQUERY_ROWS_KEY, "0")

Review Comment:
   I see now - There isn't a check in the query context that disallows the 
maxSubqueryBytes to be set to 0 using the query context. If it is negative, the 
`ClientQuerySegmentWalker` throws an exception, else it doesn't. So 0 in the 
query context accidentally does what it means, even though the value is 
disallowed on a cluster level. Given that it has been like this for a long 
time, should we keep it as is for this PR (I think negative -> unset is also 
weird, but any change to the semantics here might break queries of the existing 
clusters). 



-- 
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