Reject MSE queries with a large number of elements in an IN clause

[yashmayya]

• Even after Multi-stage: Perf issue when IN expression has a lot of entries #13617 was "resolved" through Improve performance of multi-stage queries with large IN clauses #14615 and Upgrade Calcite to 1.39.0 #15263, we still see some issues when the number of elements in an IN clause is excessively large (in the range of 1000s of elements or more).
• The symptoms are the same - broker CPU usage becomes really high because the MSE query planner thread planning such queries enters into a suboptimal algorithm in the Calcite planner and thread interruption on timeout isn't respected by Calcite either. This causes other regular queries to timeout as well.
• This patch adds a validator to reject MSE queries with a large number of elements in an IN clause (configurable through broker config / query option) in order to protect the brokers.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 73.52941% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.27%. Comparing base (41898e0) to head (9b17bc0).

Files with missing lines	Patch %	Lines
...e/pinot/common/utils/config/QueryOptionsUtils.java	33.33%	3 Missing and 1 partial ⚠️
.../query/validate/InClauseSizeValidationVisitor.java	85.71%	1 Missing and 2 partials ⚠️
...requesthandler/MultiStageBrokerRequestHandler.java	0.00%	2 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #17481      +/-   ##
============================================
- Coverage     63.27%   63.27%   -0.01%     
+ Complexity     1477     1476       -1     
============================================
  Files          3167     3168       +1     
  Lines        189062   189096      +34     
  Branches      28930    28936       +6     
============================================
+ Hits         119624   119642      +18     
- Misses        60153    60169      +16     
  Partials       9285     9285              

Flag	Coverage Δ
custom-integration1	100.00% <ø> (ø)
integration	100.00% <ø> (ø)
integration1	100.00% <ø> (ø)
integration2	0.00% <ø> (ø)
java-11	63.23% <73.52%> (-0.01%)	⬇️
java-21	63.22% <73.52%> (+0.01%)	⬆️
temurin	63.27% <73.52%> (-0.01%)	⬇️
unittests	63.26% <73.52%> (-0.01%)	⬇️
unittests1	55.56% <78.12%> (+<0.01%)	⬆️
unittests2	34.05% <32.35%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Jackie-Jiang]

This will introduce backward incompatibility for existing queries right?

[yashmayya]

Yeah, that's a fair point. Do you think we should introduce this knob but keep it off by default so that users can tune it as per their workload (if and when they run into broker issues with such queries)?

[Jackie-Jiang]

Should we keep it as deprecated? Seems like it is used only in test, and not for production usage

[yashmayya]

I've retained the VisibleForTesting annotation, why do we need to keep it as deprecated too? The suggestion to Use [#compile] and then [plan][CompiledQuery#planQuery(long)] the returned query instead also doesn't make much sense, because that's exactly what this method is doing, so I don't see why it needs to be deprecated when it's just a convenient shortcut.