fix: address CodeQL nullness and type warnings

[FrankChen021]

What

• make nullable control flow explicit before dereferences
• replace unnecessary boxed locals with primitives
• correctly preserve numeric nulls in subtract_months
• retain deliberate string-reference identity fast paths with narrow CodeQL annotations

Why

This addresses all 43 CodeQL warnings in the nullness/type group:

• 20 java/dereferenced-value-may-be-null
• 19 java/non-null-boxed-variable
• 4 java/reference-equality-on-strings

Impact

The changes prevent possible null dereferences, make test assumptions fail clearly, avoid unnecessary boxing, and fix numeric-null propagation in subtract_months. The string identity comparisons remain behavior-preserving performance fast paths.

Root cause

Several code paths relied on invariants that were not expressed to the type system, while subtract_months boxed primitive values and then performed ineffective null checks.

Checks

• affected-module compilation and Checkstyle passed
• production nullness tests: 182 passed
• boxed-variable tests: 135 passed
• test-nullness and comparator tests: 349 passed
• git diff --check

Created by GPT-5.6-Sol.

[Copilot]

Pull request overview

This PR focuses on resolving CodeQL nullness/type warnings across Druid by making null-handling explicit, reducing unnecessary boxing, and documenting intentional string-reference fast paths, while also fixing numeric-null propagation in the subtract_months expression function.

Changes:

• Replaced boxed locals with primitives and tightened null contracts/assumptions (often via Objects.requireNonNull / explicit null checks) to avoid dereferences of possibly-null values.
• Fixed numeric-null handling for subtract_months by using ExprEval.isNumericNull() prior to converting to primitives.
• Preserved deliberate String reference-equality fast paths with narrow CodeQL annotations.

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/EarliestLatestAnySqlAggregator.java	Removes unnecessary boxing around maxStringBytes; clarifies argument handling for aggregator factory creation.
server/src/test/java/org/apache/druid/test/utils/ImmutableDruidDataSourceTestUtils.java	Makes nullability of expected/actual lists explicit and avoids null dereference paths in test helper.
server/src/test/java/org/apache/druid/rpc/MockServiceClient.java	Fails fast with a clearer error when requests exceed configured expectations.
server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java	Uses the correct mount location in alert/log/error paths during mount failures.
processing/src/test/java/org/apache/druid/utils/CloseableUtilsTest.java	Simplifies exception assertions using Assert.assertThrows and preserves suppressed-exception checks.
processing/src/test/java/org/apache/druid/segment/join/table/IndexedTableJoinMatcherTest.java	Reduces boxing in tests by moving toward primitives for loop bounds.
processing/src/test/java/org/apache/druid/segment/filter/PredicateValueMatcherFactoryTest.java	Removes unnecessary Double boxing in test numeric implementations.
processing/src/test/java/org/apache/druid/segment/data/BenchmarkIndexibleWrites.java	Removes unnecessary boxing and replaces equals comparisons with primitive comparisons in benchmark/test code.
processing/src/test/java/org/apache/druid/query/timeseries/TimeseriesQueryRunnerTest.java	Adds explicit non-null assertions for expected query results in tests.
processing/src/test/java/org/apache/druid/query/aggregation/firstlast/last/StringLastBufferAggregatorTest.java	Converts boxed Integer configuration values to primitives in tests.
processing/src/test/java/org/apache/druid/query/aggregation/firstlast/first/StringFirstBufferAggregatorTest.java	Converts boxed Integer configuration values to primitives in tests.
processing/src/test/java/org/apache/druid/java/util/common/parsers/JSONFlattenerMakerTest.java	Removes unnecessary boxing and makes numeric expectations explicit.
processing/src/main/java/org/apache/druid/segment/generator/ColumnValueGenerator.java	Adds explicit non-null checks for enumerated schema inputs before use.
processing/src/main/java/org/apache/druid/query/rowsandcols/LazilyDecoratedRowsAndColumns.java	Simplifies materialization return flow after cursor iteration.
processing/src/main/java/org/apache/druid/query/ordering/StringComparators.java	Documents intentional String reference-equality fast paths with CodeQL annotations.
processing/src/main/java/org/apache/druid/math/expr/Function.java	Fixes numeric-null preservation for subtract_months by checking numeric-null before primitive conversion.
processing/src/main/java/org/apache/druid/java/util/common/logger/Logger.java	Avoids possible NPE by treating message as nullable in non-stacktrace logging paths.
processing/src/main/java/org/apache/druid/jackson/DruidDefaultSerializersModule.java	Makes cached serializer usage safe when cache is empty/uninitialized.
processing/src/main/java/org/apache/druid/hll/HyperLogLogCollector.java	Fixes null-handling and avoids mutating local buffers incorrectly during equality checks.
indexing-service/src/test/java/org/apache/druid/indexing/overlord/TaskQueueScaleTest.java	Adds null checks around work item state transitions to avoid test-time NPEs.
indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/RangePartitionMultiPhaseParallelIndexingTest.java	Moves null-value handling earlier for clearer test logic and avoids null compare paths.
indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/HashPartitionMultiPhaseParallelIndexingTest.java	Tightens shard spec casting/usage to only non-tombstone segments.
extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisor.java	Removes unnecessary boxing for taskCount in partition-to-task-group mapping.
extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java	Fixes allocation logic so preallocated arrays are only created when missing.
extensions-contrib/prometheus-emitter/src/main/java/org/apache/druid/emitter/prometheus/PrometheusEmitter.java	Avoids possible NPE on shutdown by guarding executor shutdown with a null check.
extensions-contrib/druid-deltalake-extensions/src/main/java/org/apache/druid/delta/input/DeltaInputSource.java	Makes inputRowSchema non-null requirement explicit before dereferencing.

