From 016d982f37d88399dba934f48ad8b55f9fb9ead2 Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Thu, 30 Jul 2026 23:53:05 +0800 Subject: [PATCH 1/3] Fix CodeQL control flow and API warnings --- .../rabbitstream/RabbitSequenceNumber.java | 2 + .../expressions/BloomFilterExpressions.java | 6 ++ .../autoscaling/ec2/EC2AutoScalerTest.java | 2 - .../histogram/ApproximateHistogram.java | 4 +- .../kafka/KafkaDataSourceMetadata.java | 5 +- .../indexing/kafka/KafkaSequenceNumber.java | 2 + .../kinesis/KinesisSequenceNumber.java | 2 + .../druid/indexing/common/task/Tasks.java | 2 + .../SeekableStreamIndexTaskRunnerTest.java | 2 + .../SeekableStreamSupervisorStateTest.java | 2 + .../SeekableStreamSupervisorTestBase.java | 2 + .../druid/msq/exec/ControllerHolder.java | 3 + .../SegmentGeneratorFrameProcessor.java | 2 + .../BaseControllerQueryKernelTest.java | 3 + .../druid/extendedset/intset/ConciseSet.java | 12 ++- .../druid/frame/field/FieldWriters.java | 3 + .../util/common/guava/FunctionalIterator.java | 2 + .../druid/math/expr/ExpressionType.java | 9 ++ .../math/expr/ExpressionTypeFactory.java | 3 + .../query/PrioritizedExecutorService.java | 2 + .../druid/query/filter/InDimFilter.java | 4 +- .../ByteBufferMinMaxOffsetHeap.java | 6 +- .../query/ordering/StringComparators.java | 12 ++- .../TopNColumnAggregatesProcessorFactory.java | 4 + .../segment/data/CompressedBlockReader.java | 1 - .../druid/segment/data/VSizeColumnarInts.java | 19 ++++ .../segment/filter/ExpressionFilter.java | 3 + .../SpatialDimensionRowTransformer.java | 4 +- .../druid/collections/IntSetTestUtility.java | 52 +--------- .../util/common/guava/ConcatSequenceTest.java | 5 +- .../expr/VectorExprResultConsistencyTest.java | 3 + .../druid/query/scan/ScanQueryRunnerTest.java | 2 +- .../query/search/SearchQueryRunnerTest.java | 1 - .../segment/column/TypeStrategiesTest.java | 20 +++- .../segment/transform/RowFunctionTest.java | 48 +-------- .../segment/transform/TransformerTest.java | 55 ++--------- .../HashBasedNumberedShardSpecTest.java | 26 ++++- .../druid/metadata/BasicDataSourceExt.java | 2 + .../druid/server/QueryResultPusher.java | 4 + .../rpc/NoDelayScheduledExecutorService.java | 26 ++++- .../druid/segment/realtime/sink/SinkTest.java | 97 +------------------ .../coordinator/duty/CompactSegmentsTest.java | 6 +- .../WrappingScheduledExecutorService.java | 26 ++++- sql/src/main/codegen/templates/Parser.jj | 10 ++ 44 files changed, 231 insertions(+), 275 deletions(-) diff --git a/extensions-contrib/rabbit-stream-indexing-service/src/main/java/org/apache/druid/indexing/rabbitstream/RabbitSequenceNumber.java b/extensions-contrib/rabbit-stream-indexing-service/src/main/java/org/apache/druid/indexing/rabbitstream/RabbitSequenceNumber.java index 0f5197d7a55b..8ddc005fe8f3 100644 --- a/extensions-contrib/rabbit-stream-indexing-service/src/main/java/org/apache/druid/indexing/rabbitstream/RabbitSequenceNumber.java +++ b/extensions-contrib/rabbit-stream-indexing-service/src/main/java/org/apache/druid/indexing/rabbitstream/RabbitSequenceNumber.java @@ -25,6 +25,8 @@ // OrderedSequenceNumber.equals() should be used instead. @SuppressWarnings("ComparableImplementedButEqualsNotOverridden") +// Every Rabbit sequence number is inclusive, so inherited equality and value-only ordering are consistent. +// codeql[java/inconsistent-compareto-and-equals] public class RabbitSequenceNumber extends OrderedSequenceNumber { private RabbitSequenceNumber(Long sequenceNumber) diff --git a/extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/expressions/BloomFilterExpressions.java b/extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/expressions/BloomFilterExpressions.java index 59f2b3507279..72e3a830eb0c 100644 --- a/extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/expressions/BloomFilterExpressions.java +++ b/extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/expressions/BloomFilterExpressions.java @@ -228,6 +228,9 @@ public ExprEval eval(final ObjectBinding bindings) matches = filter.testLong(longVal); } break; + case ARRAY: + case COMPLEX: + break; } return ExprEval.ofLongBoolean(matches); @@ -293,6 +296,9 @@ public ExprEval eval(final ObjectBinding bindings) matches = filter.testLong(longVal); } break; + case ARRAY: + case COMPLEX: + break; } return ExprEval.ofLongBoolean(matches); diff --git a/extensions-core/ec2-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/ec2/EC2AutoScalerTest.java b/extensions-core/ec2-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/ec2/EC2AutoScalerTest.java index 9fb8f8eff75e..1807c2809639 100644 --- a/extensions-core/ec2-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/ec2/EC2AutoScalerTest.java +++ b/extensions-core/ec2-extensions/src/test/java/org/apache/druid/indexing/overlord/autoscaling/ec2/EC2AutoScalerTest.java @@ -140,7 +140,6 @@ public void testIptoIdLookup() ); final int n = 150; - Assert.assertTrue(n <= 2 * EC2AutoScaler.MAX_AWS_FILTER_VALUES); List ips = Lists.transform( ContiguousSet.create(Range.closedOpen(0, n), DiscreteDomain.integers()).asList(), @@ -193,7 +192,6 @@ public void testIdToIpLookup() ); final int n = 150; - Assert.assertTrue(n <= 2 * EC2AutoScaler.MAX_AWS_FILTER_VALUES); List ids = Lists.transform( ContiguousSet.create(Range.closedOpen(0, n), DiscreteDomain.integers()).asList(), 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..8f72d3046ee7 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 @@ -747,17 +747,15 @@ protected int ruleCombineBins( // if there are values below the lower limit, fill in array position 1 // else array position 0 - while (j != leftBinCount || k != rightBinCount) { + if (j != leftBinCount || k != rightBinCount) { if (j != leftBinCount && (k == rightBinCount || leftPositions[j] < rightPositions[k])) { mergedPositions[pos] = leftPositions[j]; mergedBins[pos] = leftBins[j]; ++j; - break; } else { mergedPositions[pos] = rightPositions[k]; mergedBins[pos] = rightBins[k]; ++k; - break; } } diff --git a/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaDataSourceMetadata.java b/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaDataSourceMetadata.java index 04a6c2ba9285..d8b05c69c19c 100644 --- a/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaDataSourceMetadata.java +++ b/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaDataSourceMetadata.java @@ -37,7 +37,10 @@ import java.util.Comparator; import java.util.Map; -public class KafkaDataSourceMetadata extends SeekableStreamDataSourceMetadata implements Comparable +// Natural ordering intentionally compares offsets only; inherited equality also includes the non-ordering stream config. +// codeql[java/inconsistent-compareto-and-equals] +public class KafkaDataSourceMetadata extends SeekableStreamDataSourceMetadata + implements Comparable { private static final Logger LOGGER = new Logger(KafkaDataSourceMetadata.class); diff --git a/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaSequenceNumber.java b/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaSequenceNumber.java index d727e5a7baf4..1a21c6fe8a7a 100644 --- a/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaSequenceNumber.java +++ b/extensions-core/kafka-indexing-service/src/main/java/org/apache/druid/indexing/kafka/KafkaSequenceNumber.java @@ -25,6 +25,8 @@ // OrderedSequenceNumber.equals() should be used instead. @SuppressWarnings("ComparableImplementedButEqualsNotOverridden") +// Every Kafka sequence number is inclusive, so inherited equality and value-only ordering are consistent. +// codeql[java/inconsistent-compareto-and-equals] public class KafkaSequenceNumber extends OrderedSequenceNumber { private KafkaSequenceNumber(Long sequenceNumber) diff --git a/extensions-core/kinesis-indexing-service/src/main/java/org/apache/druid/indexing/kinesis/KinesisSequenceNumber.java b/extensions-core/kinesis-indexing-service/src/main/java/org/apache/druid/indexing/kinesis/KinesisSequenceNumber.java index ce5025238e6c..f6c79f4896f1 100644 --- a/extensions-core/kinesis-indexing-service/src/main/java/org/apache/druid/indexing/kinesis/KinesisSequenceNumber.java +++ b/extensions-core/kinesis-indexing-service/src/main/java/org/apache/druid/indexing/kinesis/KinesisSequenceNumber.java @@ -26,6 +26,8 @@ // OrderedSequenceNumber.equals() should be used instead. @SuppressWarnings("ComparableImplementedButEqualsNotOverridden") +// Ordering intentionally groups equivalent unread/end markers while inherited equality preserves their identities. +// codeql[java/inconsistent-compareto-and-equals] public class KinesisSequenceNumber extends OrderedSequenceNumber { /** diff --git a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/Tasks.java b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/Tasks.java index 5e09198828e7..fa2b0af0bc01 100644 --- a/indexing-service/src/main/java/org/apache/druid/indexing/common/task/Tasks.java +++ b/indexing-service/src/main/java/org/apache/druid/indexing/common/task/Tasks.java @@ -39,6 +39,8 @@ public class Tasks public static final int DEFAULT_EMBEDDED_KILL_TASK_PRIORITY = 25; static { + // Keep the independently defined indexing and compaction defaults aligned when either constant changes. + // codeql[java/constant-comparison] Verify.verify(DEFAULT_MERGE_TASK_PRIORITY == DataSourceCompactionConfig.DEFAULT_COMPACTION_TASK_PRIORITY); } diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskRunnerTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskRunnerTest.java index d4cf8bb8961d..0fc173c7d249 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskRunnerTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/SeekableStreamIndexTaskRunnerTest.java @@ -1118,6 +1118,8 @@ protected OrderedSequenceNumber createSequenceNumber(Object sequenceNumber) if (sequenceNumber == null) { return null; } + // Offset ordering intentionally excludes boundary exclusivity, which value equality includes. + // codeql[java/inconsistent-compareto-and-equals] return new OrderedSequenceNumber<>(sequenceNumber.toString(), false) { @Override diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorStateTest.java b/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorStateTest.java index 695a916383e8..876cf7a6c31f 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorStateTest.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorStateTest.java @@ -3404,6 +3404,8 @@ public SeekableStreamDataSourceMetadata createDataSourceMetaData @Override protected OrderedSequenceNumber makeSequenceNumber(String seq, boolean isExclusive) { + // Offset ordering intentionally excludes boundary exclusivity, which value equality includes. + // codeql[java/inconsistent-compareto-and-equals] return new OrderedSequenceNumber<>(seq, isExclusive) { @Override diff --git a/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorTestBase.java b/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorTestBase.java index 968e8a75092d..c60de2e941cc 100644 --- a/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorTestBase.java +++ b/indexing-service/src/test/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorTestBase.java @@ -215,6 +215,8 @@ public SeekableStreamDataSourceMetadata createDataSourceMetaData @Override protected OrderedSequenceNumber makeSequenceNumber(String seq, boolean isExclusive) { + // Offset ordering intentionally excludes boundary exclusivity, which value equality includes. + // codeql[java/inconsistent-compareto-and-equals] return new OrderedSequenceNumber<>(seq, isExclusive) { @Override diff --git a/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerHolder.java b/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerHolder.java index 5e0af589a86a..7dedebe5471f 100644 --- a/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerHolder.java +++ b/multi-stage-query/src/main/java/org/apache/druid/msq/exec/ControllerHolder.java @@ -348,6 +348,9 @@ private synchronized void updateStateOnQueryComplete(final MSQTaskReportPayload case FAILED: state = State.FAILED; break; + + case RUNNING: + break; } } diff --git a/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/processor/SegmentGeneratorFrameProcessor.java b/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/processor/SegmentGeneratorFrameProcessor.java index e37c755bdd55..4ce0b2cb5b3d 100644 --- a/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/processor/SegmentGeneratorFrameProcessor.java +++ b/multi-stage-query/src/main/java/org/apache/druid/msq/indexing/processor/SegmentGeneratorFrameProcessor.java @@ -218,6 +218,8 @@ private void addFrame(final Frame frame) } } + // Input rows are never ordered during indexing, and compareTo always rejects attempts to do so. + // codeql[java/inconsistent-compareto-and-equals] private class MSQInputRow implements InputRow { private final Object[] backingArray; diff --git a/multi-stage-query/src/test/java/org/apache/druid/msq/kernel/controller/BaseControllerQueryKernelTest.java b/multi-stage-query/src/test/java/org/apache/druid/msq/kernel/controller/BaseControllerQueryKernelTest.java index bb8023820457..41f160fcc450 100644 --- a/multi-stage-query/src/test/java/org/apache/druid/msq/kernel/controller/BaseControllerQueryKernelTest.java +++ b/multi-stage-query/src/test/java/org/apache/druid/msq/kernel/controller/BaseControllerQueryKernelTest.java @@ -210,6 +210,9 @@ public ControllerQueryKernelTester setupStage( case FAILED: controllerQueryKernel.failStage(stageId); break; + + case RETRYING: + throw new IAE("Cannot initialize a stage directly in the retrying phase"); } if (!recursiveCall) { setupStages.add(stageNumber); diff --git a/processing/src/main/java/org/apache/druid/extendedset/intset/ConciseSet.java b/processing/src/main/java/org/apache/druid/extendedset/intset/ConciseSet.java index 5bce5616a7b5..c197da823d0f 100755 --- a/processing/src/main/java/org/apache/druid/extendedset/intset/ConciseSet.java +++ b/processing/src/main/java/org/apache/druid/extendedset/intset/ConciseSet.java @@ -657,14 +657,16 @@ private ConciseSet performOperation(ConciseSet other, Operator operator) if (!otherItr.isLiteral) { int minCount = Math.min(thisItr.count, otherItr.count); res.appendFill(minCount, operator.combineLiterals(thisItr.word, otherItr.word)); - //noinspection NonShortCircuitBooleanExpression + // Both iterators must advance before testing whether either is exhausted. + // codeql[java/non-short-circuit-evaluation] if (!thisItr.prepareNext(minCount) | /* NOT || */ !otherItr.prepareNext(minCount)) { break; } } else { res.appendLiteral(operator.combineLiterals(thisItr.toLiteral(), otherItr.word)); thisItr.word--; - //noinspection NonShortCircuitBooleanExpression + // Both iterators must advance before testing whether either is exhausted. + // codeql[java/non-short-circuit-evaluation] if (!thisItr.prepareNext(1) | /* do NOT use "||" */ !otherItr.prepareNext()) { break; } @@ -672,13 +674,15 @@ private ConciseSet performOperation(ConciseSet other, Operator operator) } else if (!otherItr.isLiteral) { res.appendLiteral(operator.combineLiterals(thisItr.word, otherItr.toLiteral())); otherItr.word--; - //noinspection NonShortCircuitBooleanExpression + // Both iterators must advance before testing whether either is exhausted. + // codeql[java/non-short-circuit-evaluation] if (!thisItr.prepareNext() | /* do NOT use "||" */ !otherItr.prepareNext(1)) { break; } } else { res.appendLiteral(operator.combineLiterals(thisItr.word, otherItr.word)); - //noinspection NonShortCircuitBooleanExpression + // Both iterators must advance before testing whether either is exhausted. + // codeql[java/non-short-circuit-evaluation] if (!thisItr.prepareNext() | /* do NOT use "||" */ !otherItr.prepareNext()) { break; } diff --git a/processing/src/main/java/org/apache/druid/frame/field/FieldWriters.java b/processing/src/main/java/org/apache/druid/frame/field/FieldWriters.java index 58d325f5a208..3df53e83caf9 100644 --- a/processing/src/main/java/org/apache/druid/frame/field/FieldWriters.java +++ b/processing/src/main/java/org/apache/druid/frame/field/FieldWriters.java @@ -93,6 +93,9 @@ public static FieldWriter create( return makeFloatArrayWriter(columnSelectorFactory, columnName, frameType); case DOUBLE: return makeDoubleArrayWriter(columnSelectorFactory, columnName, frameType); + case ARRAY: + case COMPLEX: + throw new UnsupportedColumnTypeException(columnName, columnType); } default: throw new UnsupportedColumnTypeException(columnName, columnType); diff --git a/processing/src/main/java/org/apache/druid/java/util/common/guava/FunctionalIterator.java b/processing/src/main/java/org/apache/druid/java/util/common/guava/FunctionalIterator.java index fa06f25ac1bc..20527a7f5218 100644 --- a/processing/src/main/java/org/apache/druid/java/util/common/guava/FunctionalIterator.java +++ b/processing/src/main/java/org/apache/druid/java/util/common/guava/FunctionalIterator.java @@ -59,6 +59,8 @@ public T next() @Override public void remove() { + // Iterator.remove is optional; preserve the removal capability of the wrapped iterator. + // codeql[java/iterator-remove-failure] delegate.remove(); } diff --git a/processing/src/main/java/org/apache/druid/math/expr/ExpressionType.java b/processing/src/main/java/org/apache/druid/math/expr/ExpressionType.java index 42f322120978..9d993baa7d29 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/ExpressionType.java +++ b/processing/src/main/java/org/apache/druid/math/expr/ExpressionType.java @@ -103,6 +103,9 @@ public static ExpressionType asArrayType(@Nullable ExpressionType elementType) return LONG_ARRAY; case DOUBLE: return DOUBLE_ARRAY; + case ARRAY: + case COMPLEX: + return elementType; } } return elementType; @@ -138,6 +141,9 @@ public static ExpressionType fromColumnTypeStrict(@Nullable TypeSignature v return DOUBLE_ARRAY; case STRING: return STRING_ARRAY; + case ARRAY: + case COMPLEX: + break; } return ExpressionTypeFactory.getInstance().ofArray(fromColumnType(valueType.getElementType())); case COMPLEX: diff --git a/processing/src/main/java/org/apache/druid/math/expr/ExpressionTypeFactory.java b/processing/src/main/java/org/apache/druid/math/expr/ExpressionTypeFactory.java index 29b8edd5aebb..03f77b56a9ee 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/ExpressionTypeFactory.java +++ b/processing/src/main/java/org/apache/druid/math/expr/ExpressionTypeFactory.java @@ -79,6 +79,9 @@ public ExpressionType ofArray(ExpressionType elementType) return ExpressionType.DOUBLE_ARRAY; case LONG: return ExpressionType.LONG_ARRAY; + case ARRAY: + case COMPLEX: + break; } } return INTERNER.intern(new ExpressionType(ExprType.ARRAY, null, elementType)); diff --git a/processing/src/main/java/org/apache/druid/query/PrioritizedExecutorService.java b/processing/src/main/java/org/apache/druid/query/PrioritizedExecutorService.java index af14e4cc83bf..92844f4f9599 100644 --- a/processing/src/main/java/org/apache/druid/query/PrioritizedExecutorService.java +++ b/processing/src/main/java/org/apache/druid/query/PrioritizedExecutorService.java @@ -213,6 +213,8 @@ public int getActiveTasks() } } +// Tasks with equal scheduling keys remain distinct futures, so identity equality is intentional. +// codeql[java/inconsistent-compareto-and-equals] class PrioritizedListenableFutureTask implements RunnableFuture, ListenableFuture, PrioritizedRunnable, diff --git a/processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java b/processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java index 67eb8f01e9fa..7bc9137782f3 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java +++ b/processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java @@ -141,7 +141,7 @@ public InDimFilter(String dimension, Set values) { this( dimension, - values instanceof ValuesSet ? (ValuesSet) values : new ValuesSet(values), + values instanceof ValuesSet valuesSet ? valuesSet : new ValuesSet(values), null, null, null @@ -161,7 +161,7 @@ public InDimFilter(String dimension, Collection values, @Nullable Extrac { this( dimension, - values instanceof ValuesSet ? (ValuesSet) values : new ValuesSet(values), + values instanceof ValuesSet valuesSet ? valuesSet : new ValuesSet(values), extractionFn, null, null diff --git a/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/ByteBufferMinMaxOffsetHeap.java b/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/ByteBufferMinMaxOffsetHeap.java index ff2746bca29c..68b2b1a466c4 100644 --- a/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/ByteBufferMinMaxOffsetHeap.java +++ b/processing/src/main/java/org/apache/druid/query/groupby/epinephelinae/ByteBufferMinMaxOffsetHeap.java @@ -308,10 +308,8 @@ private void siftDown(Comparator comparator, int pos) int minGcOffset = buf.getInt(minGrandchild * Integer.BYTES); int cmp = comparator.compare(minChildOffset, minGcOffset); minIndex = (cmp > 0) ? minGrandchild : minChild; - } else if (minChild > -1) { - minIndex = minChild; } else { - break; + minIndex = minChild; } if (minIndex == minGrandchild) { int offset = buf.getInt(pos * Integer.BYTES); @@ -337,6 +335,8 @@ private void siftDown(Comparator comparator, int pos) } } minChild = findMinChild(comparator, minIndex); + } else { + break; } pos = minIndex; } else { 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..a2480335dcc8 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 @@ -68,7 +68,8 @@ 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 + // Identity is only a fast path; ORDERING performs the content comparison for distinct strings. + // codeql[java/reference-equality-on-strings] if (s == s2) { return 0; } @@ -311,7 +312,8 @@ public int compare(String s, String s2) public int compare(String s, String s2) { // Optimization - //noinspection StringEquality + // Identity is only a fast path; ORDERING performs the content comparison for distinct strings. + // codeql[java/reference-equality-on-strings] if (s == s2) { return 0; } @@ -373,7 +375,8 @@ 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 + // Identity is only a fast path; numeric and lexical comparison handles distinct strings. + // codeql[java/reference-equality-on-strings] if (o1 == o2) { return 0; } @@ -450,7 +453,8 @@ public static class VersionComparator extends StringComparator @Override public int compare(String o1, String o2) { - //noinspection StringEquality + // Identity is only a fast path; version comparison handles distinct strings. + // codeql[java/reference-equality-on-strings] if (o1 == o2) { return 0; } diff --git a/processing/src/main/java/org/apache/druid/query/topn/types/TopNColumnAggregatesProcessorFactory.java b/processing/src/main/java/org/apache/druid/query/topn/types/TopNColumnAggregatesProcessorFactory.java index e921e1231f83..be3ba1c8f97e 100644 --- a/processing/src/main/java/org/apache/druid/query/topn/types/TopNColumnAggregatesProcessorFactory.java +++ b/processing/src/main/java/org/apache/druid/query/topn/types/TopNColumnAggregatesProcessorFactory.java @@ -74,6 +74,10 @@ public TopNColumnAggregatesProcessor makeColumnSelectorStrategy( return new FloatTopNColumnAggregatesProcessor(converter); case DOUBLE: return new DoubleTopNColumnAggregatesProcessor(converter); + case STRING: + case ARRAY: + case COMPLEX: + break; } } diff --git a/processing/src/main/java/org/apache/druid/segment/data/CompressedBlockReader.java b/processing/src/main/java/org/apache/druid/segment/data/CompressedBlockReader.java index 951a3cca447f..78138a5aa538 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/CompressedBlockReader.java +++ b/processing/src/main/java/org/apache/druid/segment/data/CompressedBlockReader.java @@ -68,7 +68,6 @@ public static Supplier fromByteBuffer( if (versionFromBuffer == VERSION) { final CompressionStrategy compression = CompressionStrategy.forId(buffer.get()); final int blockSize = buffer.getInt(); - assert CompressedPools.BUFFER_SIZE == blockSize; Preconditions.checkState( blockSize <= CompressedPools.BUFFER_SIZE, "Maximum block size must be less than " + CompressedPools.BUFFER_SIZE diff --git a/processing/src/main/java/org/apache/druid/segment/data/VSizeColumnarInts.java b/processing/src/main/java/org/apache/druid/segment/data/VSizeColumnarInts.java index 7ce51492bd8f..fc62d469b4ff 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/VSizeColumnarInts.java +++ b/processing/src/main/java/org/apache/druid/segment/data/VSizeColumnarInts.java @@ -150,6 +150,25 @@ public int compareTo(VSizeColumnarInts o) return retVal; } + @Override + public boolean equals(Object o) + { + if (this == o) { + return true; + } + if (!(o instanceof VSizeColumnarInts)) { + return false; + } + final VSizeColumnarInts that = (VSizeColumnarInts) o; + return numBytes == that.numBytes && buffer.equals(that.buffer); + } + + @Override + public int hashCode() + { + return 31 * numBytes + buffer.hashCode(); + } + public int getNumBytes() { return numBytes; diff --git a/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java index 1864e2551707..28b6b501a1d5 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java @@ -175,6 +175,9 @@ public boolean matches(boolean includeUnknown) } return Arrays.stream(result).filter(Objects::nonNull).anyMatch(o -> Evals.asBoolean((double) o)); + case ARRAY: + case COMPLEX: + break; } } return eval.asBoolean(); diff --git a/processing/src/main/java/org/apache/druid/segment/incremental/SpatialDimensionRowTransformer.java b/processing/src/main/java/org/apache/druid/segment/incremental/SpatialDimensionRowTransformer.java index 27b908126280..f089fd09f8eb 100644 --- a/processing/src/main/java/org/apache/druid/segment/incremental/SpatialDimensionRowTransformer.java +++ b/processing/src/main/java/org/apache/druid/segment/incremental/SpatialDimensionRowTransformer.java @@ -98,7 +98,9 @@ public boolean apply(String input) ) ); - InputRow retVal = new InputRow() + // Rows are ordered by timestamp, while equality retains the wrapped row's identity semantics. + // codeql[java/inconsistent-compareto-and-equals] + final InputRow retVal = new InputRow() { @Override public List getDimensions() diff --git a/processing/src/test/java/org/apache/druid/collections/IntSetTestUtility.java b/processing/src/test/java/org/apache/druid/collections/IntSetTestUtility.java index 7a18d44c03f0..1e3fedf21ada 100644 --- a/processing/src/test/java/org/apache/druid/collections/IntSetTestUtility.java +++ b/processing/src/test/java/org/apache/druid/collections/IntSetTestUtility.java @@ -27,7 +27,6 @@ import java.util.BitSet; import java.util.HashSet; -import java.util.Iterator; import java.util.Set; /** @@ -61,54 +60,11 @@ public static void addAllToMutable(MutableBitmap mutableBitmap, Iterable s1, ImmutableBitmap s2) { - Set s3 = new HashSet<>(); - for (Integer i : new IntIt(s2.iterator())) { - s3.add(i); + final Set s3 = new HashSet<>(); + final IntIterator iterator = s2.iterator(); + while (iterator.hasNext()) { + s3.add(iterator.next()); } return Sets.difference(s1, s3).isEmpty(); } - - private static class IntIt implements Iterable - { - private final Iterator intIter; - - public IntIt(IntIterator intIt) - { - this.intIter = new IntIter(intIt); - } - - @Override - public Iterator iterator() - { - return intIter; - } - - private static class IntIter implements Iterator - { - private final IntIterator intIt; - - public IntIter(IntIterator intIt) - { - this.intIt = intIt; - } - - @Override - public boolean hasNext() - { - return intIt.hasNext(); - } - - @Override - public Integer next() - { - return intIt.next(); - } - - @Override - public void remove() - { - throw new UnsupportedOperationException("Cannot remove ints from int iterator"); - } - } - } } diff --git a/processing/src/test/java/org/apache/druid/java/util/common/guava/ConcatSequenceTest.java b/processing/src/test/java/org/apache/druid/java/util/common/guava/ConcatSequenceTest.java index 1e2bb56690c2..68e4e4aa5528 100644 --- a/processing/src/test/java/org/apache/druid/java/util/common/guava/ConcatSequenceTest.java +++ b/processing/src/test/java/org/apache/druid/java/util/common/guava/ConcatSequenceTest.java @@ -216,17 +216,16 @@ public Sequence apply(final ImmutableList input) return Sequences.simple( new Iterable<>() { - private Iterator baseIter = input.iterator(); - @Override public Iterator iterator() { + final Iterator baseIter = input.iterator(); return new Iterator<>() { @Override public boolean hasNext() { - boolean result = baseIter.hasNext(); + final boolean result = baseIter.hasNext(); if (!result) { lastSeqFullyRead.set(true); } diff --git a/processing/src/test/java/org/apache/druid/math/expr/VectorExprResultConsistencyTest.java b/processing/src/test/java/org/apache/druid/math/expr/VectorExprResultConsistencyTest.java index 9ffe5da3ace8..8d06ffdc7ea7 100644 --- a/processing/src/test/java/org/apache/druid/math/expr/VectorExprResultConsistencyTest.java +++ b/processing/src/test/java/org/apache/druid/math/expr/VectorExprResultConsistencyTest.java @@ -965,6 +965,9 @@ static NonnullPair populateBindin } vectorBinding.addString(entry.getKey(), strings); break; + case ARRAY: + case COMPLEX: + throw new IllegalArgumentException("Unsupported vector binding type: " + entry.getValue()); } } diff --git a/processing/src/test/java/org/apache/druid/query/scan/ScanQueryRunnerTest.java b/processing/src/test/java/org/apache/druid/query/scan/ScanQueryRunnerTest.java index b19312384ad1..35e57b66ae8c 100644 --- a/processing/src/test/java/org/apache/druid/query/scan/ScanQueryRunnerTest.java +++ b/processing/src/test/java/org/apache/druid/query/scan/ScanQueryRunnerTest.java @@ -941,7 +941,7 @@ public static List>> toEvents(final String[] dimSpecs, Map event = new HashMap<>(); String[] values1 = input.split("\\t"); for (int i = 0; i < dimSpecs.length; i++) { - if (dimSpecs[i] == null || i >= dimSpecs.length) { + if (dimSpecs[i] == null) { continue; } diff --git a/processing/src/test/java/org/apache/druid/query/search/SearchQueryRunnerTest.java b/processing/src/test/java/org/apache/druid/query/search/SearchQueryRunnerTest.java index 1b0012560bb6..cb2df0c47843 100644 --- a/processing/src/test/java/org/apache/druid/query/search/SearchQueryRunnerTest.java +++ b/processing/src/test/java/org/apache/druid/query/search/SearchQueryRunnerTest.java @@ -868,7 +868,6 @@ private void checkSearchQuery(Query searchQuery, QueryRunner runner, List copy = new ArrayList<>(expectedResults); for (Result result : results) { Assert.assertEquals(DateTimes.of("2011-01-12T00:00:00.000Z"), result.getTimestamp()); - Assert.assertTrue(result.getValue() instanceof Iterable); Iterable resultValues = result.getValue(); for (SearchHit resultValue : resultValues) { diff --git a/processing/src/test/java/org/apache/druid/segment/column/TypeStrategiesTest.java b/processing/src/test/java/org/apache/druid/segment/column/TypeStrategiesTest.java index 857a6d6a854a..1b73b4f2f57d 100644 --- a/processing/src/test/java/org/apache/druid/segment/column/TypeStrategiesTest.java +++ b/processing/src/test/java/org/apache/druid/segment/column/TypeStrategiesTest.java @@ -20,7 +20,6 @@ package org.apache.druid.segment.column; import com.google.common.collect.Ordering; -import com.google.common.primitives.Longs; import org.apache.druid.guice.BuiltInTypesModule; import org.apache.druid.java.util.common.IAE; import org.apache.druid.java.util.common.Pair; @@ -675,9 +674,24 @@ public NullableLongPair(@Nullable Long lhs, @Nullable Long rhs) } @Override - public int compareTo(NullableLongPair o) + public boolean equals(final Object o) { - return Comparators.naturalNullsFirst().thenComparing(Longs::compare).compare(this.lhs, o.lhs); + return super.equals(o); + } + + @Override + public int hashCode() + { + return super.hashCode(); + } + + @Override + public int compareTo(final NullableLongPair o) + { + final int lhsComparison = Comparators.naturalNullsFirst().compare(lhs, o.lhs); + return lhsComparison != 0 + ? lhsComparison + : Comparators.naturalNullsFirst().compare(rhs, o.rhs); } } diff --git a/processing/src/test/java/org/apache/druid/segment/transform/RowFunctionTest.java b/processing/src/test/java/org/apache/druid/segment/transform/RowFunctionTest.java index fa7527a3ddfc..9bf892c72fe1 100644 --- a/processing/src/test/java/org/apache/druid/segment/transform/RowFunctionTest.java +++ b/processing/src/test/java/org/apache/druid/segment/transform/RowFunctionTest.java @@ -19,15 +19,14 @@ package org.apache.druid.segment.transform; +import com.google.common.collect.ImmutableMap; +import org.apache.druid.data.input.MapBasedRow; import org.apache.druid.data.input.Row; import org.apache.druid.data.input.Rows; -import org.joda.time.DateTime; +import org.apache.druid.java.util.common.DateTimes; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -import javax.annotation.Nullable; -import java.util.List; - public class RowFunctionTest implements RowFunction { @Override @@ -39,46 +38,7 @@ public Object eval(Row row) @Test public void defaultEvalDimensionTest() { - Row row = new Row() - { - @Override - public long getTimestampFromEpoch() - { - return 0; - } - - @Override - public DateTime getTimestamp() - { - return null; - } - - @Override - public List getDimension(String dimension) - { - return null; - } - - @Nullable - @Override - public Object getRaw(String dimension) - { - return dimension; - } - - @Nullable - @Override - public Number getMetric(String metric) - { - return null; - } - - @Override - public int compareTo(Row o) - { - return 0; - } - }; + final Row row = new MapBasedRow(DateTimes.EPOCH, ImmutableMap.of()); Assertions.assertEquals(Rows.objectToStrings(eval(row)), evalDimension(row)); } } diff --git a/processing/src/test/java/org/apache/druid/segment/transform/TransformerTest.java b/processing/src/test/java/org/apache/druid/segment/transform/TransformerTest.java index 1f8ec37ddf6f..0d990a850146 100644 --- a/processing/src/test/java/org/apache/druid/segment/transform/TransformerTest.java +++ b/processing/src/test/java/org/apache/druid/segment/transform/TransformerTest.java @@ -26,7 +26,6 @@ import org.apache.druid.data.input.InputRowListPlusRawValues; import org.apache.druid.data.input.MapBasedInputRow; import org.apache.druid.data.input.MapBasedRow; -import org.apache.druid.data.input.Row; import org.apache.druid.error.DruidExceptionMatcher; import org.apache.druid.java.util.common.CloseableIterators; import org.apache.druid.java.util.common.DateTimes; @@ -42,7 +41,6 @@ import org.junit.Test; import org.junit.rules.ExpectedException; -import javax.annotation.Nullable; import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; @@ -518,52 +516,13 @@ public void testTransformWithArrayExpr() Assert.assertEquals(row.getDimension("dim"), dimList); Assert.assertEquals(row.getRaw("dim"), dimList); - final InputRow actualTranformedRow = transformer.transform(new InputRow() - { - @Override - public List getDimensions() - { - return new ArrayList<>(row.getEvent().keySet()); - } - - @Override - public long getTimestampFromEpoch() - { - return 0; - } - - @Override - public DateTime getTimestamp() - { - return row.getTimestamp(); - } - - @Override - public List getDimension(String dimension) - { - return row.getDimension(dimension); - } - - @Nullable - @Override - public Object getRaw(String dimension) - { - return row.getRaw(dimension); - } - - @Nullable - @Override - public Number getMetric(String metric) - { - return row.getMetric(metric); - } - - @Override - public int compareTo(Row o) - { - return row.compareTo(o); - } - }); + final InputRow actualTranformedRow = transformer.transform( + new MapBasedInputRow( + row.getTimestamp(), + new ArrayList<>(row.getEvent().keySet()), + row.getEvent() + ) + ); Assert.assertEquals(actualTranformedRow.getDimension("dim"), dimList.subList(0, 5)); Assert.assertArrayEquals(dimList.subList(0, 5).toArray(), (Object[]) actualTranformedRow.getRaw("dim")); Assert.assertEquals(ImmutableList.of("a"), actualTranformedRow.getDimension("dim1")); diff --git a/processing/src/test/java/org/apache/druid/timeline/partition/HashBasedNumberedShardSpecTest.java b/processing/src/test/java/org/apache/druid/timeline/partition/HashBasedNumberedShardSpecTest.java index 9996b0aeb82f..b2da88ef5dce 100644 --- a/processing/src/test/java/org/apache/druid/timeline/partition/HashBasedNumberedShardSpecTest.java +++ b/processing/src/test/java/org/apache/druid/timeline/partition/HashBasedNumberedShardSpecTest.java @@ -397,11 +397,24 @@ public static class HashInputRow implements InputRow { private final int hashcode; - HashInputRow(int hashcode) + HashInputRow(final int hashcode) { this.hashcode = hashcode; } + @Override + public boolean equals(final Object o) + { + if (this == o) { + return true; + } + if (!(o instanceof HashInputRow)) { + return false; + } + final HashInputRow that = (HashInputRow) o; + return hashcode == that.hashcode; + } + @Override public int hashCode() { @@ -445,9 +458,16 @@ public Number getMetric(String metric) } @Override - public int compareTo(Row o) + public int compareTo(final Row o) { - return 0; + if (o instanceof HashInputRow) { + return Integer.compare(hashcode, ((HashInputRow) o).hashcode); + } + + final int timestampComparison = Long.compare(getTimestampFromEpoch(), o.getTimestampFromEpoch()); + return timestampComparison != 0 + ? timestampComparison + : getClass().getName().compareTo(o.getClass().getName()); } } diff --git a/server/src/main/java/org/apache/druid/metadata/BasicDataSourceExt.java b/server/src/main/java/org/apache/druid/metadata/BasicDataSourceExt.java index cff9308a6971..1140c7693a10 100644 --- a/server/src/main/java/org/apache/druid/metadata/BasicDataSourceExt.java +++ b/server/src/main/java/org/apache/druid/metadata/BasicDataSourceExt.java @@ -101,6 +101,8 @@ public void setConnectionProperties(String connectionProperties) } @VisibleForTesting + // This test accessor returns the properties tracked by this subclass; the superclass getter is package-private. + // codeql[java/non-overriding-package-private] public Properties getConnectionProperties() { return connectionProperties; diff --git a/server/src/main/java/org/apache/druid/server/QueryResultPusher.java b/server/src/main/java/org/apache/druid/server/QueryResultPusher.java index cb958ea98572..976eb5716852 100644 --- a/server/src/main/java/org/apache/druid/server/QueryResultPusher.java +++ b/server/src/main/java/org/apache/druid/server/QueryResultPusher.java @@ -241,6 +241,10 @@ static void incrementQueryCounterForException( counter.incrementInterrupted(); break; case CAPACITY_EXCEEDED: + case CONFLICT: + case FORBIDDEN: + case NOT_FOUND: + case SERVICE_UNAVAILABLE: case UNSUPPORTED: case UNCATEGORIZED: case DEFENSIVE: diff --git a/server/src/test/java/org/apache/druid/rpc/NoDelayScheduledExecutorService.java b/server/src/test/java/org/apache/druid/rpc/NoDelayScheduledExecutorService.java index 7574be330432..60fab93ed3e1 100644 --- a/server/src/test/java/org/apache/druid/rpc/NoDelayScheduledExecutorService.java +++ b/server/src/test/java/org/apache/druid/rpc/NoDelayScheduledExecutorService.java @@ -30,6 +30,7 @@ import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicLong; /** * Used by {@link ServiceClientImplTest} so retries happen immediately. @@ -75,7 +76,10 @@ public ScheduledFuture scheduleWithFixedDelay(Runnable command, long initialD private static class NoDelayScheduledFuture implements ScheduledFuture { + private static final AtomicLong NEXT_SEQUENCE_NUMBER = new AtomicLong(); + private final Future delegate; + private final long sequenceNumber = NEXT_SEQUENCE_NUMBER.getAndIncrement(); public NoDelayScheduledFuture(final Future delegate) { @@ -89,9 +93,27 @@ public long getDelay(TimeUnit unit) } @Override - public int compareTo(Delayed o) + public int compareTo(final Delayed o) { - return 0; + if (this == o) { + return 0; + } + if (o instanceof NoDelayScheduledFuture) { + return Long.compare(sequenceNumber, ((NoDelayScheduledFuture) o).sequenceNumber); + } + return getClass().getName().compareTo(o.getClass().getName()); + } + + @Override + public boolean equals(final Object o) + { + return this == o; + } + + @Override + public int hashCode() + { + return System.identityHashCode(this); } @Override diff --git a/server/src/test/java/org/apache/druid/segment/realtime/sink/SinkTest.java b/server/src/test/java/org/apache/druid/segment/realtime/sink/SinkTest.java index 99559033ce2a..4e12583b3748 100644 --- a/server/src/test/java/org/apache/druid/segment/realtime/sink/SinkTest.java +++ b/server/src/test/java/org/apache/druid/segment/realtime/sink/SinkTest.java @@ -25,7 +25,6 @@ import com.google.common.collect.Maps; import org.apache.druid.data.input.InputRow; import org.apache.druid.data.input.MapBasedInputRow; -import org.apache.druid.data.input.Row; import org.apache.druid.data.input.impl.ClusteredValueGroupsBaseTableProjectionSpec; import org.apache.druid.data.input.impl.DimensionsSpec; import org.apache.druid.data.input.impl.LongDimensionSchema; @@ -55,13 +54,11 @@ import org.apache.druid.timeline.partition.ShardSpec; import org.apache.druid.utils.CloseableUtils; import org.easymock.EasyMock; -import org.joda.time.DateTime; import org.joda.time.Interval; import org.junit.Assert; import org.junit.Test; import java.io.IOException; -import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -99,52 +96,7 @@ public void testSwap() throws Exception TuningConfig.DEFAULT_APPENDABLE_INDEX.getDefaultMaxBytesInMemory() ); - sink.add( - new InputRow() - { - @Override - public List getDimensions() - { - return new ArrayList<>(); - } - - @Override - public long getTimestampFromEpoch() - { - return DateTimes.of("2013-01-01").getMillis(); - } - - @Override - public DateTime getTimestamp() - { - return DateTimes.of("2013-01-01"); - } - - @Override - public List getDimension(String dimension) - { - return new ArrayList<>(); - } - - @Override - public Number getMetric(String metric) - { - return 0; - } - - @Override - public Object getRaw(String dimension) - { - return null; - } - - @Override - public int compareTo(Row o) - { - return 0; - } - } - ); + sink.add(new MapBasedInputRow(DateTimes.of("2013-01-01"), ImmutableList.of(), ImmutableMap.of())); FireHydrant currHydrant = sink.getCurrHydrant(); Assert.assertEquals(Intervals.of("2013-01-01/PT1M"), currHydrant.getIndex().getInterval()); @@ -152,52 +104,7 @@ public int compareTo(Row o) FireHydrant swapHydrant = sink.swap(); - sink.add( - new InputRow() - { - @Override - public List getDimensions() - { - return new ArrayList<>(); - } - - @Override - public long getTimestampFromEpoch() - { - return DateTimes.of("2013-01-01").getMillis(); - } - - @Override - public DateTime getTimestamp() - { - return DateTimes.of("2013-01-01"); - } - - @Override - public List getDimension(String dimension) - { - return new ArrayList<>(); - } - - @Override - public Number getMetric(String metric) - { - return 0; - } - - @Override - public Object getRaw(String dimension) - { - return null; - } - - @Override - public int compareTo(Row o) - { - return 0; - } - } - ); + sink.add(new MapBasedInputRow(DateTimes.of("2013-01-01"), ImmutableList.of(), ImmutableMap.of())); Assert.assertEquals(currHydrant, swapHydrant); Assert.assertNotSame(currHydrant, sink.getCurrHydrant()); diff --git a/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java b/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java index f1ea4e553dea..ebde1f5b43da 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/duty/CompactSegmentsTest.java @@ -716,8 +716,7 @@ public void testRunMultipleCompactionTaskSlots() @Test public void testRunMultipleCompactionTaskSlotsWithUseAutoScaleSlotsOverMaxSlot() { - int maxCompactionSlot = 3; - Assert.assertTrue(maxCompactionSlot < MAXIMUM_CAPACITY_WITH_AUTO_SCALE); + final int maxCompactionSlot = 3; final TestOverlordClient overlordClient = new TestOverlordClient(JSON_MAPPER); final CompactSegments compactSegments = new CompactSegments(statusTracker, overlordClient); final CoordinatorRunStats stats = @@ -736,8 +735,7 @@ public void testRunMultipleCompactionTaskSlotsWithUseAutoScaleSlotsOverMaxSlot() @Test public void testRunMultipleCompactionTaskSlotsWithUseAutoScaleSlotsUnderMaxSlot() { - int maxCompactionSlot = 100; - Assert.assertFalse(maxCompactionSlot < MAXIMUM_CAPACITY_WITH_AUTO_SCALE); + final int maxCompactionSlot = 100; final TestOverlordClient overlordClient = new TestOverlordClient(JSON_MAPPER); final CompactSegments compactSegments = new CompactSegments(statusTracker, overlordClient); final CoordinatorRunStats stats = diff --git a/server/src/test/java/org/apache/druid/server/coordinator/simulate/WrappingScheduledExecutorService.java b/server/src/test/java/org/apache/druid/server/coordinator/simulate/WrappingScheduledExecutorService.java index 334651ee30f5..a497a1c16401 100644 --- a/server/src/test/java/org/apache/druid/server/coordinator/simulate/WrappingScheduledExecutorService.java +++ b/server/src/test/java/org/apache/druid/server/coordinator/simulate/WrappingScheduledExecutorService.java @@ -33,6 +33,7 @@ import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicLong; /** * Wraps an {@link ExecutorService} into a {@link ScheduledExecutorService}. @@ -188,7 +189,10 @@ public void execute(Runnable command) */ private static class WrappingScheduledFuture implements ScheduledFuture { + private static final AtomicLong NEXT_SEQUENCE_NUMBER = new AtomicLong(); + private final Future future; + private final long sequenceNumber = NEXT_SEQUENCE_NUMBER.getAndIncrement(); private WrappingScheduledFuture(Future future) { @@ -202,9 +206,27 @@ public long getDelay(TimeUnit unit) } @Override - public int compareTo(Delayed o) + public int compareTo(final Delayed o) { - return 0; + if (this == o) { + return 0; + } + if (o instanceof WrappingScheduledFuture) { + return Long.compare(sequenceNumber, ((WrappingScheduledFuture) o).sequenceNumber); + } + return getClass().getName().compareTo(o.getClass().getName()); + } + + @Override + public boolean equals(final Object o) + { + return this == o; + } + + @Override + public int hashCode() + { + return System.identityHashCode(this); } @Override diff --git a/sql/src/main/codegen/templates/Parser.jj b/sql/src/main/codegen/templates/Parser.jj index 0f8ae7e5f6d4..2bd0ae56500e 100644 --- a/sql/src/main/codegen/templates/Parser.jj +++ b/sql/src/main/codegen/templates/Parser.jj @@ -428,6 +428,8 @@ JAVACODE void checkQueryExpression(ExprContext exprContext) case ACCEPT_CURSOR: throw SqlUtil.newContextException(getPos(), RESOURCE.illegalQueryExpression()); + default: + break; } } @@ -437,6 +439,8 @@ JAVACODE void checkNonQueryExpression(ExprContext exprContext) case ACCEPT_QUERY: throw SqlUtil.newContextException(getPos(), RESOURCE.illegalNonQueryExpression()); + default: + break; } } @@ -831,6 +835,8 @@ SqlNode ParenthesizedExpression(ExprContext exprContext) : case ACCEPT_CURSOR: exprContext = ExprContext.ACCEPT_ALL; break; + default: + break; } } e = ExprOrJoinOrOrderedQuery(exprContext) @@ -886,6 +892,8 @@ SqlNodeList ParenthesizedQueryOrCommaList( case ACCEPT_CURSOR: firstExprContext = ExprContext.ACCEPT_ALL; break; + default: + break; } } e = OrderedQueryOrExpr(firstExprContext) { list.add(e); } @@ -927,6 +935,8 @@ SqlNodeList ParenthesizedQueryOrCommaListWithDefault( case ACCEPT_CURSOR: firstExprContext = ExprContext.ACCEPT_ALL; break; + default: + break; } } ( From 7176a3dfc05c732b22cf9853967ad6a486ac53fe Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Fri, 31 Jul 2026 01:13:10 +0800 Subject: [PATCH 2/3] Preserve query counter behavior for HTTP errors --- .../java/org/apache/druid/server/QueryResultPusher.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/QueryResultPusher.java b/server/src/main/java/org/apache/druid/server/QueryResultPusher.java index 976eb5716852..6f0b3ee272a7 100644 --- a/server/src/main/java/org/apache/druid/server/QueryResultPusher.java +++ b/server/src/main/java/org/apache/druid/server/QueryResultPusher.java @@ -241,10 +241,6 @@ static void incrementQueryCounterForException( counter.incrementInterrupted(); break; case CAPACITY_EXCEEDED: - case CONFLICT: - case FORBIDDEN: - case NOT_FOUND: - case SERVICE_UNAVAILABLE: case UNSUPPORTED: case UNCATEGORIZED: case DEFENSIVE: @@ -253,6 +249,11 @@ static void incrementQueryCounterForException( case TIMEOUT: counter.incrementTimedOut(); break; + case CONFLICT: + case FORBIDDEN: + case NOT_FOUND: + case SERVICE_UNAVAILABLE: + break; } } From b68b91ddb3bbd37f67a66066910263177c46fc33 Mon Sep 17 00:00:00 2001 From: Frank Chen Date: Fri, 31 Jul 2026 02:25:13 +0800 Subject: [PATCH 3/3] test: cover VSizeColumnarInts equality --- .../segment/data/VSizeColumnarIntsTest.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/processing/src/test/java/org/apache/druid/segment/data/VSizeColumnarIntsTest.java b/processing/src/test/java/org/apache/druid/segment/data/VSizeColumnarIntsTest.java index fb14ffa2958d..7fe775df04c8 100644 --- a/processing/src/test/java/org/apache/druid/segment/data/VSizeColumnarIntsTest.java +++ b/processing/src/test/java/org/apache/druid/segment/data/VSizeColumnarIntsTest.java @@ -62,4 +62,21 @@ public void testSerialization() throws Exception Assertions.assertEquals(array[i], deserialized.get(i)); } } + + @Test + public void testEqualsAndHashCode() + { + final VSizeColumnarInts ints = VSizeColumnarInts.fromArray(new int[]{1, 2, 3}); + final VSizeColumnarInts equalInts = VSizeColumnarInts.fromArray(new int[]{1, 2, 3}); + final VSizeColumnarInts differentValues = VSizeColumnarInts.fromArray(new int[]{1, 2, 4}); + final VSizeColumnarInts differentWidth = VSizeColumnarInts.fromArray(new int[]{1, 2, 3}, 256); + + Assertions.assertEquals(ints, ints); + Assertions.assertEquals(ints, equalInts); + Assertions.assertEquals(ints.hashCode(), equalInts.hashCode()); + Assertions.assertNotEquals(ints, null); + Assertions.assertNotEquals(ints, "not columnar ints"); + Assertions.assertNotEquals(ints, differentValues); + Assertions.assertNotEquals(ints, differentWidth); + } }