From 53752b72b682fe2ae2d754df26895675335a5839 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Novotn=C3=BD?= Date: Thu, 30 Jan 2025 14:55:52 +0100 Subject: [PATCH] fix: ArrayIndexOutOfBounds in MergedSortedRecordsSupplier Refs: #780 --- .../core/query/sort/CacheableSorter.java | 6 +-- .../core/query/sort/DeferredSorter.java | 16 +++++--- .../MergedSortedRecordsSupplier.java | 39 +++++++++++++++---- 3 files changed, 45 insertions(+), 16 deletions(-) diff --git a/evita_engine/src/main/java/io/evitadb/core/query/sort/CacheableSorter.java b/evita_engine/src/main/java/io/evitadb/core/query/sort/CacheableSorter.java index 3c34625110..9a798d774d 100644 --- a/evita_engine/src/main/java/io/evitadb/core/query/sort/CacheableSorter.java +++ b/evita_engine/src/main/java/io/evitadb/core/query/sort/CacheableSorter.java @@ -6,7 +6,7 @@ * | __/\ V /| | || (_| | |_| | |_) | * \___| \_/ |_|\__\__,_|____/|____/ * - * Copyright (c) 2023-2024 + * Copyright (c) 2023-2025 * * Licensed under the Business Source License, Version 1.1 (the "License"); * you may not use this file except in compliance with the License. @@ -25,7 +25,7 @@ import io.evitadb.api.requestResponse.EvitaResponseExtraResult; import io.evitadb.core.cache.payload.CachePayloadHeader; -import io.evitadb.core.query.QueryPlanningContext; +import io.evitadb.core.query.QueryExecutionContext; import io.evitadb.core.query.algebra.Formula; import io.evitadb.core.query.response.TransactionalDataRelatedStructure; import io.evitadb.core.query.sort.SortedRecordsSupplierFactory.SortedRecordsProvider; @@ -60,7 +60,7 @@ public interface CacheableSorter extends TransactionalDataRelatedStructure, Sort /** * Returns copy of this computer with same configuration but new method handle that references callback that - * will be called when {@link #sortAndSlice(QueryPlanningContext, Formula, int, int, int[], int)} method is first executed + * will be called when {@link #sortAndSlice(QueryExecutionContext, Formula, int, int, int[], int)} method is first executed * and memoized result is available. */ @Nonnull diff --git a/evita_engine/src/main/java/io/evitadb/core/query/sort/DeferredSorter.java b/evita_engine/src/main/java/io/evitadb/core/query/sort/DeferredSorter.java index 5c06c7f88e..ede1e7ea9d 100644 --- a/evita_engine/src/main/java/io/evitadb/core/query/sort/DeferredSorter.java +++ b/evita_engine/src/main/java/io/evitadb/core/query/sort/DeferredSorter.java @@ -6,7 +6,7 @@ * | __/\ V /| | || (_| | |_| | |_) | * \___| \_/ |_|\__\__,_|____/|____/ * - * Copyright (c) 2023-2024 + * Copyright (c) 2023-2025 * * Licensed under the Business Source License, Version 1.1 (the "License"); * you may not use this file except in compliance with the License. @@ -72,9 +72,15 @@ public int sortAndSlice( int peak, @Nullable IntConsumer skippedRecordsConsumer ) { - return executionWrapper.applyAsInt( - () -> ConditionalSorter.getFirstApplicableSorter(queryContext, sorter) - .sortAndSlice(queryContext, input, startIndex, endIndex, result, peak, skippedRecordsConsumer) - ); + final Sorter firstApplicableSorter = ConditionalSorter.getFirstApplicableSorter(queryContext, sorter); + if (firstApplicableSorter == null) { + return 0; + } else { + return executionWrapper.applyAsInt( + () -> firstApplicableSorter.sortAndSlice( + queryContext, input, startIndex, endIndex, result, peak, skippedRecordsConsumer + ) + ); + } } } diff --git a/evita_engine/src/main/java/io/evitadb/core/query/sort/attribute/MergedSortedRecordsSupplier.java b/evita_engine/src/main/java/io/evitadb/core/query/sort/attribute/MergedSortedRecordsSupplier.java index 9e94ef4e42..e647aad3d7 100644 --- a/evita_engine/src/main/java/io/evitadb/core/query/sort/attribute/MergedSortedRecordsSupplier.java +++ b/evita_engine/src/main/java/io/evitadb/core/query/sort/attribute/MergedSortedRecordsSupplier.java @@ -6,7 +6,7 @@ * | __/\ V /| | || (_| | |_| | |_) | * \___| \_/ |_|\__\__,_|____/|____/ * - * Copyright (c) 2023-2024 + * Copyright (c) 2023-2025 * * Licensed under the Business Source License, Version 1.1 (the "License"); * you may not use this file except in compliance with the License. @@ -34,6 +34,7 @@ import io.evitadb.index.bitmap.Bitmap; import io.evitadb.index.bitmap.RoaringBitmapBackedBitmap; import lombok.Getter; +import lombok.RequiredArgsConstructor; import org.roaringbitmap.BatchIterator; import org.roaringbitmap.RoaringBatchIterator; import org.roaringbitmap.RoaringBitmap; @@ -91,12 +92,14 @@ private static PartialSortResult fetchSlice( // skip previous pages quickly if (toSkip > 0) { - toSkip -= bufferPeak; - } - if (skippedRecordsConsumer != null && skip > 0) { - for (int i = 0; i < bufferPeak + toSkip; i++) { - skippedRecordsConsumer.accept(preSortedRecordIds[buffer[i]]); + if (skippedRecordsConsumer != null) { + // skip records in buffer, cap really read records in the buffer + final int skippedInBuffer = Math.max(0, Math.min(toSkip, bufferPeak)); + for (int i = 0; i < skippedInBuffer; i++) { + skippedRecordsConsumer.accept(preSortedRecordIds[buffer[i]]); + } } + toSkip -= bufferPeak; } // now we are on the page @@ -235,13 +238,15 @@ public int sortAndSlice( } else { final int[] buffer = queryContext.borrowBuffer(); try { + final SkippingRecordConsumer delegateConsumer = new SkippingRecordConsumer(skippedRecordsConsumer); final SortResult sortResult = collectPartialResults( - queryContext, selectedRecordIds, startIndex, endIndex, result, peak, buffer, skippedRecordsConsumer + queryContext, selectedRecordIds, startIndex, endIndex, result, peak, buffer, delegateConsumer ); return returnResultAppendingUnknown( queryContext, sortResult.notSortedRecords(), unknownRecordIdsSorter, - startIndex, endIndex, + Math.max(0, startIndex - delegateConsumer.getCounter()), + Math.max(0, endIndex - delegateConsumer.getCounter()), result, sortResult.peak(), buffer ); } finally { @@ -336,4 +341,22 @@ private record SortResult( ) { } + /** + * The SkippingRecordConsumer is an implementation of the {@link IntConsumer} interface that wraps an optional delegate + * {@code IntConsumer} and tracks the count of records processed. This class is intended to be used for scenarios + * where certain records need to be tracked, possibly skipped, or processed with optional side effects. + */ + @RequiredArgsConstructor + private static class SkippingRecordConsumer implements IntConsumer { + @Nullable private final IntConsumer delegate; + @Getter private int counter = 0; + + @Override + public void accept(int value) { + if (this.delegate != null) { + this.delegate.accept(value); + } + this.counter++; + } + } }