FrankChen021 commented on issue #20326: URL: https://github.com/apache/druid/issues/20326#issuecomment-5664833172
### Investigation update: regression isolated to the Calcite 1.35 → 1.37 upgrade I reproduced the exact `InPlanningBenchmark.queryStringFunctionInSql` workload introduced by [#16388](https://github.com/apache/druid/pull/16388): ```sql EXPLAIN PLAN FOR SELECT COUNT(*) FROM foo WHERE long1 = 8 OR LOWER(string1) IN ('1', '2', ..., '1000000') ``` Parameters: - `inClauseLiteralsCount=1000000` - `inSubQueryThreshold=2147483647` - `rowsPerSegment=500000` Results across Druid’s Calcite upgrades: | Druid commit | Calcite version | Result | |---|---:|---:| | #16388 merge commit `72432c2e78` | 1.35.0 | **10.39 s/op**, 14.94 GB allocated/op | | `ac19b148c2` from #16504 | 1.37.0 | No completed operation after 20 minutes | | `43ab654f85` | 1.41.0 | No completed operation after 20 minutes | | `35e9e436a7` | 1.42.0 | No completed operation after 20 minutes | “No completed operation” means the benchmark was stopped after 20 minutes; it did not produce a JMH score. #### Regression boundary [#16388](https://github.com/apache/druid/pull/16388) changed large `IN` predicates to use `SCALAR_IN_ARRAY`. At its merge commit, using Calcite 1.35, the one-million-string workload completed in approximately 10 seconds. [#16504](https://github.com/apache/druid/pull/16504) upgraded Calcite from 1.35 to 1.37. This brought in [CALCITE-5948](https://issues.apache.org/jira/browse/CALCITE-5948), a Calcite 1.36 correctness fix that explicitly casts array elements when their types differ from the derived component type. The implementation loops over the array operands and calls: ```java call.setOperand(i, castTo(operands.get(i), elementType)); ``` `SqlBasicCall.setOperand` creates a new copy of the complete immutable operand list for every replacement. With a large string array, many literals require adjustment to the common string type. This results in approximately quadratic copying. A JFR profile of the 100,000-string case on the later Calcite path attributed most allocation pressure to these copies: - `ImmutableList.copyOf`: 50.29% - `Platform.copy`: 41.28% The dominant stack was: ```text SqlArrayValueConstructor.inferReturnType -> SqlValidatorUtil.adjustTypeForArrayConstructor -> SqlValidatorUtil.adjustTypeForMultisetConstructor -> SqlBasicCall.setOperand -> ImmutableNullableList.copyOf ``` #### Release impact Both #16388 and #16504 are included in Druid 31.0.0. Therefore, the fast combination of #16388 with Calcite 1.35 existed temporarily on master but was not included in a Druid release. This is distinct from the original `simplifyOrTerms` problem addressed by #16388. It is a later Calcite array-validation regression, primarily affecting large string `IN` lists. Numeric lists generally do not require the same per-element string type adjustment. #### Suggested fix direction The underlying fix should be made in Calcite, or carried as a narrow backport in Druid: compute all required casts first and replace or reconstruct the operand list once, instead of copying the full list for every array element. -- 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]
