This is an automated email from the ASF dual-hosted git repository. xiangfu pushed a commit to branch release-0.7.1-rc in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
commit 5b373b0e77649e8c8406250a3bbd21e41c6cb247 Author: Xiaotian (Jackie) Jiang <[email protected]> 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 | 18 +++++++----------- .../selection/SelectionOperatorServiceTest.java | 4 +--- 10 files changed, 44 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 fa00339..42884ac 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; @@ -44,8 +40,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,8 @@ import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; import com.yammer.metrics.core.MetricsRegistry; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class QueryExecutorTest { @@ -96,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); @@ -114,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: [email protected] For additional commands, e-mail: [email protected]