Comments suppressed due to low confidence (1)

sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/EarliestLatestAnySqlAggregator.java:272

• Same as above: this comment about adding a null check is misleading and makes the control-flow unclear. Prefer removing the comment (or referencing the exact validation) and keep the declaration straightforward.

        final int maxStringBytes =
            RexLiteral.intValue(rexNodes.get(1)); // added not null check at the function for rexNode 1,2
        final boolean aggregateMultipleValues = RexLiteral.booleanValue(rexNodes.get(2));

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

processing/src/main/java/org/apache/druid/query/rowsandcols/LazilyDecoratedRowsAndColumns.java:289

• FrameWriter.toByteArray() requires the writer to be closed afterward (it may hold allocator-backed memory). This code calls toByteArray() and returns without closing, which can leak resources on repeated materializations. Wrap the writer in try-with-resources (same pattern likely needed in the other FrameWriter usage in this class).

      final byte[] bytes = writer.toByteArray();
      return Pair.of(bytes, siggy.get());

server/src/test/java/org/apache/druid/test/utils/ImmutableDruidDataSourceTestUtils.java:75

• This assertEquals helper is used as a statement in tests (return value ignored). Returning false when actual is null will silently let tests pass without any assertion failure. Since the method is named assertEquals and already throws/asserts in other mismatch cases, it should also fail fast when expected is non-null but actual is null.

    if (actual == null) {
      return false;
    }

[FrankChen021]

Addressed both suppressed Copilot findings in commit 0eabf30. Both FrameWriter instances are now closed with try-with-resources, and the list assertion helper now throws on either null mismatch instead of returning an ignored false value. The materialization tests (15 total, 10 existing skips) and two new null-mismatch tests pass; full processing/server Checkstyle, PMD, and enforcer validation also passes.

[FrankChen021]

I have reviewed the code for correctness, edge cases, concurrency, and integration risks; no issues found.

Reviewed 28 of 28 changed files.

---

This is an automated review by Codex GPT-5.6-Sol