You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by si...@apache.org on 2021/03/23 16:01:41 UTC
[incubator-pinot] branch master updated: Fix the default map return
value in DictionaryBasedGroupKeyGenerator (#6712)
This is an automated email from the ASF dual-hosted git repository.
siddteotia pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 5d2bc0c Fix the default map return value in DictionaryBasedGroupKeyGenerator (#6712)
5d2bc0c is described below
commit 5d2bc0c6d83825e152d30fb4774464fabf3b3e8b
Author: Xiaotian (Jackie) Jiang <17...@users.noreply.github.com>
AuthorDate: Tue Mar 23 09:01:19 2021 -0700
Fix the default map return value in DictionaryBasedGroupKeyGenerator (#6712)
---
.../groupby/DictionaryBasedGroupKeyGenerator.java | 22 ++++++++++++++++------
.../DefaultAggregationExecutorTest.java | 4 +---
.../DoubleAggregationResultHolderTest.java | 4 +---
.../AggregationGroupByTrimmingServiceTest.java | 4 +---
.../DictionaryBasedGroupKeyGeneratorTest.java | 17 ++++++++++++++---
.../groupby/DoubleGroupByResultHolderTest.java | 4 +---
.../groupby/NoDictionaryGroupKeyGeneratorTest.java | 5 +----
.../aggregation/groupby/StringGroupKeyTest.java | 3 +--
.../query/executor/QueryExecutorTest.java | 19 ++++++++-----------
.../selection/SelectionOperatorServiceTest.java | 4 +---
10 files changed, 45 insertions(+), 41 deletions(-)
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 98f42a4..29dc067 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
@@ -64,11 +64,20 @@ public class DictionaryBasedGroupKeyGenerator implements GroupKeyGenerator {
private static final int INITIAL_MAP_SIZE = (int) ((1 << 9) * 0.75f);
private static final int MAX_CACHING_MAP_SIZE = (int) ((1 << 20) * 0.75f);
- private static final ThreadLocal<IntGroupIdMap> THREAD_LOCAL_INT_MAP = ThreadLocal.withInitial(IntGroupIdMap::new);
- private static final ThreadLocal<Long2IntOpenHashMap> THREAD_LOCAL_LONG_MAP =
- ThreadLocal.withInitial(() -> new Long2IntOpenHashMap(INITIAL_MAP_SIZE));
- private static final ThreadLocal<Object2IntOpenHashMap<IntArray>> THREAD_LOCAL_INT_ARRAY_MAP =
- ThreadLocal.withInitial(() -> new Object2IntOpenHashMap<>(INITIAL_MAP_SIZE));
+ @VisibleForTesting
+ static final ThreadLocal<IntGroupIdMap> THREAD_LOCAL_INT_MAP = ThreadLocal.withInitial(IntGroupIdMap::new);
+ @VisibleForTesting
+ static final ThreadLocal<Long2IntOpenHashMap> THREAD_LOCAL_LONG_MAP = ThreadLocal.withInitial(() -> {
+ Long2IntOpenHashMap map = new Long2IntOpenHashMap(INITIAL_MAP_SIZE);
+ map.defaultReturnValue(INVALID_ID);
+ return map;
+ });
+ @VisibleForTesting
+ static final ThreadLocal<Object2IntOpenHashMap<IntArray>> THREAD_LOCAL_INT_ARRAY_MAP = ThreadLocal.withInitial(() -> {
+ Object2IntOpenHashMap<IntArray> map = new Object2IntOpenHashMap<>(INITIAL_MAP_SIZE);
+ map.defaultReturnValue(INVALID_ID);
+ return map;
+ });
private final ExpressionContext[] _groupByExpressions;
private final int _numGroupByExpressions;
@@ -1085,8 +1094,9 @@ public class DictionaryBasedGroupKeyGenerator implements GroupKeyGenerator {
/**
* Drop un-necessary checks for highest performance.
*/
+ @VisibleForTesting
@SuppressWarnings("EqualsWhichDoesntCheckParameterClass")
- private static class IntArray {
+ static class IntArray {
public int[] _elements;
public IntArray(int[] elements) {
diff --git a/pinot-core/src/test/java/org/apache/pinot/query/aggregation/DefaultAggregationExecutorTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/DefaultAggregationExecutorTest.java
similarity index 98%
rename from pinot-core/src/test/java/org/apache/pinot/query/aggregation/DefaultAggregationExecutorTest.java
rename to pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/DefaultAggregationExecutorTest.java
index 618514f..d655576 100644
--- a/pinot-core/src/test/java/org/apache/pinot/query/aggregation/DefaultAggregationExecutorTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/DefaultAggregationExecutorTest.java
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.pinot.query.aggregation;
+package org.apache.pinot.core.query.aggregation;
import java.io.File;
import java.util.ArrayList;
@@ -37,8 +37,6 @@ import org.apache.pinot.core.operator.blocks.TransformBlock;
import org.apache.pinot.core.operator.filter.MatchAllFilterOperator;
import org.apache.pinot.core.operator.transform.TransformOperator;
import org.apache.pinot.core.plan.DocIdSetPlanNode;
-import org.apache.pinot.core.query.aggregation.AggregationExecutor;
-import org.apache.pinot.core.query.aggregation.DefaultAggregationExecutor;
import org.apache.pinot.core.query.aggregation.function.AggregationFunction;
import org.apache.pinot.core.query.request.context.ExpressionContext;
import org.apache.pinot.core.query.request.context.QueryContext;
diff --git a/pinot-core/src/test/java/org/apache/pinot/query/aggregation/DoubleAggregationResultHolderTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/DoubleAggregationResultHolderTest.java
similarity index 91%
rename from pinot-core/src/test/java/org/apache/pinot/query/aggregation/DoubleAggregationResultHolderTest.java
rename to pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/DoubleAggregationResultHolderTest.java
index 1b1a97f..a28dd94 100644
--- a/pinot-core/src/test/java/org/apache/pinot/query/aggregation/DoubleAggregationResultHolderTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/DoubleAggregationResultHolderTest.java
@@ -16,11 +16,9 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.pinot.query.aggregation;
+package org.apache.pinot.core.query.aggregation;
import java.util.Random;
-import org.apache.pinot.core.query.aggregation.AggregationResultHolder;
-import org.apache.pinot.core.query.aggregation.DoubleAggregationResultHolder;
import org.testng.Assert;
import org.testng.annotations.Test;
diff --git a/pinot-core/src/test/java/org/apache/pinot/query/aggregation/groupby/AggregationGroupByTrimmingServiceTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/AggregationGroupByTrimmingServiceTest.java
similarity index 96%
rename from pinot-core/src/test/java/org/apache/pinot/query/aggregation/groupby/AggregationGroupByTrimmingServiceTest.java
rename to pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/AggregationGroupByTrimmingServiceTest.java
index eae4e30..ce1e0f4 100644
--- a/pinot-core/src/test/java/org/apache/pinot/query/aggregation/groupby/AggregationGroupByTrimmingServiceTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/AggregationGroupByTrimmingServiceTest.java
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.pinot.query.aggregation.groupby;
+package org.apache.pinot.core.query.aggregation.groupby;
import it.unimi.dsi.fastutil.ints.IntOpenHashSet;
import java.util.ArrayList;
@@ -28,8 +28,6 @@ import java.util.Random;
import java.util.Set;
import org.apache.commons.lang.RandomStringUtils;
import org.apache.pinot.common.response.broker.GroupByResult;
-import org.apache.pinot.core.query.aggregation.groupby.AggregationGroupByTrimmingService;
-import org.apache.pinot.core.query.aggregation.groupby.GroupKeyGenerator;
import org.apache.pinot.core.query.request.context.QueryContext;
import org.apache.pinot.core.query.request.context.utils.QueryContextConverterUtils;
import org.testng.Assert;
diff --git a/pinot-core/src/test/java/org/apache/pinot/query/aggregation/groupby/DictionaryBasedGroupKeyGeneratorTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/DictionaryBasedGroupKeyGeneratorTest.java
similarity index 96%
rename from pinot-core/src/test/java/org/apache/pinot/query/aggregation/groupby/DictionaryBasedGroupKeyGeneratorTest.java
rename to pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/DictionaryBasedGroupKeyGeneratorTest.java
index 1dd0b05..7a0e00c 100644
--- a/pinot-core/src/test/java/org/apache/pinot/query/aggregation/groupby/DictionaryBasedGroupKeyGeneratorTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/DictionaryBasedGroupKeyGeneratorTest.java
@@ -16,8 +16,10 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.pinot.query.aggregation.groupby;
+package org.apache.pinot.core.query.aggregation.groupby;
+import it.unimi.dsi.fastutil.longs.Long2IntOpenHashMap;
+import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap;
import java.io.File;
import java.util.ArrayList;
import java.util.Arrays;
@@ -40,8 +42,6 @@ import org.apache.pinot.core.operator.transform.TransformOperator;
import org.apache.pinot.core.plan.DocIdSetPlanNode;
import org.apache.pinot.core.plan.TransformPlanNode;
import org.apache.pinot.core.plan.maker.InstancePlanMakerImplV2;
-import org.apache.pinot.core.query.aggregation.groupby.DictionaryBasedGroupKeyGenerator;
-import org.apache.pinot.core.query.aggregation.groupby.GroupKeyGenerator;
import org.apache.pinot.core.query.request.context.ExpressionContext;
import org.apache.pinot.core.query.request.context.QueryContext;
import org.apache.pinot.core.query.request.context.utils.QueryContextConverterUtils;
@@ -423,6 +423,17 @@ public class DictionaryBasedGroupKeyGeneratorTest {
assertEquals(groupKeySet.size(), numUniqueKeys, _errorMessage);
}
+ @Test
+ public void testMapDefaultValue() {
+ Long2IntOpenHashMap longMap = DictionaryBasedGroupKeyGenerator.THREAD_LOCAL_LONG_MAP.get();
+ assertEquals(longMap.get(0L), GroupKeyGenerator.INVALID_ID);
+
+ Object2IntOpenHashMap<DictionaryBasedGroupKeyGenerator.IntArray> intArrayMap =
+ DictionaryBasedGroupKeyGenerator.THREAD_LOCAL_INT_ARRAY_MAP.get();
+ assertEquals(intArrayMap.getInt(new DictionaryBasedGroupKeyGenerator.IntArray(new int[0])),
+ GroupKeyGenerator.INVALID_ID);
+ }
+
@AfterClass
public void tearDown() {
FileUtils.deleteQuietly(new File(INDEX_DIR_PATH));
diff --git a/pinot-core/src/test/java/org/apache/pinot/query/aggregation/groupby/DoubleGroupByResultHolderTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/DoubleGroupByResultHolderTest.java
similarity index 94%
rename from pinot-core/src/test/java/org/apache/pinot/query/aggregation/groupby/DoubleGroupByResultHolderTest.java
rename to pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/DoubleGroupByResultHolderTest.java
index 3d22087..a7d78e3 100644
--- a/pinot-core/src/test/java/org/apache/pinot/query/aggregation/groupby/DoubleGroupByResultHolderTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/DoubleGroupByResultHolderTest.java
@@ -16,11 +16,9 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.pinot.query.aggregation.groupby;
+package org.apache.pinot.core.query.aggregation.groupby;
import java.util.Random;
-import org.apache.pinot.core.query.aggregation.groupby.DoubleGroupByResultHolder;
-import org.apache.pinot.core.query.aggregation.groupby.GroupByResultHolder;
import org.testng.Assert;
import org.testng.annotations.BeforeSuite;
import org.testng.annotations.Test;
diff --git a/pinot-core/src/test/java/org/apache/pinot/query/aggregation/groupby/NoDictionaryGroupKeyGeneratorTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryGroupKeyGeneratorTest.java
similarity index 97%
rename from pinot-core/src/test/java/org/apache/pinot/query/aggregation/groupby/NoDictionaryGroupKeyGeneratorTest.java
rename to pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryGroupKeyGeneratorTest.java
index 8a728a9..744087b 100644
--- a/pinot-core/src/test/java/org/apache/pinot/query/aggregation/groupby/NoDictionaryGroupKeyGeneratorTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/NoDictionaryGroupKeyGeneratorTest.java
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.pinot.query.aggregation.groupby;
+package org.apache.pinot.core.query.aggregation.groupby;
import java.io.File;
import java.io.IOException;
@@ -40,9 +40,6 @@ import org.apache.pinot.core.operator.transform.TransformOperator;
import org.apache.pinot.core.plan.DocIdSetPlanNode;
import org.apache.pinot.core.plan.TransformPlanNode;
import org.apache.pinot.core.plan.maker.InstancePlanMakerImplV2;
-import org.apache.pinot.core.query.aggregation.groupby.GroupKeyGenerator;
-import org.apache.pinot.core.query.aggregation.groupby.NoDictionaryMultiColumnGroupKeyGenerator;
-import org.apache.pinot.core.query.aggregation.groupby.NoDictionarySingleColumnGroupKeyGenerator;
import org.apache.pinot.core.query.request.context.ExpressionContext;
import org.apache.pinot.core.query.request.context.QueryContext;
import org.apache.pinot.core.query.request.context.utils.QueryContextConverterUtils;
diff --git a/pinot-core/src/test/java/org/apache/pinot/query/aggregation/groupby/StringGroupKeyTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/StringGroupKeyTest.java
similarity index 94%
rename from pinot-core/src/test/java/org/apache/pinot/query/aggregation/groupby/StringGroupKeyTest.java
rename to pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/StringGroupKeyTest.java
index ce9606f..8f3fa87 100644
--- a/pinot-core/src/test/java/org/apache/pinot/query/aggregation/groupby/StringGroupKeyTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/query/aggregation/groupby/StringGroupKeyTest.java
@@ -16,9 +16,8 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.pinot.query.aggregation.groupby;
+package org.apache.pinot.core.query.aggregation.groupby;
-import org.apache.pinot.core.query.aggregation.groupby.GroupKeyGenerator;
import org.testng.Assert;
import org.testng.annotations.Test;
diff --git a/pinot-core/src/test/java/org/apache/pinot/query/executor/QueryExecutorTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/executor/QueryExecutorTest.java
similarity index 95%
rename from pinot-core/src/test/java/org/apache/pinot/query/executor/QueryExecutorTest.java
rename to pinot-core/src/test/java/org/apache/pinot/core/query/executor/QueryExecutorTest.java
index 104192e..37e5846 100644
--- a/pinot-core/src/test/java/org/apache/pinot/query/executor/QueryExecutorTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/query/executor/QueryExecutorTest.java
@@ -16,10 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.pinot.query.executor;
-
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+package org.apache.pinot.core.query.executor;
import java.io.File;
import java.net.URL;
@@ -27,7 +24,6 @@ import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
-
import org.apache.commons.configuration.PropertiesConfiguration;
import org.apache.commons.io.FileUtils;
import org.apache.helix.HelixManager;
@@ -45,8 +41,6 @@ import org.apache.pinot.core.indexsegment.IndexSegment;
import org.apache.pinot.core.indexsegment.generator.SegmentGeneratorConfig;
import org.apache.pinot.core.indexsegment.immutable.ImmutableSegment;
import org.apache.pinot.core.indexsegment.immutable.ImmutableSegmentLoader;
-import org.apache.pinot.core.query.executor.QueryExecutor;
-import org.apache.pinot.core.query.executor.ServerQueryExecutorV1Impl;
import org.apache.pinot.core.query.request.ServerQueryRequest;
import org.apache.pinot.core.segment.creator.SegmentIndexCreationDriver;
import org.apache.pinot.core.segment.creator.impl.SegmentIndexCreationDriverImpl;
@@ -64,6 +58,9 @@ import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
public class QueryExecutorTest {
private static final String AVRO_DATA_PATH = "data/simpleData200001.avro";
@@ -95,8 +92,8 @@ public class QueryExecutorTest {
TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME).build();
int i = 0;
for (; i < NUM_SEGMENTS_TO_GENERATE; i++) {
- SegmentGeneratorConfig config =
- SegmentTestUtils.getSegmentGeneratorConfig(avroFile, FileFormat.AVRO, INDEX_DIR, TABLE_NAME, tableConfig, schema);
+ SegmentGeneratorConfig config = SegmentTestUtils
+ .getSegmentGeneratorConfig(avroFile, FileFormat.AVRO, INDEX_DIR, TABLE_NAME, tableConfig, schema);
config.setSegmentNamePostfix(Integer.toString(i));
SegmentIndexCreationDriver driver = new SegmentIndexCreationDriverImpl();
driver.init(config);
@@ -113,8 +110,8 @@ public class QueryExecutorTest {
Assert.assertNotNull(resourceUrl);
File jsonFile = new File(resourceUrl.getFile());
for (; i < NUM_SEGMENTS_TO_GENERATE + NUM_EMPTY_SEGMENTS_TO_GENERATE; i++) {
- SegmentGeneratorConfig config =
- SegmentTestUtils.getSegmentGeneratorConfig(jsonFile, FileFormat.JSON, INDEX_DIR, TABLE_NAME, tableConfig, schema);
+ SegmentGeneratorConfig config = SegmentTestUtils
+ .getSegmentGeneratorConfig(jsonFile, FileFormat.JSON, INDEX_DIR, TABLE_NAME, tableConfig, schema);
config.setSegmentNamePostfix(Integer.toString(i));
SegmentIndexCreationDriver driver = new SegmentIndexCreationDriverImpl();
driver.init(config);
diff --git a/pinot-core/src/test/java/org/apache/pinot/query/selection/SelectionOperatorServiceTest.java b/pinot-core/src/test/java/org/apache/pinot/core/query/selection/SelectionOperatorServiceTest.java
similarity index 98%
rename from pinot-core/src/test/java/org/apache/pinot/query/selection/SelectionOperatorServiceTest.java
rename to pinot-core/src/test/java/org/apache/pinot/core/query/selection/SelectionOperatorServiceTest.java
index e8c321f..324d8b6 100644
--- a/pinot-core/src/test/java/org/apache/pinot/query/selection/SelectionOperatorServiceTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/core/query/selection/SelectionOperatorServiceTest.java
@@ -16,7 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
-package org.apache.pinot.query.selection;
+package org.apache.pinot.core.query.selection;
import java.io.Serializable;
import java.util.ArrayList;
@@ -33,8 +33,6 @@ import org.apache.pinot.core.indexsegment.IndexSegment;
import org.apache.pinot.core.query.request.context.ExpressionContext;
import org.apache.pinot.core.query.request.context.QueryContext;
import org.apache.pinot.core.query.request.context.utils.QueryContextConverterUtils;
-import org.apache.pinot.core.query.selection.SelectionOperatorService;
-import org.apache.pinot.core.query.selection.SelectionOperatorUtils;
import org.apache.pinot.spi.utils.BytesUtils;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org