This is an automated email from the ASF dual-hosted git repository.
xiangfu0 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new a13a7ce49ab Fix group key generation when
optimizeMaxInitialResultHolderCapacity shrinks the cardinality product (#19379)
a13a7ce49ab is described below
commit a13a7ce49ab64ef9f89ea6440c4c60c6f952ea7c
Author: Xiang Fu <[email protected]>
AuthorDate: Wed Sep 2 17:32:11 2026 -0700
Fix group key generation when optimizeMaxInitialResultHolderCapacity
shrinks the cardinality product (#19379)
With optimizeMaxInitialResultHolderCapacity=true,
DictionaryBasedGroupKeyGenerator used the
IN/EQ-predicate-shrunk cardinality product both for holder type selection
and for the group id
upper bound. ArrayBasedHolder uses raw dictionary-id mixed-radix products
as group ids, so any
matching dictionary id beyond the shrunk bound threw
ArrayIndexOutOfBoundsException, and
resetting longOverflow could downgrade the holder to int/long raw keys that
overflow for the
full cardinalities, silently colliding distinct groups.
Holder type selection now always uses the full cardinality product; the
predicate-derived value
only caps the dense group id upper bound, and ArrayBasedHolder falls back
to IntMapBasedHolder
when the bound shrinks below the product. Multi-value group-by expressions
are excluded from
the predicate-derived bound (every value inside a matching row becomes a
group), enforced both
in DefaultGroupByExecutor for all generators and inside
DictionaryBasedGroupKeyGenerator.
---
.../groupby/DefaultGroupByExecutor.java | 18 ++-
.../groupby/DictionaryBasedGroupKeyGenerator.java | 69 ++++++----
.../DictionaryBasedGroupKeyGeneratorTest.java | 153 +++++++++++++++++++++
3 files changed, 205 insertions(+), 35 deletions(-)
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/DefaultGroupByExecutor.java
b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/DefaultGroupByExecutor.java
index f4349c1171a..8a4015cd84d 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/DefaultGroupByExecutor.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/DefaultGroupByExecutor.java
@@ -97,10 +97,6 @@ public class DefaultGroupByExecutor implements
GroupByExecutor {
// Initialize group key generator
int numGroupsLimit = queryContext.getNumGroupsLimit();
int maxInitialResultHolderCapacity =
queryContext.getMaxInitialResultHolderCapacity();
- Map<ExpressionContext, Integer> groupByExpressionSizesFromPredicates =
null;
- if (queryContext.isOptimizeMaxInitialResultHolderCapacity()) {
- groupByExpressionSizesFromPredicates =
getGroupByExpressionSizesFromPredicates(queryContext);
- }
if (groupKeyGenerator != null) {
_groupKeyGenerator = groupKeyGenerator;
} else if (groupingSets) {
@@ -108,6 +104,9 @@ public class DefaultGroupByExecutor implements
GroupByExecutor {
new GroupingSetsGroupKeyGenerator(projectOperator,
groupByExpressions, queryContext.getGroupingSets(),
numGroupsLimit, _nullHandlingEnabled);
} else {
+ Map<ExpressionContext, Integer> groupByExpressionSizesFromPredicates =
+ queryContext.isOptimizeMaxInitialResultHolderCapacity()
+ ? getGroupByExpressionSizesFromPredicates(queryContext,
projectOperator) : null;
// Null handling does not steer this choice: every generator below gives
a null an id of its own, so the
// encoding of the group-by columns decides on its own which one to use.
if (hasNoDictionaryGroupByExpression) {
@@ -149,7 +148,11 @@ public class DefaultGroupByExecutor implements
GroupByExecutor {
/// 1. If the filter context is null or lacks GroupBy expressions, return
null.
/// 2. Ensure the top-level filter context consists solely of AND-type
filters; other types for example OR we cannot
/// guarantee deterministic sizes for GroupBy expressions.
- private Map<ExpressionContext, Integer>
getGroupByExpressionSizesFromPredicates(QueryContext queryContext) {
+ /// 3. Skip multi-value GroupBy expressions: a row matching an IN/EQ
predicate on a multi-value column contributes
+ /// one group per value inside the row (not only the matching values), so
the predicate size does not bound the
+ /// number of distinct groups.
+ private Map<ExpressionContext, Integer>
getGroupByExpressionSizesFromPredicates(QueryContext queryContext,
+ BaseProjectOperator<?> projectOperator) {
FilterContext filterContext = queryContext.getFilter();
if (filterContext == null || queryContext.getGroupByExpressions() == null)
{
return null;
@@ -183,11 +186,14 @@ public class DefaultGroupByExecutor implements
GroupByExecutor {
));
// Populate the group-by expressions with sizes from the predicate map
+ // NOTE: The merge function handles duplicate group-by expressions (e.g.
GROUP BY c0, c0)
return queryContext.getGroupByExpressions().stream()
.filter(predicateSizeMap::containsKey)
+ .filter(expression ->
projectOperator.getResultColumnContext(expression).isSingleValue())
.collect(Collectors.toMap(
expression -> expression,
- expression -> predicateSizeMap.getOrDefault(expression, null)
+ expression -> predicateSizeMap.getOrDefault(expression, null),
+ Integer::min
));
}
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/DictionaryBasedGroupKeyGenerator.java
b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/DictionaryBasedGroupKeyGenerator.java
index 823f11ce971..32fdf9dc214 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/DictionaryBasedGroupKeyGenerator.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/groupby/DictionaryBasedGroupKeyGenerator.java
@@ -26,12 +26,10 @@ import it.unimi.dsi.fastutil.objects.Object2IntMap;
import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap;
import it.unimi.dsi.fastutil.objects.ObjectIterator;
import java.util.Arrays;
-import java.util.HashMap;
import java.util.Iterator;
import java.util.Map;
import java.util.function.ToIntFunction;
import javax.annotation.Nullable;
-import org.apache.commons.lang3.tuple.Pair;
import org.apache.pinot.common.request.context.ExpressionContext;
import org.apache.pinot.core.common.BlockValSet;
import org.apache.pinot.core.operator.BaseProjectOperator;
@@ -66,6 +64,11 @@ import org.roaringbitmap.RoaringBitmap;
///
/// All the logic is maintained internally, and to the outside world, the
group ids are always int type, and are
/// bounded by the number of groups limit (globalGroupIdUpperBound is always
smaller or equal to numGroupsLimit).
+///
+/// When IN/EQ predicates on the group-by expressions prove an upper bound of
the number of distinct groups smaller
+/// than the cardinality product (see optimizeMaxInitialResultHolderCapacity),
the bound shrinks
+/// globalGroupIdUpperBound but never the raw key space, so a map based holder
is used even when the bound is below
+/// the ARRAY_BASED threshold.
public class DictionaryBasedGroupKeyGenerator implements GroupKeyGenerator {
// NOTE: map size = map capacity (power of 2) * load factor
private static final int INITIAL_MAP_SIZE = (int) ((1 << 9) * 0.75f);
@@ -114,6 +117,9 @@ public class DictionaryBasedGroupKeyGenerator implements
GroupKeyGenerator {
private final int _globalGroupIdUpperBound;
private final RawKeyHolder _rawKeyHolder;
+ /// @param groupByExpressionSizesFromPredicates IN/EQ predicate sizes for
the group-by expressions, used to bound
+ /// the number of distinct groups. Sizes for multi-value expressions
are ignored: every value inside a
+ /// matching row becomes a group, so the predicate size does not
bound the group count.
public DictionaryBasedGroupKeyGenerator(BaseProjectOperator<?>
projectOperator,
ExpressionContext[] groupByExpressions, int numGroupsLimit, int
arrayBasedThreshold,
boolean nullHandlingEnabled, @Nullable Map<ExpressionContext, Integer>
groupByExpressionSizesFromPredicates) {
@@ -130,7 +136,6 @@ public class DictionaryBasedGroupKeyGenerator implements
GroupKeyGenerator {
// no need to intern dictionary values when there is only one group by
expression because
// only one call will be made to the dictionary to extract each raw value.
_internedDictionaryValues = _numGroupByExpressions > 1 ? new
Object[_numGroupByExpressions][] : null;
- Map<ExpressionContext, Integer> cardinalityMap = new
HashMap<>(_numGroupByExpressions);
long cardinalityProduct = 1L;
boolean longOverflow = false;
for (int i = 0; i < _numGroupByExpressions; i++) {
@@ -158,7 +163,6 @@ public class DictionaryBasedGroupKeyGenerator implements
GroupKeyGenerator {
}
}
_cardinalities[i] = cardinality;
- cardinalityMap.put(groupByExpression, cardinality);
if (_internedDictionaryValues != null && cardinality <
MAX_DICTIONARY_INTERN_TABLE_SIZE) {
_internedDictionaryValues[i] = new Object[cardinality];
}
@@ -180,35 +184,40 @@ public class DictionaryBasedGroupKeyGenerator implements
GroupKeyGenerator {
_nullReplacedSingleValueDictIds = null;
_nullReplacedMultiValueDictIds = null;
}
- if (groupByExpressionSizesFromPredicates != null) {
- Pair<Boolean, Long> optimizedCardinality =
getOptimizedGroupByCardinality(groupByExpressionSizesFromPredicates,
- cardinalityMap);
- if (optimizedCardinality.getLeft() && optimizedCardinality.getRight() !=
null) {
- longOverflow = false;
- cardinalityProduct = Math.min(optimizedCardinality.getRight(),
cardinalityProduct);
- }
- }
+ // An IN/EQ predicate on a group-by column bounds the number of distinct
groups, so it can shrink the group id
+ // upper bound (and thus the result holder sizes). It must not influence
the holder type selection though: raw
+ // keys are mixed-radix products over the FULL dictionary cardinalities
regardless of the filter, so the
+ // int/long/array-map decision (raw key range) has to be based on the full
cardinality product.
+ long optimizedGroupCountUpperBound = groupByExpressionSizesFromPredicates
!= null
+ ? getOptimizedGroupByCardinality(groupByExpressionSizesFromPredicates)
: Long.MAX_VALUE;
+ int cappedNumGroupsLimit = (int) Math.min(numGroupsLimit,
optimizedGroupCountUpperBound);
// NOTE: We need to clean up the thread-local map before using it in case
RawKeyHolder.close() is not called
// for the previous segment
// TODO: Ensure RawKeyHolder.close()
if (longOverflow) {
// ArrayMapBasedHolder
- _globalGroupIdUpperBound = numGroupsLimit;
+ _globalGroupIdUpperBound = cappedNumGroupsLimit;
Object2IntOpenHashMap<IntArray> groupIdMap =
THREAD_LOCAL_INT_ARRAY_MAP.get();
clearAndTrim(groupIdMap);
_rawKeyHolder = new ArrayMapBasedHolder(groupIdMap);
} else {
if (cardinalityProduct > Integer.MAX_VALUE) {
// LongMapBasedHolder
- _globalGroupIdUpperBound = numGroupsLimit;
+ _globalGroupIdUpperBound = cappedNumGroupsLimit;
Long2IntOpenHashMap groupIdMap = THREAD_LOCAL_LONG_MAP.get();
clearAndTrim(groupIdMap);
_rawKeyHolder = new LongMapBasedHolder(groupIdMap);
} else {
- _globalGroupIdUpperBound = Math.min((int) cardinalityProduct,
numGroupsLimit);
+ _globalGroupIdUpperBound = (int) Math.min(cardinalityProduct,
cappedNumGroupsLimit);
// arrayBaseHolder fails with ArrayIndexOutOfBoundsException if
numGroupsLimit < cardinalityProduct
- // because array doesn't fit all (potentially unsorted) values
- if (cardinalityProduct > arrayBasedThreshold || numGroupsLimit <
cardinalityProduct) {
+ // because array doesn't fit all (potentially unsorted) values.
+ // ArrayBasedHolder uses the raw key directly as the group id, so it
is only valid when the group id space
+ // covers the full cardinality product; when the predicate-based
optimization proves a smaller upper bound,
+ // fall back to IntMapBasedHolder which maps the sparse raw keys onto
dense group ids. This deliberately
+ // trades a hash lookup per row for smaller result holders, which is
what the opt-in
+ // optimizeMaxInitialResultHolderCapacity query option asks for.
+ if (cardinalityProduct > arrayBasedThreshold || numGroupsLimit <
cardinalityProduct
+ || optimizedGroupCountUpperBound < cardinalityProduct) {
// IntMapBasedHolder
IntGroupIdMap groupIdMap = THREAD_LOCAL_INT_MAP.get();
groupIdMap.clearAndTrim();
@@ -220,20 +229,22 @@ public class DictionaryBasedGroupKeyGenerator implements
GroupKeyGenerator {
}
}
- private Pair<Boolean, Long>
getOptimizedGroupByCardinality(Map<ExpressionContext, Integer>
groupByExpressionSizes,
- Map<ExpressionContext, Integer> columnCardinalityMap) {
- long maxInitialResultHolderCapacity = 1L;
- for (Map.Entry<ExpressionContext, Integer> entry :
columnCardinalityMap.entrySet()) {
- Integer cardinality = entry.getValue();
- Integer size = groupByExpressionSizes.get(entry.getKey());
- int minSize = size != null ? Math.min(size, cardinality) : cardinality;
- if (maxInitialResultHolderCapacity > Long.MAX_VALUE / minSize) {
- return Pair.of(false, null);
- } else {
- maxInitialResultHolderCapacity *= minSize;
+ /// Returns the upper bound of the number of distinct groups derived from
the IN/EQ predicate sizes on the group-by
+ /// expressions, or [Long#MAX_VALUE] if the bound overflows long (i.e. no
usable bound). Multi-value expressions
+ /// always contribute their full cardinality: every value inside a matching
row becomes a group, so the predicate
+ /// size does not bound their group count. Iterating the expression array
(rather than a deduplicated map) keeps
+ /// the bound comparable to the cardinality product when the same expression
appears multiple times.
+ private long getOptimizedGroupByCardinality(Map<ExpressionContext, Integer>
groupByExpressionSizes) {
+ long groupCountUpperBound = 1L;
+ for (int i = 0; i < _numGroupByExpressions; i++) {
+ Integer size = _isSingleValueColumn[i] ?
groupByExpressionSizes.get(_groupByExpressions[i]) : null;
+ int minSize = size != null ? Math.min(size, _cardinalities[i]) :
_cardinalities[i];
+ if (minSize <= 0 || groupCountUpperBound > Long.MAX_VALUE / minSize) {
+ return Long.MAX_VALUE;
}
+ groupCountUpperBound *= minSize;
}
- return Pair.of(true, maxInitialResultHolderCapacity);
+ return groupCountUpperBound;
}
private static void clearAndTrim(Long2IntOpenHashMap map) {
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/DictionaryBasedGroupKeyGeneratorTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/DictionaryBasedGroupKeyGeneratorTest.java
index f77711f320e..05819441b80 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/DictionaryBasedGroupKeyGeneratorTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/DictionaryBasedGroupKeyGeneratorTest.java
@@ -24,6 +24,7 @@ import java.util.Arrays;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
+import java.util.Map;
import java.util.Random;
import java.util.Set;
import java.util.UUID;
@@ -53,6 +54,7 @@ import org.apache.pinot.spi.utils.ReadMode;
import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
+import org.testng.annotations.BeforeMethod;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;
@@ -165,6 +167,14 @@ public class DictionaryBasedGroupKeyGeneratorTest {
_valueBlock = _projectOperator.nextBlock();
}
+ // Clear the thread-local maps so that the holder type assertions on them
are order-independent
+ @BeforeMethod
+ public void clearThreadLocalMaps() {
+ DictionaryBasedGroupKeyGenerator.THREAD_LOCAL_INT_MAP.get().clearAndTrim();
+ DictionaryBasedGroupKeyGenerator.THREAD_LOCAL_LONG_MAP.get().clear();
+ DictionaryBasedGroupKeyGenerator.THREAD_LOCAL_INT_ARRAY_MAP.get().clear();
+ }
+
@Test
public void testArrayBasedSingleValue() {
// Cardinality product (100) smaller than arrayBasedThreshold
@@ -480,6 +490,142 @@ public class DictionaryBasedGroupKeyGeneratorTest {
dictionaryBasedGroupKeyGenerator.close();
}
+ @Test
+ public void testOptimizedUpperBoundSmallerThanCardinalityProduct() {
+ // Regression test: with optimizeMaxInitialResultHolderCapacity, an EQ/IN
predicate on the group-by column can
+ // shrink the group id upper bound below the cardinality product (here to
1). ArrayBasedHolder uses the raw
+ // dictionary-id based key directly as the group id, so it must not be
selected with the shrunk bound - doing so
+ // threw ArrayIndexOutOfBoundsException for any matching dictionary id
beyond the bound. IntMapBasedHolder must be
+ // used instead to map the sparse raw keys onto dense group ids.
+ String[] groupByColumns = {"s1"};
+ ExpressionContext[] expressions = getExpressions(groupByColumns);
+ // Simulate WHERE s1 = <value> (predicate size 1)
+ DictionaryBasedGroupKeyGenerator dictionaryBasedGroupKeyGenerator =
+ new DictionaryBasedGroupKeyGenerator(_projectOperator, expressions,
+ Server.DEFAULT_QUERY_EXECUTOR_NUM_GROUPS_LIMIT,
+ Server.DEFAULT_QUERY_EXECUTOR_MAX_INITIAL_RESULT_HOLDER_CAPACITY,
false, Map.of(expressions[0], 1));
+
assertEquals(dictionaryBasedGroupKeyGenerator.getGlobalGroupKeyUpperBound(), 1,
_errorMessage);
+
+ // The block contains 2 unique rows whose dictionary ids exceed the
optimized upper bound; generating group keys
+ // must not throw
+ dictionaryBasedGroupKeyGenerator.generateKeysForBlock(_valueBlock,
SV_GROUP_KEY_BUFFER);
+
assertEquals(dictionaryBasedGroupKeyGenerator.getCurrentGroupKeyUpperBound(),
1, _errorMessage);
+ // Only 1 group can be generated; the other unique row exceeds the upper
bound and gets INVALID_ID
+ for (int i = 0; i < NUM_GROUPS; i += 2) {
+ assertEquals(SV_GROUP_KEY_BUFFER[i], 0, _errorMessage);
+ assertEquals(SV_GROUP_KEY_BUFFER[i + 1], GroupKeyGenerator.INVALID_ID,
_errorMessage);
+ }
+ testGetGroupKeys(dictionaryBasedGroupKeyGenerator.getGroupKeys(), 1);
+
assertEquals(DictionaryBasedGroupKeyGenerator.THREAD_LOCAL_INT_MAP.get().size(),
1);
+ dictionaryBasedGroupKeyGenerator.close();
+
assertEquals(DictionaryBasedGroupKeyGenerator.THREAD_LOCAL_INT_MAP.get().size(),
0);
+ }
+
+ @Test
+ public void testOptimizedUpperBoundKeepsLongMapBasedHolder() {
+ // The predicate-based optimization must not downgrade the holder type:
raw keys are mixed-radix products over the
+ // full cardinality product (10,000,000,000 here, beyond
Integer.MAX_VALUE), so LongMapBasedHolder is required
+ // even though the predicates prove fewer groups. Downgrading to
IntMapBasedHolder would overflow the int raw keys
+ // and silently produce wrong groups.
+ String[] groupByColumns = {"s1", "s2", "s3", "s4", "s5"};
+ ExpressionContext[] expressions = getExpressions(groupByColumns);
+ DictionaryBasedGroupKeyGenerator dictionaryBasedGroupKeyGenerator =
+ new DictionaryBasedGroupKeyGenerator(_projectOperator, expressions,
+ Server.DEFAULT_QUERY_EXECUTOR_NUM_GROUPS_LIMIT,
+ Server.DEFAULT_QUERY_EXECUTOR_MAX_INITIAL_RESULT_HOLDER_CAPACITY,
false, Map.of(expressions[0], 2));
+ // Optimized upper bound (2 * 100^4 = 200,000,000) is capped by
numGroupsLimit
+
assertEquals(dictionaryBasedGroupKeyGenerator.getGlobalGroupKeyUpperBound(),
+ Server.DEFAULT_QUERY_EXECUTOR_NUM_GROUPS_LIMIT, _errorMessage);
+
+ // Test group key generation
+ dictionaryBasedGroupKeyGenerator.generateKeysForBlock(_valueBlock,
SV_GROUP_KEY_BUFFER);
+
assertEquals(dictionaryBasedGroupKeyGenerator.getCurrentGroupKeyUpperBound(),
2, _errorMessage);
+ compareSingleValueBuffer();
+ testGetGroupKeys(dictionaryBasedGroupKeyGenerator.getGroupKeys(), 2);
+
assertEquals(DictionaryBasedGroupKeyGenerator.THREAD_LOCAL_LONG_MAP.get().size(),
2);
+
assertEquals(DictionaryBasedGroupKeyGenerator.THREAD_LOCAL_INT_MAP.get().size(),
0);
+ dictionaryBasedGroupKeyGenerator.close();
+
assertEquals(DictionaryBasedGroupKeyGenerator.THREAD_LOCAL_LONG_MAP.get().size(),
0);
+ }
+
+ @Test
+ public void testOptimizedUpperBoundKeepsArrayMapBasedHolder() {
+ // Same as above for the long overflow case: the full cardinality product
(100^10) exceeds Long.MAX_VALUE, so
+ // ArrayMapBasedHolder is required even though the predicates prove fewer
groups. Downgrading to a long raw key
+ // based holder would overflow the long raw keys and silently produce
wrong groups.
+ String[] groupByColumns = {"s1", "s2", "s3", "s4", "s5", "s6", "s7", "s8",
"s9", "s10"};
+ ExpressionContext[] expressions = getExpressions(groupByColumns);
+ DictionaryBasedGroupKeyGenerator dictionaryBasedGroupKeyGenerator =
+ new DictionaryBasedGroupKeyGenerator(_projectOperator, expressions,
+ Server.DEFAULT_QUERY_EXECUTOR_NUM_GROUPS_LIMIT,
+ Server.DEFAULT_QUERY_EXECUTOR_MAX_INITIAL_RESULT_HOLDER_CAPACITY,
false, Map.of(expressions[0], 2));
+ // Optimized upper bound (2 * 100^9) is capped by numGroupsLimit
+
assertEquals(dictionaryBasedGroupKeyGenerator.getGlobalGroupKeyUpperBound(),
+ Server.DEFAULT_QUERY_EXECUTOR_NUM_GROUPS_LIMIT, _errorMessage);
+
+ // Test group key generation
+ dictionaryBasedGroupKeyGenerator.generateKeysForBlock(_valueBlock,
SV_GROUP_KEY_BUFFER);
+
assertEquals(dictionaryBasedGroupKeyGenerator.getCurrentGroupKeyUpperBound(),
2, _errorMessage);
+ compareSingleValueBuffer();
+ testGetGroupKeys(dictionaryBasedGroupKeyGenerator.getGroupKeys(), 2);
+
assertEquals(DictionaryBasedGroupKeyGenerator.THREAD_LOCAL_INT_ARRAY_MAP.get().size(),
2);
+
assertEquals(DictionaryBasedGroupKeyGenerator.THREAD_LOCAL_LONG_MAP.get().size(),
0);
+ dictionaryBasedGroupKeyGenerator.close();
+
assertEquals(DictionaryBasedGroupKeyGenerator.THREAD_LOCAL_INT_ARRAY_MAP.get().size(),
0);
+ }
+
+ @Test
+ public void
testOptimizedUpperBoundMatchingCardinalityProductKeepsArrayBasedHolder() {
+ // When the predicates do not prove fewer groups than the cardinality
product, ArrayBasedHolder must still be
+ // selected (the optimization must not deoptimize the direct-addressed
holder)
+ String[] groupByColumns = {"s1"};
+ ExpressionContext[] expressions = getExpressions(groupByColumns);
+ DictionaryBasedGroupKeyGenerator dictionaryBasedGroupKeyGenerator =
+ new DictionaryBasedGroupKeyGenerator(_projectOperator, expressions,
+ Server.DEFAULT_QUERY_EXECUTOR_NUM_GROUPS_LIMIT,
+ Server.DEFAULT_QUERY_EXECUTOR_MAX_INITIAL_RESULT_HOLDER_CAPACITY,
false,
+ Map.of(expressions[0], UNIQUE_ROWS));
+
assertEquals(dictionaryBasedGroupKeyGenerator.getGlobalGroupKeyUpperBound(),
UNIQUE_ROWS, _errorMessage);
+ // ArrayBasedHolder reports the full upper bound before generating any
keys; the map-based holders report 0
+
assertEquals(dictionaryBasedGroupKeyGenerator.getCurrentGroupKeyUpperBound(),
UNIQUE_ROWS, _errorMessage);
+
+ // Test group key generation
+ dictionaryBasedGroupKeyGenerator.generateKeysForBlock(_valueBlock,
SV_GROUP_KEY_BUFFER);
+ compareSingleValueBuffer();
+ testGetGroupKeys(dictionaryBasedGroupKeyGenerator.getGroupKeys(), 2);
+
assertEquals(DictionaryBasedGroupKeyGenerator.THREAD_LOCAL_INT_MAP.get().size(),
0);
+ dictionaryBasedGroupKeyGenerator.close();
+ }
+
+ @Test
+ public void testOptimizedUpperBoundIgnoresMultiValuePredicates() {
+ // A row matching an IN/EQ predicate on a multi-value column contributes
one group per value inside the row, not
+ // only the matching values, so the predicate size must not shrink the
group id upper bound for multi-value
+ // group-by expressions
+ String query = "SELECT COUNT(*) FROM testTable WHERE m1 = 1 GROUP BY m1";
+ QueryContext queryContext =
QueryContextConverterUtils.getQueryContext(query);
+ ExpressionContext[] expressions = getExpressions(new String[]{"m1"});
+ // NOTE: Use the executors one after the other - concurrently alive
generators could share a thread-local map
+ DefaultGroupByExecutor groupByExecutor = new
DefaultGroupByExecutor(queryContext, expressions, _projectOperator);
+ int upperBound =
groupByExecutor.getGroupKeyGenerator().getGlobalGroupKeyUpperBound();
+ groupByExecutor.process(_valueBlock);
+ int numGroups = groupByExecutor.getNumGroups();
+ // Precondition: with more groups than the EQ predicate size (1), a shrunk
bound would actually drop groups. The
+ // block holds 2 rows with disjoint multi-value sets, so this holds for
every random fixture.
+ assertTrue(numGroups > 1, _errorMessage);
+
+ QueryContext optimizedQueryContext =
QueryContextConverterUtils.getQueryContext(query);
+ optimizedQueryContext.setOptimizeMaxInitialResultHolderCapacity(true);
+ DefaultGroupByExecutor optimizedGroupByExecutor =
+ new DefaultGroupByExecutor(optimizedQueryContext, expressions,
_projectOperator);
+
assertEquals(optimizedGroupByExecutor.getGroupKeyGenerator().getGlobalGroupKeyUpperBound(),
upperBound,
+ _errorMessage);
+
+ // The block contains multi-value entries beyond the predicate values; all
of them must become groups
+ optimizedGroupByExecutor.process(_valueBlock);
+ assertEquals(optimizedGroupByExecutor.getNumGroups(), numGroups,
_errorMessage);
+ }
+
private static ExpressionContext[] getExpressions(String[] columns) {
int numColumns = columns.length;
ExpressionContext[] expressions = new ExpressionContext[numColumns];
@@ -561,6 +707,11 @@ public class DictionaryBasedGroupKeyGeneratorTest {
new DefaultGroupByExecutor(queryContext, expressions,
_projectOperator);
assertEquals(defaultGroupByExecutor.getGroupKeyGenerator().getGlobalGroupKeyUpperBound(),
expectedCapacity,
_errorMessage);
+ // Generate group keys and aggregate over the block to ensure the
optimized upper bound does not break key
+ // generation (regression: ArrayBasedHolder was selected with a shrunk
upper bound and threw
+ // ArrayIndexOutOfBoundsException)
+ defaultGroupByExecutor.process(_valueBlock);
+ assertTrue(defaultGroupByExecutor.getNumGroups() <= expectedCapacity,
_errorMessage);
}
@DataProvider(name = "groupByResultHolderCapacityDataProvider")
@@ -589,6 +740,8 @@ public class DictionaryBasedGroupKeyGeneratorTest {
{"SELECT COUNT(s9), s1, s2 FROM testTable WHERE s1 IN (1, 2, 3) GROUP
BY s1, s2 LIMIT 1000;", 300},
// OR Predicate -> (100 [s1] * 100 [s2]) = 1000 [Just cardinality
cross product]
{"SELECT COUNT(s9), s1, s2 FROM testTable WHERE s1 IN (1, 2, 3) OR s2
> 1 GROUP BY s1, s2 LIMIT 20000;", 10000},
+ // Duplicate group-by expression -> (3 [s1] * 3 [s1]) = 9 (regression:
used to throw on the duplicate map key)
+ {"SELECT COUNT(s9), s1 FROM testTable WHERE s1 IN (1, 2, 3) GROUP BY
s1, s1 LIMIT 10;", 9},
};
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
