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]