From 713ba226ebf069a779eb5e1f71ab91cbf25fb6de Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Fri, 31 Jul 2026 00:19:12 +0800 Subject: [PATCH 1/3] Fix CodeQL nullness and type warnings --- .../druid/delta/input/DeltaInputSource.java | 3 +- .../emitter/prometheus/PrometheusEmitter.java | 4 +- .../histogram/ApproximateHistogram.java | 2 + .../kafka/supervisor/KafkaSupervisor.java | 2 +- ...rtitionMultiPhaseParallelIndexingTest.java | 5 +- ...rtitionMultiPhaseParallelIndexingTest.java | 11 ++-- .../indexing/overlord/TaskQueueScaleTest.java | 7 +-- .../druid/hll/HyperLogLogCollector.java | 21 ++++---- .../DruidDefaultSerializersModule.java | 5 +- .../druid/java/util/common/logger/Logger.java | 2 +- .../org/apache/druid/math/expr/Function.java | 10 ++-- .../query/ordering/StringComparators.java | 5 ++ .../LazilyDecoratedRowsAndColumns.java | 11 +--- .../generator/ColumnValueGenerator.java | 9 +++- .../parsers/JSONFlattenerMakerTest.java | 4 +- .../StringFirstBufferAggregatorTest.java | 8 +-- .../last/StringLastBufferAggregatorTest.java | 8 +-- .../timeseries/TimeseriesQueryRunnerTest.java | 13 +++-- .../data/BenchmarkIndexibleWrites.java | 11 ++-- .../PredicateValueMatcherFactoryTest.java | 16 +++--- .../table/IndexedTableJoinMatcherTest.java | 4 +- .../druid/utils/CloseableUtilsTest.java | 50 ++++++++----------- .../loading/SegmentLocalCacheManager.java | 6 +-- .../apache/druid/rpc/MockServiceClient.java | 8 ++- .../ImmutableDruidDataSourceTestUtils.java | 8 ++- .../EarliestLatestAnySqlAggregator.java | 15 +++--- 26 files changed, 136 insertions(+), 112 deletions(-) diff --git a/extensions-contrib/druid-deltalake-extensions/src/main/java/org/apache/druid/delta/input/DeltaInputSource.java b/extensions-contrib/druid-deltalake-extensions/src/main/java/org/apache/druid/delta/input/DeltaInputSource.java index 4f255d020f76..0895903d4164 100644 --- a/extensions-contrib/druid-deltalake-extensions/src/main/java/org/apache/druid/delta/input/DeltaInputSource.java +++ b/extensions-contrib/druid-deltalake-extensions/src/main/java/org/apache/druid/delta/input/DeltaInputSource.java @@ -61,6 +61,7 @@ import java.util.ArrayList; import java.util.Iterator; import java.util.List; +import java.util.Objects; import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -160,7 +161,7 @@ public InputSourceReader reader( final StructType fullSnapshotSchema = snapshot.getSchema(engine); final StructType prunedSchema = pruneSchema( fullSnapshotSchema, - inputRowSchema.getColumnsFilter() + Objects.requireNonNull(inputRowSchema, "inputRowSchema").getColumnsFilter() ); final ScanBuilder scanBuilder = snapshot.getScanBuilder(engine); diff --git a/extensions-contrib/prometheus-emitter/src/main/java/org/apache/druid/emitter/prometheus/PrometheusEmitter.java b/extensions-contrib/prometheus-emitter/src/main/java/org/apache/druid/emitter/prometheus/PrometheusEmitter.java index 60af27fcd4d1..e73fdfcf4443 100644 --- a/extensions-contrib/prometheus-emitter/src/main/java/org/apache/druid/emitter/prometheus/PrometheusEmitter.java +++ b/extensions-contrib/prometheus-emitter/src/main/java/org/apache/druid/emitter/prometheus/PrometheusEmitter.java @@ -236,7 +236,9 @@ public void close() server.close(); } } else { - exec.shutdownNow(); + if (exec != null) { + exec.shutdownNow(); + } flush(); try { diff --git a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java index 11236b8d25fe..322172091fe4 100644 --- a/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java +++ b/extensions-core/histogram/src/main/java/org/apache/druid/query/aggregation/histogram/ApproximateHistogram.java @@ -626,6 +626,8 @@ protected ApproximateHistogram foldRule(ApproximateHistogram h, @Nullable float[ // use preallocated arrays if passed if (mergedPositions == null) { mergedPositions = new float[this.size]; + } + if (mergedBins == null) { mergedBins = new long[this.size]; } diff --git a/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisor.java b/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisor.java index 0d8fa70f7f18..c658f45c45ca 100644 --- a/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisor.java +++ b/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/supervisor/KafkaSupervisor.java @@ -145,7 +145,7 @@ protected RecordSupplier setupReco @Override protected int getTaskGroupIdForPartition(KafkaTopicPartition partitionId) { - Integer taskCount = spec.getIoConfig().getTaskCount(); + final int taskCount = spec.getIoConfig().getTaskCount(); if (partitionId.isMultiTopicPartition()) { return Math.abs(31 * partitionId.topic().hashCode() + partitionId.partition()) % taskCount; } else { diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/HashPartitionMultiPhaseParallelIndexingTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/HashPartitionMultiPhaseParallelIndexingTest.java index 8a0b55d1ba16..dae40f38dbe1 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/HashPartitionMultiPhaseParallelIndexingTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/HashPartitionMultiPhaseParallelIndexingTest.java @@ -354,18 +354,17 @@ private void assertHashedPartition( List segmentsInInterval = entry.getValue(); Assert.assertEquals(expectedIntervalToNumSegments.get(interval).intValue(), segmentsInInterval.size()); for (DataSegment segment : segmentsInInterval) { - HashBasedNumberedShardSpec shardSpec = null; if (segment.isTombstone()) { Assert.assertSame(TombstoneShardSpec.class, segment.getShardSpec().getClass()); } else { Assert.assertSame(HashBasedNumberedShardSpec.class, segment.getShardSpec().getClass()); - shardSpec = (HashBasedNumberedShardSpec) segment.getShardSpec(); - Assert.assertEquals(HashPartitionFunction.MURMUR3_32_ABS, shardSpec.getPartitionFunction()); } List results = querySegment(segment, ImmutableList.of("dim1", "dim2"), tempSegmentDir); if (segment.isTombstone()) { Assert.assertTrue(results.isEmpty()); } else { + final HashBasedNumberedShardSpec shardSpec = (HashBasedNumberedShardSpec) segment.getShardSpec(); + Assert.assertEquals(HashPartitionFunction.MURMUR3_32_ABS, shardSpec.getPartitionFunction()); final int hash = shardSpec.getPartitionFunction().hash( HashBasedNumberedShardSpec.serializeGroupKey( getObjectMapper(), diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/RangePartitionMultiPhaseParallelIndexingTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/RangePartitionMultiPhaseParallelIndexingTest.java index cc3abacba0a7..f663aa2cba7f 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/RangePartitionMultiPhaseParallelIndexingTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/common/task/batch/parallel/RangePartitionMultiPhaseParallelIndexingTest.java @@ -454,16 +454,17 @@ private static void assertValuesInRange(List values, DataSegment se Assert.assertTrue(shardSpec.toString(), start != null || end != null); for (StringTuple value : values) { + if (value == null) { + Assert.assertNull("null values should be in first partition", start); + continue; + } + if (start != null) { Assert.assertThat(value.compareTo(start), Matchers.greaterThanOrEqualTo(0)); } if (end != null) { - if (value == null) { - Assert.assertNull("null values should be in first partition", start); - } else { - Assert.assertThat(value.compareTo(end), Matchers.lessThan(0)); - } + Assert.assertThat(value.compareTo(end), Matchers.lessThan(0)); } } } diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/TaskQueueScaleTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/TaskQueueScaleTest.java index 190d78bb33c9..4af3547d29cb 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/overlord/TaskQueueScaleTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/overlord/TaskQueueScaleTest.java @@ -271,7 +271,7 @@ public ListenableFuture run(Task task) try { synchronized (knownTasks) { final TestTaskRunnerWorkItem item2 = knownTasks.get(task.getId()); - if (item2.getState() == RunnerTaskState.PENDING) { + if (item2 != null && item2.getState() == RunnerTaskState.PENDING) { knownTasks.put(task.getId(), item2.withState(RunnerTaskState.RUNNING)); } } @@ -282,7 +282,9 @@ public ListenableFuture run(Task task) final TestTaskRunnerWorkItem item2; synchronized (knownTasks) { item2 = knownTasks.get(task.getId()); - knownTasks.put(task.getId(), item2.withState(RunnerTaskState.NONE)); + if (item2 != null) { + knownTasks.put(task.getId(), item2.withState(RunnerTaskState.NONE)); + } } if (item2 != null) { item2.setResult(TaskStatus.success(task.getId())); @@ -494,4 +496,3 @@ public TestTaskRunnerWorkItem withState(final RunnerTaskState newState) } } } - diff --git a/processing/src/main/java/org/apache/druid/hll/HyperLogLogCollector.java b/processing/src/main/java/org/apache/druid/hll/HyperLogLogCollector.java index d285f1cd044e..9b558a1e1cc8 100644 --- a/processing/src/main/java/org/apache/druid/hll/HyperLogLogCollector.java +++ b/processing/src/main/java/org/apache/druid/hll/HyperLogLogCollector.java @@ -562,32 +562,31 @@ public boolean equals(Object o) return false; } - ByteBuffer otherBuffer = ((HyperLogLogCollector) o).storageBuffer; + final ByteBuffer otherBuffer = ((HyperLogLogCollector) o).storageBuffer; - if (storageBuffer != null ? false : otherBuffer != null) { - return false; - } - - if (storageBuffer == null && otherBuffer == null) { - return true; + if (storageBuffer == null || otherBuffer == null) { + return storageBuffer == otherBuffer; } final ByteBuffer denseStorageBuffer; if (storageBuffer.remaining() != getNumBytesForDenseStorage()) { - HyperLogLogCollector denseCollector = HyperLogLogCollector.makeCollector(storageBuffer.duplicate()); + final HyperLogLogCollector denseCollector = HyperLogLogCollector.makeCollector(storageBuffer.duplicate()); denseCollector.convertToDenseStorage(); denseStorageBuffer = denseCollector.storageBuffer; } else { denseStorageBuffer = storageBuffer; } + final ByteBuffer denseOtherBuffer; if (otherBuffer.remaining() != getNumBytesForDenseStorage()) { - HyperLogLogCollector otherCollector = HyperLogLogCollector.makeCollector(otherBuffer.duplicate()); + final HyperLogLogCollector otherCollector = HyperLogLogCollector.makeCollector(otherBuffer.duplicate()); otherCollector.convertToDenseStorage(); - otherBuffer = otherCollector.storageBuffer; + denseOtherBuffer = otherCollector.storageBuffer; + } else { + denseOtherBuffer = otherBuffer; } - return denseStorageBuffer.equals(otherBuffer); + return denseStorageBuffer.equals(denseOtherBuffer); } @Override diff --git a/processing/src/main/java/org/apache/druid/jackson/DruidDefaultSerializersModule.java b/processing/src/main/java/org/apache/druid/jackson/DruidDefaultSerializersModule.java index da78a0a74616..65489c3db093 100644 --- a/processing/src/main/java/org/apache/druid/jackson/DruidDefaultSerializersModule.java +++ b/processing/src/main/java/org/apache/druid/jackson/DruidDefaultSerializersModule.java @@ -40,6 +40,7 @@ import java.io.IOException; import java.nio.ByteOrder; +import java.util.Objects; /** * @@ -152,8 +153,8 @@ public void serialize(Yielder yielder, final JsonGenerator jgen, SerializerProvi } else { final Class clazz = o.getClass(); - if (serializerClass != clazz) { - serializer = JacksonUtils.getSerializer(provider, clazz); + if (serializer == null || serializerClass != clazz) { + serializer = Objects.requireNonNull(JacksonUtils.getSerializer(provider, clazz)); serializerClass = clazz; } diff --git a/processing/src/main/java/org/apache/druid/java/util/common/logger/Logger.java b/processing/src/main/java/org/apache/druid/java/util/common/logger/Logger.java index 1c1285c8b0c2..c9221d5a3576 100644 --- a/processing/src/main/java/org/apache/druid/java/util/common/logger/Logger.java +++ b/processing/src/main/java/org/apache/druid/java/util/common/logger/Logger.java @@ -295,7 +295,7 @@ private void logException(BiConsumer fn, Throwable t, String fn.accept(message, t); } else { - if (message.isEmpty()) { + if (message == null || message.isEmpty()) { fn.accept(t.toString(), null); } else { fn.accept(StringUtils.nonStrictFormat("%s (%s)", message, t.toString()), null); diff --git a/processing/src/main/java/org/apache/druid/math/expr/Function.java b/processing/src/main/java/org/apache/druid/math/expr/Function.java index 3dd9e61b5303..8cbbaf478f46 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/Function.java +++ b/processing/src/main/java/org/apache/druid/math/expr/Function.java @@ -3257,13 +3257,15 @@ public String name() @Override public ExprEval apply(List args, Expr.ObjectBinding bindings) { - Long left = args.get(0).eval(bindings).asLong(); - Long right = args.get(1).eval(bindings).asLong(); - DateTimeZone timeZone = DateTimes.inferTzFromString(args.get(2).eval(bindings).asString()); + final ExprEval leftEval = args.get(0).eval(bindings); + final ExprEval rightEval = args.get(1).eval(bindings); + final DateTimeZone timeZone = DateTimes.inferTzFromString(args.get(2).eval(bindings).asString()); - if (left == null || right == null) { + if (leftEval.isNumericNull() || rightEval.isNumericNull()) { return ExprEval.ofLong(null); } else { + final long left = leftEval.asLong(); + final long right = rightEval.asLong(); return ExprEval.of(DateTimes.subMonths(right, left, timeZone)); } diff --git a/processing/src/main/java/org/apache/druid/query/ordering/StringComparators.java b/processing/src/main/java/org/apache/druid/query/ordering/StringComparators.java index 650b259ed562..6aa23b415bf5 100644 --- a/processing/src/main/java/org/apache/druid/query/ordering/StringComparators.java +++ b/processing/src/main/java/org/apache/druid/query/ordering/StringComparators.java @@ -69,6 +69,7 @@ public int compare(String s, String s2) // Avoid comparisons for equal references // Assuming we mostly compare different strings, checking s.equals(s2) will only make the comparison slower. //noinspection StringEquality + // codeql[java/reference-equality-on-strings] if (s == s2) { return 0; } @@ -312,6 +313,7 @@ public int compare(String s, String s2) { // Optimization //noinspection StringEquality + // codeql[java/reference-equality-on-strings] if (s == s2) { return 0; } @@ -374,6 +376,7 @@ public int compare(String o1, String o2) // return if o1 and o2 are the same object // Assuming we mostly compare different strings, checking o1.equals(o2) will only make the comparison slower. //noinspection StringEquality + // codeql[java/reference-equality-on-strings] if (o1 == o2) { return 0; } @@ -450,7 +453,9 @@ public static class VersionComparator extends StringComparator @Override public int compare(String o1, String o2) { + // Reference equality is an intentional fast path; value comparison follows below. //noinspection StringEquality + // codeql[java/reference-equality-on-strings] if (o1 == o2) { return 0; } diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/LazilyDecoratedRowsAndColumns.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/LazilyDecoratedRowsAndColumns.java index a45c999b3c8c..df0dd83d07d8 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/LazilyDecoratedRowsAndColumns.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/LazilyDecoratedRowsAndColumns.java @@ -285,15 +285,8 @@ private Pair materializeCursorFactory(CursorFactory cursor cursor.advance(); } - if (writer == null) { - // This means that the accumulate was never called, which can only happen if we didn't have any cursors. - // We would only have zero cursors if we essentially didn't match anything, meaning that our RowsAndColumns - // should be completely empty. - return null; - } else { - final byte[] bytes = writer.toByteArray(); - return Pair.of(bytes, siggy.get()); - } + final byte[] bytes = writer.toByteArray(); + return Pair.of(bytes, siggy.get()); } } diff --git a/processing/src/main/java/org/apache/druid/segment/generator/ColumnValueGenerator.java b/processing/src/main/java/org/apache/druid/segment/generator/ColumnValueGenerator.java index 1fc09f57be6a..9b6b6338fcc3 100644 --- a/processing/src/main/java/org/apache/druid/segment/generator/ColumnValueGenerator.java +++ b/processing/src/main/java/org/apache/druid/segment/generator/ColumnValueGenerator.java @@ -19,6 +19,7 @@ package org.apache.druid.segment.generator; +import com.google.common.base.Preconditions; import org.apache.commons.math3.distribution.AbstractIntegerDistribution; import org.apache.commons.math3.distribution.AbstractRealDistribution; import org.apache.commons.math3.distribution.EnumeratedDistribution; @@ -207,8 +208,12 @@ private void initDistribution() distribution = new UniformIntegerDistribution(schema.getStartInt(), schema.getEndInt()); break; case ENUMERATED: - for (int i = 0; i < enumeratedValues.size(); i++) { - probabilities.add(new Pair<>(enumeratedValues.get(i), enumeratedProbabilities.get(i))); + final List nonNullEnumeratedValues = + Preconditions.checkNotNull(enumeratedValues, "enumeratedValues"); + final List nonNullEnumeratedProbabilities = + Preconditions.checkNotNull(enumeratedProbabilities, "enumeratedProbabilities"); + for (int i = 0; i < nonNullEnumeratedValues.size(); i++) { + probabilities.add(new Pair<>(nonNullEnumeratedValues.get(i), nonNullEnumeratedProbabilities.get(i))); } distribution = new EnumeratedTreeDistribution<>(probabilities); break; diff --git a/processing/src/test/java/org/apache/druid/java/util/common/parsers/JSONFlattenerMakerTest.java b/processing/src/test/java/org/apache/druid/java/util/common/parsers/JSONFlattenerMakerTest.java index 6f1d87a8c35e..13b4b2b63a48 100644 --- a/processing/src/test/java/org/apache/druid/java/util/common/parsers/JSONFlattenerMakerTest.java +++ b/processing/src/test/java/org/apache/druid/java/util/common/parsers/JSONFlattenerMakerTest.java @@ -67,11 +67,11 @@ public void testNumbers() throws JsonProcessingException { JsonNode node; Object result; - Integer i1 = 123; + final int i1 = 123; node = OBJECT_MAPPER.readTree(OBJECT_MAPPER.writeValueAsString(i1)); Assertions.assertTrue(node.isInt()); result = FLATTENER_MAKER.finalizeConversionForMap(node); - Assertions.assertEquals(i1.longValue(), result); + Assertions.assertEquals((long) i1, result); Long l1 = 1L + Integer.MAX_VALUE; node = OBJECT_MAPPER.readTree(OBJECT_MAPPER.writeValueAsString(l1)); diff --git a/processing/src/test/java/org/apache/druid/query/aggregation/firstlast/first/StringFirstBufferAggregatorTest.java b/processing/src/test/java/org/apache/druid/query/aggregation/firstlast/first/StringFirstBufferAggregatorTest.java index 581707b4be00..a5d83f6a0a1f 100644 --- a/processing/src/test/java/org/apache/druid/query/aggregation/firstlast/first/StringFirstBufferAggregatorTest.java +++ b/processing/src/test/java/org/apache/druid/query/aggregation/firstlast/first/StringFirstBufferAggregatorTest.java @@ -48,7 +48,7 @@ public void testBufferAggregate() { final long[] timestamps = {1526724600L, 1526724700L, 1526724800L, 1526725900L, 1526725000L}; final String[] strings = {"AAAA", "BBBB", "CCCC", "DDDD", "EEEE"}; - Integer maxStringBytes = 1024; + final int maxStringBytes = 1024; TestLongColumnSelector longColumnSelector = new TestLongColumnSelector(timestamps); TestObjectColumnSelector objectColumnSelector = new TestObjectColumnSelector<>(strings); @@ -85,7 +85,7 @@ public void testBufferAggregateWithFoldCheck() { final long[] timestamps = {1526724600L, 1526724700L, 1526724800L, 1526725900L, 1526725000L}; final String[] strings = {"AAAA", "BBBB", "CCCC", "DDDD", "EEEE"}; - Integer maxStringBytes = 1024; + final int maxStringBytes = 1024; TestLongColumnSelector longColumnSelector = new TestLongColumnSelector(timestamps); TestObjectColumnSelector objectColumnSelector = new TestObjectColumnSelector<>(strings); @@ -123,7 +123,7 @@ public void testNullBufferAggregate() final long[] timestamps = {2222L, 1111L, 3333L, 4444L, 5555L}; final String[] strings = {null, "AAAA", "BBBB", "DDDD", "EEEE"}; - Integer maxStringBytes = 1024; + final int maxStringBytes = 1024; TestLongColumnSelector longColumnSelector = new TestLongColumnSelector(timestamps); TestObjectColumnSelector objectColumnSelector = new TestObjectColumnSelector<>(strings); @@ -162,7 +162,7 @@ public void testNoStringValue() final long[] timestamps = {1526724000L, 1526724600L}; final Double[] doubles = {null, 2.00}; - Integer maxStringBytes = 1024; + final int maxStringBytes = 1024; TestLongColumnSelector longColumnSelector = new TestLongColumnSelector(timestamps); TestObjectColumnSelector objectColumnSelector = new TestObjectColumnSelector<>(doubles); diff --git a/processing/src/test/java/org/apache/druid/query/aggregation/firstlast/last/StringLastBufferAggregatorTest.java b/processing/src/test/java/org/apache/druid/query/aggregation/firstlast/last/StringLastBufferAggregatorTest.java index 3f1e66f802a5..5f38c623982e 100644 --- a/processing/src/test/java/org/apache/druid/query/aggregation/firstlast/last/StringLastBufferAggregatorTest.java +++ b/processing/src/test/java/org/apache/druid/query/aggregation/firstlast/last/StringLastBufferAggregatorTest.java @@ -49,7 +49,7 @@ public void testBufferAggregate() final long[] timestamps = {1526724600L, 1526724700L, 1526724800L, 1526725900L, 1526725000L}; final String[] strings = {"AAAA", "BBBB", "CCCC", "DDDD", "EEEE"}; - Integer maxStringBytes = 1024; + final int maxStringBytes = 1024; TestLongColumnSelector longColumnSelector = new TestLongColumnSelector(timestamps); TestObjectColumnSelector objectColumnSelector = new TestObjectColumnSelector<>(strings); @@ -87,7 +87,7 @@ public void testBufferAggregateWithFoldCheck() { final long[] timestamps = {1526724600L, 1526724700L, 1526724800L, 1526725900L, 1526725000L}; final String[] strings = {"AAAA", "BBBB", "CCCC", "DDDD", "EEEE"}; - Integer maxStringBytes = 1024; + final int maxStringBytes = 1024; TestLongColumnSelector longColumnSelector = new TestLongColumnSelector(timestamps); TestObjectColumnSelector objectColumnSelector = new TestObjectColumnSelector<>(strings); @@ -125,7 +125,7 @@ public void testNullBufferAggregate() final long[] timestamps = {1111L, 2222L, 6666L, 4444L, 5555L}; final String[] strings = {"CCCC", "AAAA", "BBBB", null, "EEEE"}; - Integer maxStringBytes = 1024; + final int maxStringBytes = 1024; TestLongColumnSelector longColumnSelector = new TestLongColumnSelector(timestamps); TestObjectColumnSelector objectColumnSelector = new TestObjectColumnSelector<>(strings); @@ -164,7 +164,7 @@ public void testNonStringValue() final long[] timestamps = {1526724000L, 1526724600L}; final Double[] doubles = {null, 2.00}; - Integer maxStringBytes = 1024; + final int maxStringBytes = 1024; TestLongColumnSelector longColumnSelector = new TestLongColumnSelector(timestamps); TestObjectColumnSelector objectColumnSelector = new TestObjectColumnSelector<>(doubles); diff --git a/processing/src/test/java/org/apache/druid/query/timeseries/TimeseriesQueryRunnerTest.java b/processing/src/test/java/org/apache/druid/query/timeseries/TimeseriesQueryRunnerTest.java index 27697e572210..7ac3bf878014 100644 --- a/processing/src/test/java/org/apache/druid/query/timeseries/TimeseriesQueryRunnerTest.java +++ b/processing/src/test/java/org/apache/druid/query/timeseries/TimeseriesQueryRunnerTest.java @@ -96,6 +96,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.stream.Collectors; import java.util.stream.StreamSupport; @@ -324,7 +325,8 @@ public void testFullOnTimeseries() } stubServiceEmitter.verifyEmitted("query/wait/time", ImmutableMap.of("vectorized", vectorize), 1); - Assert.assertEquals(lastResult.toString(), expectedLast, lastResult.getTimestamp()); + final Result nonNullLastResult = Objects.requireNonNull(lastResult, "Expected at least one query result"); + Assert.assertEquals(nonNullLastResult.toString(), expectedLast, nonNullLastResult.getTimestamp()); } @Test @@ -356,7 +358,8 @@ public void testTimeseriesNoAggregators() lastResult = result; } - Assert.assertEquals(lastResult.toString(), expectedLast, lastResult.getTimestamp()); + final Result nonNullLastResult = Objects.requireNonNull(lastResult, "Expected at least one query result"); + Assert.assertEquals(nonNullLastResult.toString(), expectedLast, nonNullLastResult.getTimestamp()); } @Test @@ -2820,7 +2823,8 @@ public void testTimeseriesWithTimestampResultFieldContextForArrayResponse() lastResult = result; ++count; } - Assert.assertEquals(expectedLast, lastResult[0]); + final Object[] nonNullLastResult = Objects.requireNonNull(lastResult, "Expected at least one query result"); + Assert.assertEquals(expectedLast, nonNullLastResult[0]); } @Test @@ -2925,7 +2929,8 @@ public void testTimeseriesWithTimestampResultFieldContextForMapResponse() ++count; } - Assert.assertEquals(lastResult.toString(), expectedLast, lastResult.getTimestamp()); + final Result nonNullLastResult = Objects.requireNonNull(lastResult, "Expected at least one query result"); + Assert.assertEquals(nonNullLastResult.toString(), expectedLast, nonNullLastResult.getTimestamp()); } @Test diff --git a/processing/src/test/java/org/apache/druid/segment/data/BenchmarkIndexibleWrites.java b/processing/src/test/java/org/apache/druid/segment/data/BenchmarkIndexibleWrites.java index 9b79ff1ff17f..9804e0c784c0 100644 --- a/processing/src/test/java/org/apache/druid/segment/data/BenchmarkIndexibleWrites.java +++ b/processing/src/test/java/org/apache/druid/segment/data/BenchmarkIndexibleWrites.java @@ -133,7 +133,7 @@ public void clear() reference.set((V[]) new Object[initSize]); } - private static Boolean wasCopying(Long val) + private static boolean wasCopying(long val) { return (val & 1L) > 0; } @@ -142,12 +142,13 @@ private static Boolean wasCopying(Long val) public void set(Integer index, V object) { ensureCapacity(index + 1); - Long pre, post; + long pre; + long post; do { pre = resizeCount.get(); reference.get()[index] = object; post = resizeCount.get(); - } while (wasCopying(pre) || wasCopying(post) || (!pre.equals(post))); + } while (wasCopying(pre) || wasCopying(post) || pre != post); } private final Object resizeMutex = new Object(); @@ -198,7 +199,7 @@ public void testConcurrentWrites() throws ExecutionException, InterruptedExcepti final AtomicInteger index = new AtomicInteger(0); List> futures = new ArrayList<>(); - final Integer loops = totalIndexSize / concurrentThreads; + final int loops = totalIndexSize / concurrentThreads; for (int i = 0; i < concurrentThreads; ++i) { futures.add( @@ -251,7 +252,7 @@ public void testConcurrentReads() throws ExecutionException, InterruptedExceptio final AtomicInteger queryableIndex = new AtomicInteger(0); List> futures = new ArrayList<>(); - final Integer loops = totalIndexSize / concurrentThreads; + final int loops = totalIndexSize / concurrentThreads; final AtomicBoolean done = new AtomicBoolean(false); diff --git a/processing/src/test/java/org/apache/druid/segment/filter/PredicateValueMatcherFactoryTest.java b/processing/src/test/java/org/apache/druid/segment/filter/PredicateValueMatcherFactoryTest.java index c82292661416..dc701fcb24d8 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/PredicateValueMatcherFactoryTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/PredicateValueMatcherFactoryTest.java @@ -157,26 +157,26 @@ public void testDoubleProcessorNotMatchingValue() @Test public void testNumberProcessorMatchingValue() { - Double num = 2.; + final double num = 2.; final TestColumnValueSelector columnValueSelector = TestColumnValueSelector.of( Number.class, ImmutableList.of(new Number() { @Override public int intValue() { - return num.intValue(); + return (int) num; } @Override public long longValue() { - return num.longValue(); + return (long) num; } @Override public float floatValue() { - return num.floatValue(); + return (float) num; } @Override @@ -194,26 +194,26 @@ public double doubleValue() @Test public void testNumberProcessorNotMatchingValue() { - Double num = 2.; + final double num = 2.; final TestColumnValueSelector columnValueSelector = TestColumnValueSelector.of( Double.class, ImmutableList.of(new Number() { @Override public int intValue() { - return num.intValue(); + return (int) num; } @Override public long longValue() { - return num.longValue(); + return (long) num; } @Override public float floatValue() { - return num.floatValue(); + return (float) num; } @Override diff --git a/processing/src/test/java/org/apache/druid/segment/join/table/IndexedTableJoinMatcherTest.java b/processing/src/test/java/org/apache/druid/segment/join/table/IndexedTableJoinMatcherTest.java index 5ddd8258626f..d95fc2cbd3de 100644 --- a/processing/src/test/java/org/apache/druid/segment/join/table/IndexedTableJoinMatcherTest.java +++ b/processing/src/test/java/org/apache/druid/segment/join/table/IndexedTableJoinMatcherTest.java @@ -381,8 +381,8 @@ public void doesNotLoadIfPresent() @Test public void evictsLeastRecentlyUsed() { - Long start = 1L; - Long next = start + SIZE; + final long start = 1L; + final Long next = start + SIZE; for (long i = start; i < next; i++) { Long key = i; diff --git a/processing/src/test/java/org/apache/druid/utils/CloseableUtilsTest.java b/processing/src/test/java/org/apache/druid/utils/CloseableUtilsTest.java index 68d37f80205f..413a87e6af44 100644 --- a/processing/src/test/java/org/apache/druid/utils/CloseableUtilsTest.java +++ b/processing/src/test/java/org/apache/druid/utils/CloseableUtilsTest.java @@ -64,19 +64,19 @@ public void test_closeAll_list_quiet() throws IOException @Test public void test_closeAll_array_loud() { - Exception e = null; - try { - CloseableUtils.closeAll(quietCloseable, null, ioExceptionCloseable, quietCloseable2, runtimeExceptionCloseable); - } - catch (Exception e2) { - e = e2; - } + final IOException e = Assert.assertThrows( + IOException.class, + () -> CloseableUtils.closeAll( + quietCloseable, + null, + ioExceptionCloseable, + quietCloseable2, + runtimeExceptionCloseable + ) + ); assertClosed(quietCloseable, ioExceptionCloseable, quietCloseable2, runtimeExceptionCloseable); - // First exception - Assert.assertThat(e, CoreMatchers.instanceOf(IOException.class)); - // Second exception Assert.assertEquals(1, e.getSuppressed().length); Assert.assertThat(e.getSuppressed()[0], CoreMatchers.instanceOf(IllegalArgumentException.class)); @@ -85,27 +85,21 @@ public void test_closeAll_array_loud() @Test public void test_closeAll_list_loud() { - Exception e = null; - try { - CloseableUtils.closeAll( - Arrays.asList( - quietCloseable, - null, - ioExceptionCloseable, - quietCloseable2, - runtimeExceptionCloseable - ) - ); - } - catch (Exception e2) { - e = e2; - } + final IOException e = Assert.assertThrows( + IOException.class, + () -> CloseableUtils.closeAll( + Arrays.asList( + quietCloseable, + null, + ioExceptionCloseable, + quietCloseable2, + runtimeExceptionCloseable + ) + ) + ); assertClosed(quietCloseable, ioExceptionCloseable, quietCloseable2, runtimeExceptionCloseable); - // First exception - Assert.assertThat(e, CoreMatchers.instanceOf(IOException.class)); - // Second exception Assert.assertEquals(1, e.getSuppressed().length); Assert.assertThat(e.getSuppressed()[0], CoreMatchers.instanceOf(IllegalArgumentException.class)); diff --git a/server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java b/server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java index 8ee13001f237..873a06b4926c 100644 --- a/server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java +++ b/server/src/main/java/org/apache/druid/segment/loading/SegmentLocalCacheManager.java @@ -2014,13 +2014,13 @@ public void mount(StorageLocation mountLocation) throws SegmentLoadingException log.makeAlert( e, "Failed to load segment in current location [%s], try next location if any", - location.getPath().getAbsolutePath() - ).addData("location", location.getPath().getAbsolutePath()).emit(); + mountLocation.getPath().getAbsolutePath() + ).addData("location", mountLocation.getPath().getAbsolutePath()).emit(); throw new SegmentLoadingException( "Failed to load segment[%s] in reserved location[%s]", dataSegment.getId(), - location.getPath().getAbsolutePath() + mountLocation.getPath().getAbsolutePath() ); } finally { diff --git a/server/src/test/java/org/apache/druid/rpc/MockServiceClient.java b/server/src/test/java/org/apache/druid/rpc/MockServiceClient.java index 4dacf0e62ee8..71adf530d206 100644 --- a/server/src/test/java/org/apache/druid/rpc/MockServiceClient.java +++ b/server/src/test/java/org/apache/druid/rpc/MockServiceClient.java @@ -34,6 +34,7 @@ import java.util.ArrayDeque; import java.util.Map; +import java.util.Objects; import java.util.Queue; /** @@ -50,12 +51,15 @@ public ListenableFuture asyncRequest( final HttpResponseHandler handler ) { - final Expectation expectation = expectations.poll(); + final Expectation expectation = Objects.requireNonNull( + expectations.poll(), + "No expectation configured for the next request" + ); requestNumber++; Assert.assertEquals( "request[" + requestNumber + "]", - expectation == null ? null : expectation.request, + expectation.request, requestBuilder ); diff --git a/server/src/test/java/org/apache/druid/test/utils/ImmutableDruidDataSourceTestUtils.java b/server/src/test/java/org/apache/druid/test/utils/ImmutableDruidDataSourceTestUtils.java index 80036debd093..be9da0cfb7e0 100644 --- a/server/src/test/java/org/apache/druid/test/utils/ImmutableDruidDataSourceTestUtils.java +++ b/server/src/test/java/org/apache/druid/test/utils/ImmutableDruidDataSourceTestUtils.java @@ -62,11 +62,17 @@ private static boolean checkEquals( * @param actual actual list * @return */ - public static boolean assertEquals(List expected, List actual) + public static boolean assertEquals( + @Nullable List expected, + @Nullable List actual + ) { if (expected == null) { return actual == null; } + if (actual == null) { + return false; + } Assert.assertEquals("expected and actual ImmutableDruidDataSource lists should be of equal size", expected.size(), actual.size()); diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/EarliestLatestAnySqlAggregator.java b/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/EarliestLatestAnySqlAggregator.java index 73209a76a987..3262a95104c5 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/EarliestLatestAnySqlAggregator.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/EarliestLatestAnySqlAggregator.java @@ -254,20 +254,22 @@ public Aggregation toDruidAggregation( case 1: theAggFactory = aggregatorType.createAggregatorFactory(aggregatorName, fieldName, null, outputType, null, true); break; - case 2: - Integer maxStringBytes = RexLiteral.intValue(rexNodes.get(1)); // added not null check at the function + case 2: { + final int maxStringBytes = RexLiteral.intValue(rexNodes.get(1)); // added not null check at the function theAggFactory = aggregatorType.createAggregatorFactory( aggregatorName, fieldName, null, outputType, - maxStringBytes.intValue(), + maxStringBytes, true ); break; - case 3: - maxStringBytes = RexLiteral.intValue(rexNodes.get(1)); // added not null check at the function for rexNode 1,2 - boolean aggregateMultipleValues = RexLiteral.booleanValue(rexNodes.get(2)); + } + case 3: { + 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)); theAggFactory = aggregatorType.createAggregatorFactory( aggregatorName, fieldName, @@ -277,6 +279,7 @@ public Aggregation toDruidAggregation( aggregateMultipleValues ); break; + } default: throw InvalidSqlInput.exception( "Function [%s] expects 1 or 2 or 3 arguments but found [%s]", From 62678a7841eae68c54d23eaeb24ea040a5754fe2 Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Fri, 31 Jul 2026 06:51:37 +0800 Subject: [PATCH 2/3] fix: address nullness review feedback --- .../generator/ColumnValueGenerator.java | 6 ++++++ .../segment/generator/DataGeneratorTest.java | 21 +++++++++++++++++++ .../table/IndexedTableJoinMatcherTest.java | 8 +++---- .../EarliestLatestAnySqlAggregator.java | 5 ++--- 4 files changed, 33 insertions(+), 7 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/segment/generator/ColumnValueGenerator.java b/processing/src/main/java/org/apache/druid/segment/generator/ColumnValueGenerator.java index 9b6b6338fcc3..b4c4e83bbea4 100644 --- a/processing/src/main/java/org/apache/druid/segment/generator/ColumnValueGenerator.java +++ b/processing/src/main/java/org/apache/druid/segment/generator/ColumnValueGenerator.java @@ -212,6 +212,12 @@ private void initDistribution() Preconditions.checkNotNull(enumeratedValues, "enumeratedValues"); final List nonNullEnumeratedProbabilities = Preconditions.checkNotNull(enumeratedProbabilities, "enumeratedProbabilities"); + Preconditions.checkArgument( + nonNullEnumeratedValues.size() == nonNullEnumeratedProbabilities.size(), + "enumeratedValues size[%s] must match enumeratedProbabilities size[%s]", + nonNullEnumeratedValues.size(), + nonNullEnumeratedProbabilities.size() + ); for (int i = 0; i < nonNullEnumeratedValues.size(); i++) { probabilities.add(new Pair<>(nonNullEnumeratedValues.get(i), nonNullEnumeratedProbabilities.get(i))); } diff --git a/processing/src/test/java/org/apache/druid/segment/generator/DataGeneratorTest.java b/processing/src/test/java/org/apache/druid/segment/generator/DataGeneratorTest.java index 11f5223d6831..5bc2482179c7 100644 --- a/processing/src/test/java/org/apache/druid/segment/generator/DataGeneratorTest.java +++ b/processing/src/test/java/org/apache/druid/segment/generator/DataGeneratorTest.java @@ -298,6 +298,27 @@ public void testEnumerated() tracker.assertTotalRowSize("dimA", 10000); } + @Test + public void testEnumeratedRejectsMismatchedValuesAndProbabilities() + { + final GeneratorColumnSchema schema = GeneratorColumnSchema.makeEnumerated( + "dimA", + ValueType.STRING, + false, + 1, + null, + Arrays.asList("Hello", "World"), + Collections.singletonList(1.0) + ); + + final IllegalArgumentException exception = Assert.assertThrows( + IllegalArgumentException.class, + () -> schema.makeGenerator(9999) + ); + Assert.assertTrue(exception.getMessage().contains("enumeratedValues size[2]")); + Assert.assertTrue(exception.getMessage().contains("enumeratedProbabilities size[1]")); + } + @Test public void testNormal() { diff --git a/processing/src/test/java/org/apache/druid/segment/join/table/IndexedTableJoinMatcherTest.java b/processing/src/test/java/org/apache/druid/segment/join/table/IndexedTableJoinMatcherTest.java index d95fc2cbd3de..9ecd5c9f9c1a 100644 --- a/processing/src/test/java/org/apache/druid/segment/join/table/IndexedTableJoinMatcherTest.java +++ b/processing/src/test/java/org/apache/druid/segment/join/table/IndexedTableJoinMatcherTest.java @@ -382,14 +382,14 @@ public void doesNotLoadIfPresent() public void evictsLeastRecentlyUsed() { final long start = 1L; - final Long next = start + SIZE; + final long next = start + SIZE; for (long i = start; i < next; i++) { - Long key = i; - Assert.assertEquals(key, target.getAndLoadIfAbsent(key)); + final long key = i; + Assert.assertEquals(key, target.getAndLoadIfAbsent(key).longValue()); } - Assert.assertEquals(next, target.getAndLoadIfAbsent(next)); + Assert.assertEquals(next, target.getAndLoadIfAbsent(next).longValue()); Assert.assertNull(target.get(start)); Assert.assertEquals(SIZE + 1, counter.longValue()); diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/EarliestLatestAnySqlAggregator.java b/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/EarliestLatestAnySqlAggregator.java index 3262a95104c5..902132f1812c 100644 --- a/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/EarliestLatestAnySqlAggregator.java +++ b/sql/src/main/java/org/apache/druid/sql/calcite/aggregation/builtin/EarliestLatestAnySqlAggregator.java @@ -255,7 +255,7 @@ public Aggregation toDruidAggregation( theAggFactory = aggregatorType.createAggregatorFactory(aggregatorName, fieldName, null, outputType, null, true); break; case 2: { - final int maxStringBytes = RexLiteral.intValue(rexNodes.get(1)); // added not null check at the function + final int maxStringBytes = RexLiteral.intValue(rexNodes.get(1)); theAggFactory = aggregatorType.createAggregatorFactory( aggregatorName, fieldName, @@ -267,8 +267,7 @@ public Aggregation toDruidAggregation( break; } case 3: { - final int maxStringBytes = - RexLiteral.intValue(rexNodes.get(1)); // added not null check at the function for rexNode 1,2 + final int maxStringBytes = RexLiteral.intValue(rexNodes.get(1)); final boolean aggregateMultipleValues = RexLiteral.booleanValue(rexNodes.get(2)); theAggFactory = aggregatorType.createAggregatorFactory( aggregatorName, From 0eabf304a2581cd226283a148f3fa8854dde6e26 Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Fri, 31 Jul 2026 07:18:19 +0800 Subject: [PATCH 3/3] fix: address suppressed nullness review findings --- .../LazilyDecoratedRowsAndColumns.java | 61 ++++++++++--------- .../ImmutableDruidDataSourceTestUtils.java | 11 ++-- ...ImmutableDruidDataSourceTestUtilsTest.java | 46 ++++++++++++++ 3 files changed, 82 insertions(+), 36 deletions(-) create mode 100644 server/src/test/java/org/apache/druid/test/utils/ImmutableDruidDataSourceTestUtilsTest.java diff --git a/processing/src/main/java/org/apache/druid/query/rowsandcols/LazilyDecoratedRowsAndColumns.java b/processing/src/main/java/org/apache/druid/query/rowsandcols/LazilyDecoratedRowsAndColumns.java index df0dd83d07d8..270ab566814b 100644 --- a/processing/src/main/java/org/apache/druid/query/rowsandcols/LazilyDecoratedRowsAndColumns.java +++ b/processing/src/main/java/org/apache/druid/query/rowsandcols/LazilyDecoratedRowsAndColumns.java @@ -276,17 +276,18 @@ private Pair materializeCursorFactory(CursorFactory cursor sortColumns ); - final FrameWriter writer = frameWriterFactory.newFrameWriter(columnSelectorFactory); - for (; !cursor.isDoneOrInterrupted() && remainingRowsToSkip > 0; remainingRowsToSkip--) { - cursor.advance(); - } - for (; !cursor.isDoneOrInterrupted() && remainingRowsToFetch > 0; remainingRowsToFetch--) { - writer.addSelection(); - cursor.advance(); - } + try (final FrameWriter writer = frameWriterFactory.newFrameWriter(columnSelectorFactory)) { + for (; !cursor.isDoneOrInterrupted() && remainingRowsToSkip > 0; remainingRowsToSkip--) { + cursor.advance(); + } + for (; !cursor.isDoneOrInterrupted() && remainingRowsToFetch > 0; remainingRowsToFetch--) { + writer.addSelection(); + cursor.advance(); + } - final byte[] bytes = writer.toByteArray(); - return Pair.of(bytes, siggy.get()); + final byte[] bytes = writer.toByteArray(); + return Pair.of(bytes, siggy.get()); + } } } @@ -376,26 +377,28 @@ private Pair naiveMaterialize(RowsAndColumns rac) long remainingRowsToSkip = limit.getOffset(); long remainingRowsToFetch = limit.getLimitOrMax(); - final FrameWriter frameWriter = FrameWriters.makeColumnBasedFrameWriterFactory( - memFactory, - sigBob.build(), - Collections.emptyList() - ).newFrameWriter(selectorFactory); - - rowId.set(0); - for (; rowId.get() < numRows && remainingRowsToFetch > 0; rowId.incrementAndGet()) { - final int theId = rowId.get(); - if (rowsToSkip != null && rowsToSkip.get(theId)) { - continue; - } - if (remainingRowsToSkip > 0) { - remainingRowsToSkip--; - continue; + try ( + final FrameWriter frameWriter = FrameWriters.makeColumnBasedFrameWriterFactory( + memFactory, + sigBob.build(), + Collections.emptyList() + ).newFrameWriter(selectorFactory) + ) { + rowId.set(0); + for (; rowId.get() < numRows && remainingRowsToFetch > 0; rowId.incrementAndGet()) { + final int theId = rowId.get(); + if (rowsToSkip != null && rowsToSkip.get(theId)) { + continue; + } + if (remainingRowsToSkip > 0) { + remainingRowsToSkip--; + continue; + } + remainingRowsToFetch--; + frameWriter.addSelection(); } - remainingRowsToFetch--; - frameWriter.addSelection(); - } - return Pair.of(frameWriter.toByteArray(), sigBob.build()); + return Pair.of(frameWriter.toByteArray(), sigBob.build()); + } } } diff --git a/server/src/test/java/org/apache/druid/test/utils/ImmutableDruidDataSourceTestUtils.java b/server/src/test/java/org/apache/druid/test/utils/ImmutableDruidDataSourceTestUtils.java index be9da0cfb7e0..fbeaeb0f4274 100644 --- a/server/src/test/java/org/apache/druid/test/utils/ImmutableDruidDataSourceTestUtils.java +++ b/server/src/test/java/org/apache/druid/test/utils/ImmutableDruidDataSourceTestUtils.java @@ -60,19 +60,17 @@ private static boolean checkEquals( * test code * @param expected expected list * @param actual actual list - * @return */ - public static boolean assertEquals( + public static void assertEquals( @Nullable List expected, @Nullable List actual ) { if (expected == null) { - return actual == null; - } - if (actual == null) { - return false; + Assert.assertNull(actual); + return; } + Assert.assertNotNull(actual); Assert.assertEquals("expected and actual ImmutableDruidDataSource lists should be of equal size", expected.size(), actual.size()); @@ -83,7 +81,6 @@ public static boolean assertEquals( "ImmutableDruidDataSource's equalsForTesting()" + " method"); } } - return true; } private static boolean contains(ImmutableDruidDataSource expected, List actualList) diff --git a/server/src/test/java/org/apache/druid/test/utils/ImmutableDruidDataSourceTestUtilsTest.java b/server/src/test/java/org/apache/druid/test/utils/ImmutableDruidDataSourceTestUtilsTest.java new file mode 100644 index 000000000000..01018edf5473 --- /dev/null +++ b/server/src/test/java/org/apache/druid/test/utils/ImmutableDruidDataSourceTestUtilsTest.java @@ -0,0 +1,46 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.druid.test.utils; + +import org.junit.Assert; +import org.junit.Test; + +import java.util.Collections; + +public class ImmutableDruidDataSourceTestUtilsTest +{ + @Test + public void testListAssertEqualsRejectsNullActual() + { + Assert.assertThrows( + AssertionError.class, + () -> ImmutableDruidDataSourceTestUtils.assertEquals(Collections.emptyList(), null) + ); + } + + @Test + public void testListAssertEqualsRejectsNullExpected() + { + Assert.assertThrows( + AssertionError.class, + () -> ImmutableDruidDataSourceTestUtils.assertEquals(null, Collections.emptyList()) + ); + } +}