You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2021/11/25 08:57:58 UTC
[GitHub] [pinot] atris opened a new pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
atris opened a new pull request #7830:
URL: https://github.com/apache/pinot/pull/7830
This PR implements FILTER clauses using the concept of multiple predicates within a single query.
A FILTER clause query:
SELECT SUM(COL1) FILTER(WHERE fooBar > 5) FROM table1 WHERE barBar < 7
The query plan will introduce multiple transformation chains, each having a filter operator for the relevant FILTER clause. This allows indices to be used for each FILTER clause execution.
The main filter (WHERE barBar < 7) is evaluated once, with its results being broadcasted to all FILTER clause swim lanes, which then use that block to further evaluate their own predicate.
A mixture of filtered and non filtered aggregates (SELECT SUM(COL1) FILTER(WHERE barBar > 5), MAX(COL2) FROM table1 WHERE fooBar > 3) is allowed.
Please refer to the design document:
https://docs.google.com/document/d/1ZM-2c0jJkbeJ61m8sJF0qj19t5UYLhnTFvIAz-HCJmk/edit?usp=sharing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r764451918
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/FilteredAggregationsTest.java
##########
@@ -0,0 +1,307 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.queries;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.common.response.broker.BrokerResponseNative;
+import org.apache.pinot.common.response.broker.ResultTable;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.segment.local.indexsegment.immutable.ImmutableSegmentLoader;
+import org.apache.pinot.segment.local.segment.creator.impl.SegmentIndexCreationDriverImpl;
+import org.apache.pinot.segment.local.segment.index.loader.IndexLoadingConfig;
+import org.apache.pinot.segment.local.segment.readers.GenericRowRecordReader;
+import org.apache.pinot.segment.spi.ImmutableSegment;
+import org.apache.pinot.segment.spi.IndexSegment;
+import org.apache.pinot.segment.spi.creator.SegmentGeneratorConfig;
+import org.apache.pinot.spi.config.table.FieldConfig;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.data.readers.GenericRow;
+import org.apache.pinot.spi.data.readers.RecordReader;
+import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+
+public class FilteredAggregationsTest extends BaseQueriesTest {
+ private static final File INDEX_DIR = new File(FileUtils.getTempDirectory(), "FilteredAggregationsTest");
+ private static final String TABLE_NAME = "MyTable";
+ private static final String FIRST_SEGMENT_NAME = "firstTestSegment";
+ private static final String SECOND_SEGMENT_NAME = "secondTestSegment";
+ private static final String INT_COL_NAME = "INT_COL";
+ private static final String NO_INDEX_INT_COL_NAME = "NO_INDEX_COL";
+ private static final Integer INT_BASE_VALUE = 0;
+ private static final Integer NUM_ROWS = 30000;
+
+
+ private IndexSegment _indexSegment;
+ private List<IndexSegment> _indexSegments;
+
+ @Override
+ protected String getFilter() {
+ return "";
+ }
+
+ @Override
+ protected IndexSegment getIndexSegment() {
+ return _indexSegment;
+ }
+
+ @Override
+ protected List<IndexSegment> getIndexSegments() {
+ return _indexSegments;
+ }
+
+ @BeforeClass
+ public void setUp()
+ throws Exception {
+ FileUtils.deleteQuietly(INDEX_DIR);
+
+ buildSegment(FIRST_SEGMENT_NAME);
+ buildSegment(SECOND_SEGMENT_NAME);
+ IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig();
+
+ Set<String> invertedIndexCols = new HashSet<>();
+ invertedIndexCols.add(INT_COL_NAME);
+
+ indexLoadingConfig.setInvertedIndexColumns(invertedIndexCols);
+ ImmutableSegment firstImmutableSegment =
+ ImmutableSegmentLoader.load(new File(INDEX_DIR, FIRST_SEGMENT_NAME), indexLoadingConfig);
+ ImmutableSegment secondImmutableSegment =
+ ImmutableSegmentLoader.load(new File(INDEX_DIR, SECOND_SEGMENT_NAME), indexLoadingConfig);
+ _indexSegment = firstImmutableSegment;
+ _indexSegments = Arrays.asList(firstImmutableSegment, secondImmutableSegment);
+ }
+
+ @AfterClass
+ public void tearDown() {
+ _indexSegment.destroy();
+ FileUtils.deleteQuietly(INDEX_DIR);
+ }
+
+ private List<GenericRow> createTestData(int numRows) {
+ List<GenericRow> rows = new ArrayList<>();
+
+ for (int i = 0; i < numRows; i++) {
+ GenericRow row = new GenericRow();
+ row.putField(INT_COL_NAME, INT_BASE_VALUE + i);
+ row.putField(NO_INDEX_INT_COL_NAME, i);
+
+ rows.add(row);
+ }
+ return rows;
+ }
+
+ private void buildSegment(String segmentName)
+ throws Exception {
+ List<GenericRow> rows = createTestData(NUM_ROWS);
+ List<FieldConfig> fieldConfigs = new ArrayList<>();
+
+ TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME)
+ .setInvertedIndexColumns(Arrays.asList(INT_COL_NAME)).setFieldConfigList(fieldConfigs).build();
+ Schema schema = new Schema.SchemaBuilder().setSchemaName(TABLE_NAME)
+ .addSingleValueDimension(NO_INDEX_INT_COL_NAME, FieldSpec.DataType.INT)
+ .addMetric(INT_COL_NAME, FieldSpec.DataType.INT).build();
+ SegmentGeneratorConfig config = new SegmentGeneratorConfig(tableConfig, schema);
+ config.setOutDir(INDEX_DIR.getPath());
+ config.setTableName(TABLE_NAME);
+ config.setSegmentName(segmentName);
+
+ SegmentIndexCreationDriverImpl driver = new SegmentIndexCreationDriverImpl();
+ try (RecordReader recordReader = new GenericRowRecordReader(rows)) {
+ driver.init(config, recordReader);
+ driver.build();
+ }
+ }
+
+ private void testInterSegmentAggregationQueryHelper(String firstQuery, String secondQuery) {
+ // SQL
+ BrokerResponseNative firstBrokerResponseNative = getBrokerResponseForSqlQuery(firstQuery);
+ BrokerResponseNative secondBrokerResponseNative = getBrokerResponseForSqlQuery(secondQuery);
+ ResultTable firstResultTable = firstBrokerResponseNative.getResultTable();
+ ResultTable secondResultTable = secondBrokerResponseNative.getResultTable();
+ DataSchema firstDataSchema = firstResultTable.getDataSchema();
+ DataSchema secondDataSchema = secondResultTable.getDataSchema();
+
+ Assert.assertEquals(firstDataSchema.size(), secondDataSchema.size());
+
+ List<Object[]> firstSetOfRows = firstResultTable.getRows();
+ List<Object[]> secondSetOfRows = secondResultTable.getRows();
+
+ Assert.assertEquals(firstSetOfRows.size(), secondSetOfRows.size());
+
+ for (int i = 0; i < firstSetOfRows.size(); i++) {
+ Object[] firstSetRow = firstSetOfRows.get(i);
+ Object[] secondSetRow = secondSetOfRows.get(i);
+
+ Assert.assertEquals(firstSetRow.length, secondSetRow.length);
+
+ for (int j = 0; j < firstSetRow.length; j++) {
+ //System.out.println("FIRST " + firstSetRow[j] + " SECOND " + secondSetRow[j] + " j " + j);
+ Assert.assertEquals(firstSetRow[j], secondSetRow[j]);
+ }
+ }
+ }
+
+ @Test
+ public void testInterSegment() {
+
+ String query =
+ "SELECT SUM(INT_COL) FILTER(WHERE INT_COL > 9999)"
+ + "FROM MyTable WHERE INT_COL < 1000000";
+
+ String nonFilterQuery =
+ "SELECT SUM(INT_COL)"
+ + "FROM MyTable WHERE INT_COL > 9999 AND INT_COL < 1000000";
+
+ testInterSegmentAggregationQueryHelper(query, nonFilterQuery);
+
+ query = "SELECT SUM(INT_COL) FILTER(WHERE INT_COL > 1234 AND INT_COL < 22000)"
+ + "FROM MyTable";
+
+ nonFilterQuery = "SELECT SUM("
+ + "CASE "
+ + "WHEN (INT_COL > 1234 AND INT_COL < 22000) THEN INT_COL "
+ + "ELSE 0 "
+ + "END) AS total_sum "
+ + "FROM MyTable";
+
+ testInterSegmentAggregationQueryHelper(query, nonFilterQuery);
+
+ query =
Review comment:
Can we add one query where the outer predicate is somewhat nested ?
Something like `WHERE cond1 OR cond2`
So the predicate in `nonFilterQuery` becomes `WHERE (cond1 OR cond2) AND (innerFilterCond)`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r764503815
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BlockDrivenAndFilterOperator.java
##########
@@ -0,0 +1,126 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.filter;
+
+import java.util.ArrayList;
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.core.common.BlockDocIdSet;
+import org.apache.pinot.core.common.Operator;
+import org.apache.pinot.core.operator.VisitableOperator;
+import org.apache.pinot.core.operator.blocks.FilterBlock;
+import org.apache.pinot.core.operator.blocks.TransformBlock;
+import org.apache.pinot.core.operator.dociditerators.ArrayBasedDocIdIterator;
+import org.apache.pinot.core.operator.docidsets.AndDocIdSet;
+import org.apache.pinot.core.operator.docidsets.ArrayBasedDocIdSet;
+import org.apache.pinot.core.operator.docidsets.BitmapDocIdSet;
+import org.apache.pinot.core.operator.docidsets.FilterBlockDocIdSet;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
+
+/**
+ * Performs an AND operation on top of a Filter Block DocIDSet
+ * and a block from the given filter operator.
+ */
+public class BlockDrivenAndFilterOperator extends BaseFilterOperator
+ implements VisitableOperator {
+ private static final String OPERATOR_NAME = "BlockDrivenAndFilterOperator";
+
+ private final BaseFilterOperator _filterOperator;
+ private FilterBlockDocIdSet _filterBlockDocIdSet;
+ private final int _numDocs;
+
+ public BlockDrivenAndFilterOperator(BaseFilterOperator filterOperator, int numDocs) {
+ _filterOperator = filterOperator;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public FilterBlock getNextBlock() {
+
+ if (_filterBlockDocIdSet != null) {
+ List<FilterBlockDocIdSet> filterBlockDocIdSets = new ArrayList<>(2);
+
+ filterBlockDocIdSets.add(_filterBlockDocIdSet);
+ filterBlockDocIdSets.add(_filterOperator.nextBlock().getBlockDocIdSet());
+
+ return new FilterBlock(new AndDocIdSet(filterBlockDocIdSets));
+ }
+
+ return _filterOperator.nextBlock();
+ }
+
+ @Override
+ public String getOperatorName() {
+ return OPERATOR_NAME;
+ }
+
+ @Override
+ public List<Operator> getChildOperators() {
+ return null;
+ }
+
+ @Nullable
+ @Override
+ public String toExplainString() {
+ return null;
+ }
+
+ @Override
+ public <T> void accept(T incomingObject) {
Review comment:
I am going to pull the code locally to understand the flow better. But, is it correct that this is being called to accept the outer transform block for the main WHERE clause ?
Is there a strong need to add these visit/accept semantics ? The flow from execution perspective is not super clear at this point on how to use these methods.
When we know that BlockDrivenAndFilterOperator is going to be created (at least as of now) for FILTER aggregation queries, then we know the execution plan ahead of time. So, I don't think we need any custom transformation to happen on the nodes of a tree (which is where Visitor pattern might be more helpful afaik). The use does not look very intuitive to me so far.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r764465294
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/plan/FilterPlanNode.java
##########
@@ -70,9 +71,16 @@ public FilterPlanNode(IndexSegment indexSegment, QueryContext queryContext) {
_numDocs = _indexSegment.getSegmentMetadata().getTotalDocs();
}
+ public FilterPlanNode(IndexSegment indexSegment, QueryContext queryContext,
+ FilterContext filterContext) {
Review comment:
Can we rename `filterContext` to `aggregationFilterContext` or something to indicate that this FilterPlanNode is constructed from AggregationPlanNode for FILTERed aggregation ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r764465294
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/plan/FilterPlanNode.java
##########
@@ -70,9 +71,16 @@ public FilterPlanNode(IndexSegment indexSegment, QueryContext queryContext) {
_numDocs = _indexSegment.getSegmentMetadata().getTotalDocs();
}
+ public FilterPlanNode(IndexSegment indexSegment, QueryContext queryContext,
+ FilterContext filterContext) {
Review comment:
Can we rename `filterContext` to `aggregationFilterContext` or something to indicate that this FilterPlanNode is constructed from AggregationPlanNode for FILTERed aggregation and not DocIdSetPlanNode for outer filter
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] atris commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r763693154
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/FilterableAggregation.java
##########
@@ -0,0 +1,31 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+public class FilterableAggregation {
Review comment:
It was an attempt to reduce code duplication. Removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#issuecomment-979136818
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7830](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e51d55b) into [master](https://codecov.io/gh/apache/pinot/commit/47e49ecd6e11aebe74f8868cdac22051b175d4c5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47e49ec) will **decrease** coverage by `40.65%`.
> The diff coverage is `30.91%`.
[](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7830 +/- ##
=============================================
- Coverage 71.63% 30.97% -40.66%
=============================================
Files 1580 1577 -3
Lines 81100 81192 +92
Branches 12068 12122 +54
=============================================
- Hits 58093 25148 -32945
- Misses 19092 53853 +34761
+ Partials 3915 2191 -1724
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `29.40% <30.91%> (+<0.01%)` | :arrow_up: |
| integration2 | `27.83% <30.15%> (-0.07%)` | :arrow_down: |
| unittests1 | `?` | |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...g/apache/pinot/core/operator/DocIdSetOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9Eb2NJZFNldE9wZXJhdG9yLmphdmE=) | `87.09% <0.00%> (-6.01%)` | :arrow_down: |
| [...t/core/operator/blocks/CombinedTransformBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvQ29tYmluZWRUcmFuc2Zvcm1CbG9jay5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...che/pinot/core/operator/blocks/TransformBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvVHJhbnNmb3JtQmxvY2suamF2YQ==) | `69.23% <0.00%> (ø)` | |
| [.../operator/filter/BlockDrivenAndFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvQmxvY2tEcml2ZW5BbmRGaWx0ZXJPcGVyYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [.../operator/transform/CombinedTransformOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vQ29tYmluZWRUcmFuc2Zvcm1PcGVyYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...erator/transform/PassThroughTransformOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vUGFzc1Rocm91Z2hUcmFuc2Zvcm1PcGVyYXRvci5qYXZh) | `70.00% <0.00%> (-17.50%)` | :arrow_down: |
| [...t/core/plan/AggregationGroupByOrderByPlanNode.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0FnZ3JlZ2F0aW9uR3JvdXBCeU9yZGVyQnlQbGFuTm9kZS5qYXZh) | `45.45% <0.00%> (ø)` | |
| [...aggregation/FilteredClauseAggregationExecutor.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9GaWx0ZXJlZENsYXVzZUFnZ3JlZ2F0aW9uRXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...aggregation/function/AggregationFunctionUtils.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9BZ2dyZWdhdGlvbkZ1bmN0aW9uVXRpbHMuamF2YQ==) | `70.90% <0.00%> (-16.33%)` | :arrow_down: |
| [...n/function/BaseSingleInputAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9CYXNlU2luZ2xlSW5wdXRBZ2dyZWdhdGlvbkZ1bmN0aW9uLmphdmE=) | `46.15% <ø> (-30.77%)` | :arrow_down: |
| ... and [1106 more](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [41c98d7...e51d55b](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#issuecomment-979136818
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7830](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (371c89b) into [master](https://codecov.io/gh/apache/pinot/commit/13c9ee9556498bb6dc4ab60734743edb8b89773c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (13c9ee9) will **decrease** coverage by `2.72%`.
> The diff coverage is `66.85%`.
> :exclamation: Current head 371c89b differs from pull request most recent head 97bd184. Consider uploading reports for the commit 97bd184 to get more accurate results
[](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7830 +/- ##
============================================
- Coverage 71.49% 68.77% -2.73%
+ Complexity 4064 4009 -55
============================================
Files 1577 1190 -387
Lines 80554 58365 -22189
Branches 11965 8967 -2998
============================================
- Hits 57592 40140 -17452
+ Misses 19078 15396 -3682
+ Partials 3884 2829 -1055
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `68.77% <66.85%> (+0.17%)` | :arrow_up: |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...e/pinot/common/utils/FileUploadDownloadClient.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvRmlsZVVwbG9hZERvd25sb2FkQ2xpZW50LmphdmE=) | `17.39% <0.00%> (-48.86%)` | :arrow_down: |
| [...a/org/apache/pinot/common/utils/ServiceStatus.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvU2VydmljZVN0YXR1cy5qYXZh) | `54.59% <0.00%> (-9.70%)` | :arrow_down: |
| [...apache/pinot/common/utils/SimpleHttpErrorInfo.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9waW5vdC9jb21tb24vdXRpbHMvU2ltcGxlSHR0cEVycm9ySW5mby5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...rg/apache/pinot/core/minion/RawIndexConverter.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9taW5pb24vUmF3SW5kZXhDb252ZXJ0ZXIuamF2YQ==) | `0.00% <0.00%> (-56.61%)` | :arrow_down: |
| [...operator/AcquireReleaseColumnsSegmentOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9BY3F1aXJlUmVsZWFzZUNvbHVtbnNTZWdtZW50T3BlcmF0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...not/core/operator/transform/TransformOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vVHJhbnNmb3JtT3BlcmF0b3IuamF2YQ==) | `86.36% <0.00%> (-8.64%)` | :arrow_down: |
| [...t/core/plan/AggregationGroupByOrderByPlanNode.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0FnZ3JlZ2F0aW9uR3JvdXBCeU9yZGVyQnlQbGFuTm9kZS5qYXZh) | `45.45% <0.00%> (ø)` | |
| [...he/pinot/core/plan/AggregationGroupByPlanNode.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0FnZ3JlZ2F0aW9uR3JvdXBCeVBsYW5Ob2RlLmphdmE=) | `45.45% <0.00%> (-36.37%)` | :arrow_down: |
| [...aggregation/function/AggregationFunctionUtils.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9BZ2dyZWdhdGlvbkZ1bmN0aW9uVXRpbHMuamF2YQ==) | `65.45% <0.00%> (-21.78%)` | :arrow_down: |
| [...n/function/BaseSingleInputAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9CYXNlU2luZ2xlSW5wdXRBZ2dyZWdhdGlvbkZ1bmN0aW9uLmphdmE=) | `100.00% <ø> (ø)` | |
| ... and [691 more](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [13c9ee9...97bd184](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r764465294
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/plan/FilterPlanNode.java
##########
@@ -70,9 +71,16 @@ public FilterPlanNode(IndexSegment indexSegment, QueryContext queryContext) {
_numDocs = _indexSegment.getSegmentMetadata().getTotalDocs();
}
+ public FilterPlanNode(IndexSegment indexSegment, QueryContext queryContext,
+ FilterContext filterContext) {
Review comment:
Can we rename `filterContext` to `aggregationFilterContext` or something to indicate that this FilterPlanNode is constructed from `AggregationPlanNode` for FILTERed aggregation and not `DocIdSetPlanNode` for outer filter
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] amrishlal commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r765327959
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/operator/filter/BlockDrivenAndFilterOperatorTest.java
##########
@@ -0,0 +1,272 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.filter;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Stream;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.common.request.context.ExpressionContext;
+import org.apache.pinot.core.common.BlockDocIdIterator;
+import org.apache.pinot.core.operator.DocIdSetOperator;
+import org.apache.pinot.core.operator.ProjectionOperator;
+import org.apache.pinot.core.operator.VisitableOperator;
+import org.apache.pinot.core.operator.blocks.TransformBlock;
+import org.apache.pinot.core.operator.transform.TransformOperator;
+import org.apache.pinot.core.plan.DocIdSetPlanNode;
+import org.apache.pinot.core.query.request.context.QueryContext;
+import org.apache.pinot.core.query.request.context.utils.QueryContextConverterUtils;
+import org.apache.pinot.segment.local.indexsegment.immutable.ImmutableSegmentLoader;
+import org.apache.pinot.segment.local.segment.creator.impl.SegmentIndexCreationDriverImpl;
+import org.apache.pinot.segment.local.segment.readers.GenericRowRecordReader;
+import org.apache.pinot.segment.spi.Constants;
+import org.apache.pinot.segment.spi.IndexSegment;
+import org.apache.pinot.segment.spi.creator.SegmentGeneratorConfig;
+import org.apache.pinot.segment.spi.datasource.DataSource;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.MetricFieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.data.readers.GenericRow;
+import org.apache.pinot.spi.utils.ReadMode;
+import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
+import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import static org.testng.Assert.assertTrue;
+
+public class BlockDrivenAndFilterOperatorTest {
+ private static final File INDEX_DIR = new File(FileUtils.getTempDirectory(),
+ "BlockDrivenAndFilterOperatorTest");
+ private static final String SEGMENT_NAME = "TestBlockDrivenAndOperator";
+
+ private static final String METRIC_PREFIX = "metric_";
+ private static final int NUM_METRIC_COLUMNS = 2;
+ private static final int NUM_ROWS = 20000;
+
+ public static IndexSegment _indexSegment;
+ private String[] _columns;
+ private QueryContext _queryContext;
+ private double[][] _inputData;
+
+ /**
+ * Initializations prior to the test:
+ * - Build a segment with metric columns (that will be aggregated) containing
+ * randomly generated data.
+ *
+ * @throws Exception
+ */
+ @BeforeClass
+ public void setUp()
+ throws Exception {
+ int numColumns = NUM_METRIC_COLUMNS;
+
+ _inputData = new double[numColumns][NUM_ROWS];
+
+ _columns = new String[numColumns];
+
+ setupSegment();
+
+ StringBuilder queryBuilder = new StringBuilder("SELECT SUM(INT_COL)");
+
+ queryBuilder.append(" FROM testTable");
+
+ _queryContext = QueryContextConverterUtils.getQueryContextFromSQL(queryBuilder.toString());
+ }
+
+ /**
+ * Helper method to setup the index segment on which to perform aggregation tests.
+ * - Generates a segment with {@link #NUM_METRIC_COLUMNS} and {@link #NUM_ROWS}
+ * - Random 'double' data filled in the metric columns. The data is also populated
+ * into the _inputData[], so it can be used to test the results.
+ *
+ * @throws Exception
+ */
+ private void setupSegment()
+ throws Exception {
+ if (INDEX_DIR.exists()) {
+ FileUtils.deleteQuietly(INDEX_DIR);
+ }
+
+ SegmentGeneratorConfig config =
+ new SegmentGeneratorConfig(new TableConfigBuilder(TableType.OFFLINE).setTableName("test")
+ .build(),
+ buildSchema());
+ config.setSegmentName(SEGMENT_NAME);
+ config.setOutDir(INDEX_DIR.getAbsolutePath());
+
+ List<GenericRow> rows = new ArrayList<>(NUM_ROWS);
+ for (int i = 0; i < NUM_ROWS; i++) {
+ Map<String, Object> map = new HashMap<>();
+
+ for (int j = 0; j < _columns.length; j++) {
+ String metricName = _columns[j];
+ int value = i;
+ _inputData[j][i] = value;
+ map.put(metricName, value);
+ }
+
+ GenericRow genericRow = new GenericRow();
+ genericRow.init(map);
+ rows.add(genericRow);
+ }
+
+ SegmentIndexCreationDriverImpl driver = new SegmentIndexCreationDriverImpl();
+ driver.init(config, new GenericRowRecordReader(rows));
+ driver.build();
+
+ _indexSegment = ImmutableSegmentLoader.load(new File(INDEX_DIR, driver.getSegmentName()), ReadMode.heap);
+ }
+
+ /**
+ * Helper method to build schema for the segment on which aggregation tests will be run.
+ *
+ * @return
+ */
+ private Schema buildSchema() {
+ Schema schema = new Schema();
+
+ for (int i = 0; i < NUM_METRIC_COLUMNS; i++) {
+ String metricName = METRIC_PREFIX + i;
+ MetricFieldSpec metricFieldSpec = new MetricFieldSpec(metricName, FieldSpec.DataType.DOUBLE);
+ schema.addField(metricFieldSpec);
+ _columns[i] = metricName;
+ }
+ return schema;
+ }
+
+ @AfterClass
+ void tearDown() {
+ FileUtils.deleteQuietly(INDEX_DIR);
+ }
+
+ @Test
+ void testBlockFilteredAggregation() {
+ int totalDocs = _indexSegment.getSegmentMetadata().getTotalDocs();
+
+ executeTest(new MatchAllFilterOperator(totalDocs), null);
+
+ int limit = NUM_ROWS / 4;
+ int[] dataArray = new int[limit];
+
+ Set<Integer> seenNumber = new HashSet<>();
+
+ for (int i = 0; i < limit; i++) {
+ dataArray[i] = getRandomNumber(i, NUM_ROWS, seenNumber);
+ }
+
+ BitmapBasedFilterOperator bitmapBasedFilterOperator = new BitmapBasedFilterOperator(
+ ImmutableRoaringBitmap.bitmapOf(dataArray), false, totalDocs);
+
+ executeTest(bitmapBasedFilterOperator, dataArray);
+ }
+
+ private void executeTest(BaseFilterOperator baseFilterOperator, int[] addedDocIDs) {
+ Map<String, DataSource> dataSourceMap = new HashMap<>();
+ List<ExpressionContext> expressions = new ArrayList<>();
+ for (String column : _indexSegment.getPhysicalColumnNames()) {
+ dataSourceMap.put(column, _indexSegment.getDataSource(column));
+ expressions.add(ExpressionContext.forIdentifier(column));
+ }
+ int totalDocs = _indexSegment.getSegmentMetadata().getTotalDocs();
+
+ DocIdSetOperator docIdSetOperator = new DocIdSetOperator(baseFilterOperator,
+ DocIdSetPlanNode.MAX_DOC_PER_CALL);
+ ProjectionOperator projectionOperator = new ProjectionOperator(dataSourceMap, docIdSetOperator);
+ TransformOperator transformOperator = new TransformOperator(_queryContext, projectionOperator, expressions);
+ TransformBlock transformBlock = transformOperator.nextBlock();
+
+ int limit = NUM_ROWS / 2;
+ int[] dataArray = new int[limit];
+
+ Set<Integer> seenNumber = new HashSet<>();
+
+ for (int i = 0; i < limit; i++) {
+ dataArray[i] = getRandomNumber(i, NUM_ROWS, seenNumber);
+ }
+
+ int[] finalArray;
+
+ if (addedDocIDs != null) {
+ finalArray = performIntersection(dataArray, addedDocIDs);
+ } else {
+ finalArray = dataArray;
+ }
+
+ assertTrue(finalArray != null);
+
+ BitmapBasedFilterOperator bitmapBasedFilterOperator = new BitmapBasedFilterOperator(
+ ImmutableRoaringBitmap.bitmapOf(finalArray), false, totalDocs);
+ BlockDrivenAndFilterOperator blockDrivenAndFilterOperator = new BlockDrivenAndFilterOperator(
+ bitmapBasedFilterOperator, totalDocs);
+
+ Set<Integer> resultSet = new HashSet<>();
+
+ while (transformBlock != null) {
+ ((VisitableOperator) blockDrivenAndFilterOperator).accept(transformBlock);
+
+ BlockDocIdIterator blockDocIdIterator = blockDrivenAndFilterOperator.getNextBlock()
+ .getBlockDocIdSet().iterator();
+
+ int currentValue = blockDocIdIterator.next();
+
+ while (currentValue != Constants.EOF) {
+ resultSet.add(currentValue);
+
+ currentValue = blockDocIdIterator.next();
+ }
+
+ transformBlock = transformOperator.nextBlock();
+ }
+
+ assert resultSet.size() == finalArray.length;
+
+ for (int i = 0; i < finalArray.length; i++) {
+ assertTrue(resultSet.contains(finalArray[i]), "Result does not contain " + finalArray[i]);
+ }
+ }
+
+ public static int[] performIntersection(int[] a, int[] b) {
+ return Arrays.stream(Stream.of(a)
Review comment:
Pinot coding convention doesn't allow use of `stream` api for performance reasons.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] atris commented on pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
atris commented on pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#issuecomment-995777724
Superseded by #7916
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#issuecomment-982323715
Reviewed the design doc. Will review the PR tomorrow
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#issuecomment-979136818
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7830](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a0e92fd) into [master](https://codecov.io/gh/apache/pinot/commit/47e49ecd6e11aebe74f8868cdac22051b175d4c5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47e49ec) will **decrease** coverage by `57.12%`.
> The diff coverage is `0.37%`.
[](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7830 +/- ##
=============================================
- Coverage 71.63% 14.50% -57.13%
+ Complexity 4088 80 -4008
=============================================
Files 1580 1541 -39
Lines 81100 79672 -1428
Branches 12068 11958 -110
=============================================
- Hits 58093 11558 -46535
- Misses 19092 67253 +48161
+ Partials 3915 861 -3054
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `14.50% <0.37%> (-0.06%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...a/org/apache/pinot/core/operator/BaseOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9CYXNlT3BlcmF0b3IuamF2YQ==) | `0.00% <0.00%> (-38.47%)` | :arrow_down: |
| [...g/apache/pinot/core/operator/DocIdSetOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9Eb2NJZFNldE9wZXJhdG9yLmphdmE=) | `0.00% <0.00%> (-93.11%)` | :arrow_down: |
| [...apache/pinot/core/operator/ProjectionOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9Qcm9qZWN0aW9uT3BlcmF0b3IuamF2YQ==) | `0.00% <0.00%> (-84.62%)` | :arrow_down: |
| [...t/core/operator/blocks/CombinedTransformBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvQ29tYmluZWRUcmFuc2Zvcm1CbG9jay5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ache/pinot/core/operator/blocks/DocIdSetBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvRG9jSWRTZXRCbG9jay5qYXZh) | `0.00% <0.00%> (-60.00%)` | :arrow_down: |
| [...he/pinot/core/operator/blocks/ProjectionBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvUHJvamVjdGlvbkJsb2NrLmphdmE=) | `0.00% <0.00%> (-60.00%)` | :arrow_down: |
| [...che/pinot/core/operator/blocks/TransformBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvVHJhbnNmb3JtQmxvY2suamF2YQ==) | `0.00% <0.00%> (-69.24%)` | :arrow_down: |
| [.../operator/filter/BlockDrivenAndFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvQmxvY2tEcml2ZW5BbmRGaWx0ZXJPcGVyYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...pinot/core/operator/query/AggregationOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9xdWVyeS9BZ2dyZWdhdGlvbk9wZXJhdG9yLmphdmE=) | `0.00% <0.00%> (-92.00%)` | :arrow_down: |
| [.../operator/transform/CombinedTransformOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vQ29tYmluZWRUcmFuc2Zvcm1PcGVyYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| ... and [1283 more](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [41c98d7...a0e92fd](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r764465715
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/FilteredClauseAggregationExecutor.java
##########
@@ -0,0 +1,87 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.pinot.core.operator.blocks.CombinedTransformBlock;
+import org.apache.pinot.core.operator.blocks.TransformBlock;
+import org.apache.pinot.core.query.aggregation.function.AggregationFunction;
+import org.apache.pinot.core.query.aggregation.function.AggregationFunctionUtils;
+
+
+public class FilteredClauseAggregationExecutor implements AggregationExecutor {
Review comment:
Brief javadoc please ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter commented on pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#issuecomment-979136818
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7830](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e1021d5) into [master](https://codecov.io/gh/apache/pinot/commit/13c9ee9556498bb6dc4ab60734743edb8b89773c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (13c9ee9) will **decrease** coverage by `56.93%`.
> The diff coverage is `0.00%`.
[](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7830 +/- ##
=============================================
- Coverage 71.49% 14.55% -56.94%
+ Complexity 4064 80 -3984
=============================================
Files 1577 1539 -38
Lines 80554 79161 -1393
Branches 11965 11839 -126
=============================================
- Hits 57592 11525 -46067
- Misses 19078 66790 +47712
+ Partials 3884 846 -3038
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `14.55% <0.00%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...a/org/apache/pinot/core/operator/BaseOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9CYXNlT3BlcmF0b3IuamF2YQ==) | `0.00% <0.00%> (-38.47%)` | :arrow_down: |
| [...g/apache/pinot/core/operator/DocIdSetOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9Eb2NJZFNldE9wZXJhdG9yLmphdmE=) | `0.00% <0.00%> (-92.31%)` | :arrow_down: |
| [...apache/pinot/core/operator/ProjectionOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9Qcm9qZWN0aW9uT3BlcmF0b3IuamF2YQ==) | `0.00% <0.00%> (-73.34%)` | :arrow_down: |
| [...t/core/operator/blocks/CombinedTransformBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvQ29tYmluZWRUcmFuc2Zvcm1CbG9jay5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...he/pinot/core/operator/blocks/ProjectionBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvUHJvamVjdGlvbkJsb2NrLmphdmE=) | `0.00% <0.00%> (-60.00%)` | :arrow_down: |
| [...che/pinot/core/operator/blocks/TransformBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvVHJhbnNmb3JtQmxvY2suamF2YQ==) | `0.00% <0.00%> (-69.24%)` | :arrow_down: |
| [.../operator/filter/BlockDrivenAndFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvQmxvY2tEcml2ZW5BbmRGaWx0ZXJPcGVyYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...pinot/core/operator/query/AggregationOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9xdWVyeS9BZ2dyZWdhdGlvbk9wZXJhdG9yLmphdmE=) | `0.00% <0.00%> (-94.45%)` | :arrow_down: |
| [.../operator/transform/CombinedTransformOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vQ29tYmluZWRUcmFuc2Zvcm1PcGVyYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...erator/transform/PassThroughTransformOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vUGFzc1Rocm91Z2hUcmFuc2Zvcm1PcGVyYXRvci5qYXZh) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
| ... and [1287 more](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [13c9ee9...e1021d5](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r764453959
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/FilteredAggregationsTest.java
##########
@@ -0,0 +1,307 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.queries;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import org.apache.commons.io.FileUtils;
+import org.apache.pinot.common.response.broker.BrokerResponseNative;
+import org.apache.pinot.common.response.broker.ResultTable;
+import org.apache.pinot.common.utils.DataSchema;
+import org.apache.pinot.segment.local.indexsegment.immutable.ImmutableSegmentLoader;
+import org.apache.pinot.segment.local.segment.creator.impl.SegmentIndexCreationDriverImpl;
+import org.apache.pinot.segment.local.segment.index.loader.IndexLoadingConfig;
+import org.apache.pinot.segment.local.segment.readers.GenericRowRecordReader;
+import org.apache.pinot.segment.spi.ImmutableSegment;
+import org.apache.pinot.segment.spi.IndexSegment;
+import org.apache.pinot.segment.spi.creator.SegmentGeneratorConfig;
+import org.apache.pinot.spi.config.table.FieldConfig;
+import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.data.readers.GenericRow;
+import org.apache.pinot.spi.data.readers.RecordReader;
+import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
+import org.testng.Assert;
+import org.testng.annotations.AfterClass;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+
+public class FilteredAggregationsTest extends BaseQueriesTest {
+ private static final File INDEX_DIR = new File(FileUtils.getTempDirectory(), "FilteredAggregationsTest");
+ private static final String TABLE_NAME = "MyTable";
+ private static final String FIRST_SEGMENT_NAME = "firstTestSegment";
+ private static final String SECOND_SEGMENT_NAME = "secondTestSegment";
+ private static final String INT_COL_NAME = "INT_COL";
+ private static final String NO_INDEX_INT_COL_NAME = "NO_INDEX_COL";
+ private static final Integer INT_BASE_VALUE = 0;
+ private static final Integer NUM_ROWS = 30000;
+
+
+ private IndexSegment _indexSegment;
+ private List<IndexSegment> _indexSegments;
+
+ @Override
+ protected String getFilter() {
+ return "";
+ }
+
+ @Override
+ protected IndexSegment getIndexSegment() {
+ return _indexSegment;
+ }
+
+ @Override
+ protected List<IndexSegment> getIndexSegments() {
+ return _indexSegments;
+ }
+
+ @BeforeClass
+ public void setUp()
+ throws Exception {
+ FileUtils.deleteQuietly(INDEX_DIR);
+
+ buildSegment(FIRST_SEGMENT_NAME);
+ buildSegment(SECOND_SEGMENT_NAME);
+ IndexLoadingConfig indexLoadingConfig = new IndexLoadingConfig();
+
+ Set<String> invertedIndexCols = new HashSet<>();
+ invertedIndexCols.add(INT_COL_NAME);
+
+ indexLoadingConfig.setInvertedIndexColumns(invertedIndexCols);
+ ImmutableSegment firstImmutableSegment =
+ ImmutableSegmentLoader.load(new File(INDEX_DIR, FIRST_SEGMENT_NAME), indexLoadingConfig);
+ ImmutableSegment secondImmutableSegment =
+ ImmutableSegmentLoader.load(new File(INDEX_DIR, SECOND_SEGMENT_NAME), indexLoadingConfig);
+ _indexSegment = firstImmutableSegment;
+ _indexSegments = Arrays.asList(firstImmutableSegment, secondImmutableSegment);
+ }
+
+ @AfterClass
+ public void tearDown() {
+ _indexSegment.destroy();
+ FileUtils.deleteQuietly(INDEX_DIR);
+ }
+
+ private List<GenericRow> createTestData(int numRows) {
+ List<GenericRow> rows = new ArrayList<>();
+
+ for (int i = 0; i < numRows; i++) {
+ GenericRow row = new GenericRow();
+ row.putField(INT_COL_NAME, INT_BASE_VALUE + i);
+ row.putField(NO_INDEX_INT_COL_NAME, i);
+
+ rows.add(row);
+ }
+ return rows;
+ }
+
+ private void buildSegment(String segmentName)
+ throws Exception {
+ List<GenericRow> rows = createTestData(NUM_ROWS);
+ List<FieldConfig> fieldConfigs = new ArrayList<>();
+
+ TableConfig tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME)
+ .setInvertedIndexColumns(Arrays.asList(INT_COL_NAME)).setFieldConfigList(fieldConfigs).build();
+ Schema schema = new Schema.SchemaBuilder().setSchemaName(TABLE_NAME)
+ .addSingleValueDimension(NO_INDEX_INT_COL_NAME, FieldSpec.DataType.INT)
+ .addMetric(INT_COL_NAME, FieldSpec.DataType.INT).build();
+ SegmentGeneratorConfig config = new SegmentGeneratorConfig(tableConfig, schema);
+ config.setOutDir(INDEX_DIR.getPath());
+ config.setTableName(TABLE_NAME);
+ config.setSegmentName(segmentName);
+
+ SegmentIndexCreationDriverImpl driver = new SegmentIndexCreationDriverImpl();
+ try (RecordReader recordReader = new GenericRowRecordReader(rows)) {
+ driver.init(config, recordReader);
+ driver.build();
+ }
+ }
+
+ private void testInterSegmentAggregationQueryHelper(String firstQuery, String secondQuery) {
+ // SQL
+ BrokerResponseNative firstBrokerResponseNative = getBrokerResponseForSqlQuery(firstQuery);
+ BrokerResponseNative secondBrokerResponseNative = getBrokerResponseForSqlQuery(secondQuery);
+ ResultTable firstResultTable = firstBrokerResponseNative.getResultTable();
+ ResultTable secondResultTable = secondBrokerResponseNative.getResultTable();
+ DataSchema firstDataSchema = firstResultTable.getDataSchema();
+ DataSchema secondDataSchema = secondResultTable.getDataSchema();
+
+ Assert.assertEquals(firstDataSchema.size(), secondDataSchema.size());
+
+ List<Object[]> firstSetOfRows = firstResultTable.getRows();
+ List<Object[]> secondSetOfRows = secondResultTable.getRows();
+
+ Assert.assertEquals(firstSetOfRows.size(), secondSetOfRows.size());
+
+ for (int i = 0; i < firstSetOfRows.size(); i++) {
+ Object[] firstSetRow = firstSetOfRows.get(i);
+ Object[] secondSetRow = secondSetOfRows.get(i);
+
+ Assert.assertEquals(firstSetRow.length, secondSetRow.length);
+
+ for (int j = 0; j < firstSetRow.length; j++) {
+ //System.out.println("FIRST " + firstSetRow[j] + " SECOND " + secondSetRow[j] + " j " + j);
+ Assert.assertEquals(firstSetRow[j], secondSetRow[j]);
+ }
+ }
+ }
+
+ @Test
+ public void testInterSegment() {
+
+ String query =
Review comment:
Seems like testing out filtering along with aggregation will get good coverage for this functionality. But can we still add a couple of GROUP BY queries as well just to make sure correctness is good. Something like
```
SELECT
col1, col2
AGG1 AS withoutFilterAgg1
AGG1 FILTER (....) AS withFilterAgg1
AGG2 ...
FROM .. GROUP BY col1, col2
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r764446789
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/InnerSegmentAggregationSingleValueQueriesTest.java
##########
@@ -49,27 +49,49 @@
" GROUP BY column1, column3, column6, column7, column9, column11, column12, column17, column18";
@Test
- public void testAggregationOnly() {
- String query = "SELECT" + AGGREGATION + " FROM testTable";
Review comment:
For aggregation tests with FILTER clause, can we please also have some tests where filter is on a transform function and not just identifier ? Let's also ensure to have all combinations tested - mix of aggregations with and without FILTER and with/without outer predicate
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r764484966
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java
##########
@@ -62,57 +68,31 @@ public AggregationPlanNode(IndexSegment indexSegment, QueryContext queryContext)
public Operator<IntermediateResultsBlock> run() {
assert _queryContext.getAggregationFunctions() != null;
- int numTotalDocs = _indexSegment.getSegmentMetadata().getTotalDocs();
- AggregationFunction[] aggregationFunctions = _queryContext.getAggregationFunctions();
+ boolean hasFilteredPredicates = _queryContext.getFilteredAggregationContexts()
+ .size() != 0;
- FilterPlanNode filterPlanNode = new FilterPlanNode(_indexSegment, _queryContext);
- BaseFilterOperator filterOperator = filterPlanNode.run();
+ Pair<Pair<BaseFilterOperator, TransformOperator>,
+ BaseOperator<IntermediateResultsBlock>> pair =
+ buildOperators(_queryContext.getFilter(), false);
- // Use metadata/dictionary to solve the query if possible
- // TODO: Use the same operator for both of them so that COUNT(*), MAX(col) can be optimized
- if (filterOperator.isResultMatchingAll()) {
- if (isFitForMetadataBasedPlan(aggregationFunctions)) {
- return new MetadataBasedAggregationOperator(aggregationFunctions, _indexSegment.getSegmentMetadata(),
- Collections.emptyMap());
- } else if (isFitForDictionaryBasedPlan(aggregationFunctions, _indexSegment)) {
- Map<String, Dictionary> dictionaryMap = new HashMap<>();
- for (AggregationFunction aggregationFunction : aggregationFunctions) {
- String column = ((ExpressionContext) aggregationFunction.getInputExpressions().get(0)).getIdentifier();
- dictionaryMap.computeIfAbsent(column, k -> _indexSegment.getDataSource(k).getDictionary());
- }
- return new DictionaryBasedAggregationOperator(aggregationFunctions, dictionaryMap, numTotalDocs);
- }
- }
+ if (hasFilteredPredicates) {
+ int numTotalDocs = _indexSegment.getSegmentMetadata().getTotalDocs();
+ AggregationFunction[] aggregationFunctions = _queryContext.getAggregationFunctions();
- // Use star-tree to solve the query if possible
- List<StarTreeV2> starTrees = _indexSegment.getStarTrees();
- if (starTrees != null && !StarTreeUtils.isStarTreeDisabled(_queryContext)) {
- AggregationFunctionColumnPair[] aggregationFunctionColumnPairs =
- StarTreeUtils.extractAggregationFunctionPairs(aggregationFunctions);
- if (aggregationFunctionColumnPairs != null) {
- Map<String, List<CompositePredicateEvaluator>> predicateEvaluatorsMap =
- StarTreeUtils.extractPredicateEvaluatorsMap(_indexSegment, _queryContext.getFilter(),
- filterPlanNode.getPredicateEvaluatorMap());
- if (predicateEvaluatorsMap != null) {
- for (StarTreeV2 starTreeV2 : starTrees) {
- if (StarTreeUtils.isFitForStarTree(starTreeV2.getMetadata(), aggregationFunctionColumnPairs, null,
- predicateEvaluatorsMap.keySet())) {
- TransformOperator transformOperator =
- new StarTreeTransformPlanNode(starTreeV2, aggregationFunctionColumnPairs, null,
- predicateEvaluatorsMap, _queryContext.getDebugOptions()).run();
- return new AggregationOperator(aggregationFunctions, transformOperator, numTotalDocs, true);
- }
- }
- }
- }
+ Set<ExpressionContext> expressionsToTransform =
+ AggregationFunctionUtils.collectExpressionsToTransform(aggregationFunctions, null);
+ BaseOperator<IntermediateResultsBlock> aggregationOperator = pair.getRight();
+
+ assert pair != null && aggregationOperator != null;
+
+ //TODO: For non star tree, non filtered aggregation, share the main filter transform operator
Review comment:
I suggest making this code a bit more readable. I find it super confusing because of all the Pair logic. Comments and descriptive variable names would be helpful
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r764475919
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/request/context/QueryContext.java
##########
@@ -441,34 +442,40 @@ public QueryContext build() {
*/
private void generateAggregationFunctions(QueryContext queryContext) {
List<AggregationFunction> aggregationFunctions = new ArrayList<>();
- List<Pair<AggregationFunction, FilterContext>> filteredAggregationFunctions = new ArrayList<>();
+ List<FilterContext> filteredAggregationFunctions = new ArrayList<>();
Map<FunctionContext, Integer> aggregationFunctionIndexMap = new HashMap<>();
// Add aggregation functions in the SELECT clause
// NOTE: DO NOT deduplicate the aggregation functions in the SELECT clause because that involves protocol change.
- List<FunctionContext> aggregationsInSelect = new ArrayList<>();
- List<Pair<FunctionContext, FilterContext>> filteredAggregations = new ArrayList<>();
+ List<Pair<Boolean, FunctionContext>> aggregationsInSelect = new ArrayList<>();
+ List<FilterContext> filteredAggregationContexts = new ArrayList<>();
for (ExpressionContext selectExpression : queryContext._selectExpressions) {
- getAggregations(selectExpression, aggregationsInSelect, filteredAggregations);
+ getAggregations(selectExpression, aggregationsInSelect,
+ filteredAggregationContexts);
}
- for (FunctionContext function : aggregationsInSelect) {
+ for (Pair<Boolean, FunctionContext> pair : aggregationsInSelect) {
+ FunctionContext function = pair.getRight();
int functionIndex = aggregationFunctions.size();
AggregationFunction aggregationFunction =
AggregationFunctionFactory.getAggregationFunction(function, queryContext);
+
+
+ aggregationFunction.setFilteredAggregation(pair.getLeft());
+
aggregationFunctions.add(aggregationFunction);
+
aggregationFunctionIndexMap.put(function, functionIndex);
}
- for (Pair<FunctionContext, FilterContext> pair : filteredAggregations) {
- AggregationFunction aggregationFunction =
- aggregationFunctions.get(aggregationFunctionIndexMap.get(pair.getLeft()));
- filteredAggregationFunctions.add(Pair.of(aggregationFunction, pair.getRight()));
+ for (FilterContext filterContext : filteredAggregationContexts) {
Review comment:
Why do we need to create two same lists for filtered aggregations ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r764330022
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BlockDrivenAndFilterOperator.java
##########
@@ -0,0 +1,126 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.filter;
+
+import java.util.ArrayList;
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.core.common.BlockDocIdSet;
+import org.apache.pinot.core.common.Operator;
+import org.apache.pinot.core.operator.VisitableOperator;
+import org.apache.pinot.core.operator.blocks.FilterBlock;
+import org.apache.pinot.core.operator.blocks.TransformBlock;
+import org.apache.pinot.core.operator.dociditerators.ArrayBasedDocIdIterator;
+import org.apache.pinot.core.operator.docidsets.AndDocIdSet;
+import org.apache.pinot.core.operator.docidsets.ArrayBasedDocIdSet;
+import org.apache.pinot.core.operator.docidsets.BitmapDocIdSet;
+import org.apache.pinot.core.operator.docidsets.FilterBlockDocIdSet;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
+
+/**
+ * Performs an AND operation on top of a Filter Block DocIDSet
+ * and a block from the given filter operator.
+ */
+public class BlockDrivenAndFilterOperator extends BaseFilterOperator
+ implements VisitableOperator {
+ private static final String OPERATOR_NAME = "BlockDrivenAndFilterOperator";
+
+ private final BaseFilterOperator _filterOperator;
+ private FilterBlockDocIdSet _filterBlockDocIdSet;
+ private final int _numDocs;
+
+ public BlockDrivenAndFilterOperator(BaseFilterOperator filterOperator, int numDocs) {
+ _filterOperator = filterOperator;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public FilterBlock getNextBlock() {
+
+ if (_filterBlockDocIdSet != null) {
+ List<FilterBlockDocIdSet> filterBlockDocIdSets = new ArrayList<>(2);
+
+ filterBlockDocIdSets.add(_filterBlockDocIdSet);
+ filterBlockDocIdSets.add(_filterOperator.nextBlock().getBlockDocIdSet());
Review comment:
The current (Existing) mechanism is to invoke the filter first completely. So the nextBlock() on root FilterOperator is called exactly once by DocIdSetOperator. It creates an iterator and then every call (per 10K records) from Transform to Project to DocIdSet operator iterates over the filtered block docIDIterator.
I think the processing done over the child operators in this new operator in the if block is same as being done currently in the `AndFilterOperator::getNextBlock` (which is called exactly once). But are you expecting `getNextBlock()` of `BlockDrivenAndFilterOperator` to be called multiple times ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r764484966
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java
##########
@@ -62,57 +68,31 @@ public AggregationPlanNode(IndexSegment indexSegment, QueryContext queryContext)
public Operator<IntermediateResultsBlock> run() {
assert _queryContext.getAggregationFunctions() != null;
- int numTotalDocs = _indexSegment.getSegmentMetadata().getTotalDocs();
- AggregationFunction[] aggregationFunctions = _queryContext.getAggregationFunctions();
+ boolean hasFilteredPredicates = _queryContext.getFilteredAggregationContexts()
+ .size() != 0;
- FilterPlanNode filterPlanNode = new FilterPlanNode(_indexSegment, _queryContext);
- BaseFilterOperator filterOperator = filterPlanNode.run();
+ Pair<Pair<BaseFilterOperator, TransformOperator>,
+ BaseOperator<IntermediateResultsBlock>> pair =
+ buildOperators(_queryContext.getFilter(), false);
- // Use metadata/dictionary to solve the query if possible
- // TODO: Use the same operator for both of them so that COUNT(*), MAX(col) can be optimized
- if (filterOperator.isResultMatchingAll()) {
- if (isFitForMetadataBasedPlan(aggregationFunctions)) {
- return new MetadataBasedAggregationOperator(aggregationFunctions, _indexSegment.getSegmentMetadata(),
- Collections.emptyMap());
- } else if (isFitForDictionaryBasedPlan(aggregationFunctions, _indexSegment)) {
- Map<String, Dictionary> dictionaryMap = new HashMap<>();
- for (AggregationFunction aggregationFunction : aggregationFunctions) {
- String column = ((ExpressionContext) aggregationFunction.getInputExpressions().get(0)).getIdentifier();
- dictionaryMap.computeIfAbsent(column, k -> _indexSegment.getDataSource(k).getDictionary());
- }
- return new DictionaryBasedAggregationOperator(aggregationFunctions, dictionaryMap, numTotalDocs);
- }
- }
+ if (hasFilteredPredicates) {
+ int numTotalDocs = _indexSegment.getSegmentMetadata().getTotalDocs();
+ AggregationFunction[] aggregationFunctions = _queryContext.getAggregationFunctions();
- // Use star-tree to solve the query if possible
- List<StarTreeV2> starTrees = _indexSegment.getStarTrees();
- if (starTrees != null && !StarTreeUtils.isStarTreeDisabled(_queryContext)) {
- AggregationFunctionColumnPair[] aggregationFunctionColumnPairs =
- StarTreeUtils.extractAggregationFunctionPairs(aggregationFunctions);
- if (aggregationFunctionColumnPairs != null) {
- Map<String, List<CompositePredicateEvaluator>> predicateEvaluatorsMap =
- StarTreeUtils.extractPredicateEvaluatorsMap(_indexSegment, _queryContext.getFilter(),
- filterPlanNode.getPredicateEvaluatorMap());
- if (predicateEvaluatorsMap != null) {
- for (StarTreeV2 starTreeV2 : starTrees) {
- if (StarTreeUtils.isFitForStarTree(starTreeV2.getMetadata(), aggregationFunctionColumnPairs, null,
- predicateEvaluatorsMap.keySet())) {
- TransformOperator transformOperator =
- new StarTreeTransformPlanNode(starTreeV2, aggregationFunctionColumnPairs, null,
- predicateEvaluatorsMap, _queryContext.getDebugOptions()).run();
- return new AggregationOperator(aggregationFunctions, transformOperator, numTotalDocs, true);
- }
- }
- }
- }
+ Set<ExpressionContext> expressionsToTransform =
+ AggregationFunctionUtils.collectExpressionsToTransform(aggregationFunctions, null);
+ BaseOperator<IntermediateResultsBlock> aggregationOperator = pair.getRight();
+
+ assert pair != null && aggregationOperator != null;
+
+ //TODO: For non star tree, non filtered aggregation, share the main filter transform operator
Review comment:
I suggest making code in this classs a bit more readable. I find it super confusing because of all the Pair logic. Comments and descriptive variable names would be helpful
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r764330022
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BlockDrivenAndFilterOperator.java
##########
@@ -0,0 +1,126 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.filter;
+
+import java.util.ArrayList;
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.core.common.BlockDocIdSet;
+import org.apache.pinot.core.common.Operator;
+import org.apache.pinot.core.operator.VisitableOperator;
+import org.apache.pinot.core.operator.blocks.FilterBlock;
+import org.apache.pinot.core.operator.blocks.TransformBlock;
+import org.apache.pinot.core.operator.dociditerators.ArrayBasedDocIdIterator;
+import org.apache.pinot.core.operator.docidsets.AndDocIdSet;
+import org.apache.pinot.core.operator.docidsets.ArrayBasedDocIdSet;
+import org.apache.pinot.core.operator.docidsets.BitmapDocIdSet;
+import org.apache.pinot.core.operator.docidsets.FilterBlockDocIdSet;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
+
+/**
+ * Performs an AND operation on top of a Filter Block DocIDSet
+ * and a block from the given filter operator.
+ */
+public class BlockDrivenAndFilterOperator extends BaseFilterOperator
+ implements VisitableOperator {
+ private static final String OPERATOR_NAME = "BlockDrivenAndFilterOperator";
+
+ private final BaseFilterOperator _filterOperator;
+ private FilterBlockDocIdSet _filterBlockDocIdSet;
+ private final int _numDocs;
+
+ public BlockDrivenAndFilterOperator(BaseFilterOperator filterOperator, int numDocs) {
+ _filterOperator = filterOperator;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public FilterBlock getNextBlock() {
+
+ if (_filterBlockDocIdSet != null) {
+ List<FilterBlockDocIdSet> filterBlockDocIdSets = new ArrayList<>(2);
+
+ filterBlockDocIdSets.add(_filterBlockDocIdSet);
+ filterBlockDocIdSets.add(_filterOperator.nextBlock().getBlockDocIdSet());
Review comment:
The current (Existing) mechanism is to invoke the filter first completely. So the nextBlock() on root FilterOperator is called exactly once by DocIdSetOperator. It creates an iterator and then every call (per 10K records) from Transform to Project to DocIdSet operator iterates over the already processed filtered block docIDIterator.
I think the processing done over the child operators in this new operator in the if block is same as being done currently in the AndFilterOperator::getNextBlock (which is called exactly once). But are you expecting getNextBlock() of BlockDrivenAndFilterOperator to be called multiple times ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r764330022
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BlockDrivenAndFilterOperator.java
##########
@@ -0,0 +1,126 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.filter;
+
+import java.util.ArrayList;
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.core.common.BlockDocIdSet;
+import org.apache.pinot.core.common.Operator;
+import org.apache.pinot.core.operator.VisitableOperator;
+import org.apache.pinot.core.operator.blocks.FilterBlock;
+import org.apache.pinot.core.operator.blocks.TransformBlock;
+import org.apache.pinot.core.operator.dociditerators.ArrayBasedDocIdIterator;
+import org.apache.pinot.core.operator.docidsets.AndDocIdSet;
+import org.apache.pinot.core.operator.docidsets.ArrayBasedDocIdSet;
+import org.apache.pinot.core.operator.docidsets.BitmapDocIdSet;
+import org.apache.pinot.core.operator.docidsets.FilterBlockDocIdSet;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
+
+/**
+ * Performs an AND operation on top of a Filter Block DocIDSet
+ * and a block from the given filter operator.
+ */
+public class BlockDrivenAndFilterOperator extends BaseFilterOperator
+ implements VisitableOperator {
+ private static final String OPERATOR_NAME = "BlockDrivenAndFilterOperator";
+
+ private final BaseFilterOperator _filterOperator;
+ private FilterBlockDocIdSet _filterBlockDocIdSet;
+ private final int _numDocs;
+
+ public BlockDrivenAndFilterOperator(BaseFilterOperator filterOperator, int numDocs) {
+ _filterOperator = filterOperator;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public FilterBlock getNextBlock() {
+
+ if (_filterBlockDocIdSet != null) {
+ List<FilterBlockDocIdSet> filterBlockDocIdSets = new ArrayList<>(2);
+
+ filterBlockDocIdSets.add(_filterBlockDocIdSet);
+ filterBlockDocIdSets.add(_filterOperator.nextBlock().getBlockDocIdSet());
Review comment:
The current (Existing) mechanism is to invoke the filter first completely. So the nextBlock() on root FilterOperator is called exactly once by DocIdSetOperator. It creates an iterator and then every call (per 10K records) from Transform to Project to DocIdSet operator iterates over the filtered block docIDIterator.
I think the processing done over the child operators in this new operator in the if block is same as being done currently in the AndFilterOperator::getNextBlock (which is called exactly once). But are you expecting getNextBlock() of BlockDrivenAndFilterOperator to be called multiple times ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r764503815
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BlockDrivenAndFilterOperator.java
##########
@@ -0,0 +1,126 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.filter;
+
+import java.util.ArrayList;
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.core.common.BlockDocIdSet;
+import org.apache.pinot.core.common.Operator;
+import org.apache.pinot.core.operator.VisitableOperator;
+import org.apache.pinot.core.operator.blocks.FilterBlock;
+import org.apache.pinot.core.operator.blocks.TransformBlock;
+import org.apache.pinot.core.operator.dociditerators.ArrayBasedDocIdIterator;
+import org.apache.pinot.core.operator.docidsets.AndDocIdSet;
+import org.apache.pinot.core.operator.docidsets.ArrayBasedDocIdSet;
+import org.apache.pinot.core.operator.docidsets.BitmapDocIdSet;
+import org.apache.pinot.core.operator.docidsets.FilterBlockDocIdSet;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
+
+/**
+ * Performs an AND operation on top of a Filter Block DocIDSet
+ * and a block from the given filter operator.
+ */
+public class BlockDrivenAndFilterOperator extends BaseFilterOperator
+ implements VisitableOperator {
+ private static final String OPERATOR_NAME = "BlockDrivenAndFilterOperator";
+
+ private final BaseFilterOperator _filterOperator;
+ private FilterBlockDocIdSet _filterBlockDocIdSet;
+ private final int _numDocs;
+
+ public BlockDrivenAndFilterOperator(BaseFilterOperator filterOperator, int numDocs) {
+ _filterOperator = filterOperator;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public FilterBlock getNextBlock() {
+
+ if (_filterBlockDocIdSet != null) {
+ List<FilterBlockDocIdSet> filterBlockDocIdSets = new ArrayList<>(2);
+
+ filterBlockDocIdSets.add(_filterBlockDocIdSet);
+ filterBlockDocIdSets.add(_filterOperator.nextBlock().getBlockDocIdSet());
+
+ return new FilterBlock(new AndDocIdSet(filterBlockDocIdSets));
+ }
+
+ return _filterOperator.nextBlock();
+ }
+
+ @Override
+ public String getOperatorName() {
+ return OPERATOR_NAME;
+ }
+
+ @Override
+ public List<Operator> getChildOperators() {
+ return null;
+ }
+
+ @Nullable
+ @Override
+ public String toExplainString() {
+ return null;
+ }
+
+ @Override
+ public <T> void accept(T incomingObject) {
Review comment:
I am going to pull the code locally to understand the flow better. But, is it correct that this is being called to accept the outer transform block for the main WHERE clause ?
Is there a strong need to add these visit/accept semantics ? The flow from execution perspective is not super clear at this point on how to use these methods.
When we know that BlockDrivenAndFilterOperator is going to be created (at least as of now) for FILTER aggregation queries, then we know the execution plan ahead of time. So, I don't think we need any custom transformation to happen on the nodes of a tree (which is where Visitor pattern might be more helpful afaik). The use does not look very intuitive to me at this point.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r764474811
##########
File path: pinot-core/src/test/java/org/apache/pinot/core/query/request/context/utils/BrokerRequestToQueryContextConverterTest.java
##########
@@ -581,14 +579,12 @@ public void testHardcodedQueries() {
public void testFilteredAggregations() {
Review comment:
Can you please add a test where filter inside FILTER clause is on a function and/or combination of identifier and function ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#issuecomment-979136818
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7830](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8d3dc1b) into [master](https://codecov.io/gh/apache/pinot/commit/13c9ee9556498bb6dc4ab60734743edb8b89773c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (13c9ee9) will **decrease** coverage by `43.60%`.
> The diff coverage is `30.15%`.
[](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7830 +/- ##
=============================================
- Coverage 71.49% 27.88% -43.61%
=============================================
Files 1577 1575 -2
Lines 80554 80656 +102
Branches 11965 12011 +46
=============================================
- Hits 57592 22493 -35099
- Misses 19078 56113 +37035
+ Partials 3884 2050 -1834
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `27.88% <30.15%> (+0.01%)` | :arrow_up: |
| unittests1 | `?` | |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...g/apache/pinot/core/operator/DocIdSetOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9Eb2NJZFNldE9wZXJhdG9yLmphdmE=) | `85.71% <0.00%> (-6.60%)` | :arrow_down: |
| [...t/core/operator/blocks/CombinedTransformBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvQ29tYmluZWRUcmFuc2Zvcm1CbG9jay5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...che/pinot/core/operator/blocks/TransformBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvVHJhbnNmb3JtQmxvY2suamF2YQ==) | `69.23% <0.00%> (ø)` | |
| [.../operator/filter/BlockDrivenAndFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvQmxvY2tEcml2ZW5BbmRGaWx0ZXJPcGVyYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [.../operator/transform/CombinedTransformOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vQ29tYmluZWRUcmFuc2Zvcm1PcGVyYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...erator/transform/PassThroughTransformOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vUGFzc1Rocm91Z2hUcmFuc2Zvcm1PcGVyYXRvci5qYXZh) | `66.66% <0.00%> (-19.05%)` | :arrow_down: |
| [...not/core/operator/transform/TransformOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vVHJhbnNmb3JtT3BlcmF0b3IuamF2YQ==) | `86.36% <0.00%> (-8.64%)` | :arrow_down: |
| [...t/core/plan/AggregationGroupByOrderByPlanNode.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0FnZ3JlZ2F0aW9uR3JvdXBCeU9yZGVyQnlQbGFuTm9kZS5qYXZh) | `45.45% <0.00%> (ø)` | |
| [...he/pinot/core/plan/AggregationGroupByPlanNode.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0FnZ3JlZ2F0aW9uR3JvdXBCeVBsYW5Ob2RlLmphdmE=) | `45.45% <0.00%> (-36.37%)` | :arrow_down: |
| [...aggregation/FilteredClauseAggregationExecutor.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9GaWx0ZXJlZENsYXVzZUFnZ3JlZ2F0aW9uRXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| ... and [1157 more](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [13c9ee9...8d3dc1b](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#issuecomment-979136818
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7830](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e51d55b) into [master](https://codecov.io/gh/apache/pinot/commit/47e49ecd6e11aebe74f8868cdac22051b175d4c5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47e49ec) will **decrease** coverage by `42.22%`.
> The diff coverage is `30.91%`.
[](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7830 +/- ##
=============================================
- Coverage 71.63% 29.40% -42.23%
=============================================
Files 1580 1577 -3
Lines 81100 81192 +92
Branches 12068 12122 +54
=============================================
- Hits 58093 23873 -34220
- Misses 19092 55177 +36085
+ Partials 3915 2142 -1773
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `29.40% <30.91%> (+<0.01%)` | :arrow_up: |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...g/apache/pinot/core/operator/DocIdSetOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9Eb2NJZFNldE9wZXJhdG9yLmphdmE=) | `87.09% <0.00%> (-6.01%)` | :arrow_down: |
| [...t/core/operator/blocks/CombinedTransformBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvQ29tYmluZWRUcmFuc2Zvcm1CbG9jay5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...che/pinot/core/operator/blocks/TransformBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvVHJhbnNmb3JtQmxvY2suamF2YQ==) | `69.23% <0.00%> (ø)` | |
| [.../operator/filter/BlockDrivenAndFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvQmxvY2tEcml2ZW5BbmRGaWx0ZXJPcGVyYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [.../operator/transform/CombinedTransformOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vQ29tYmluZWRUcmFuc2Zvcm1PcGVyYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...erator/transform/PassThroughTransformOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vUGFzc1Rocm91Z2hUcmFuc2Zvcm1PcGVyYXRvci5qYXZh) | `70.00% <0.00%> (-17.50%)` | :arrow_down: |
| [...t/core/plan/AggregationGroupByOrderByPlanNode.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0FnZ3JlZ2F0aW9uR3JvdXBCeU9yZGVyQnlQbGFuTm9kZS5qYXZh) | `45.45% <0.00%> (ø)` | |
| [...aggregation/FilteredClauseAggregationExecutor.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9GaWx0ZXJlZENsYXVzZUFnZ3JlZ2F0aW9uRXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...aggregation/function/AggregationFunctionUtils.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9BZ2dyZWdhdGlvbkZ1bmN0aW9uVXRpbHMuamF2YQ==) | `60.00% <0.00%> (-27.24%)` | :arrow_down: |
| [...n/function/BaseSingleInputAggregationFunction.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9CYXNlU2luZ2xlSW5wdXRBZ2dyZWdhdGlvbkZ1bmN0aW9uLmphdmE=) | `46.15% <ø> (-30.77%)` | :arrow_down: |
| ... and [1157 more](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [41c98d7...e51d55b](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#issuecomment-979136818
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7830](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (763c5c0) into [master](https://codecov.io/gh/apache/pinot/commit/47e49ecd6e11aebe74f8868cdac22051b175d4c5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47e49ec) will **decrease** coverage by `57.25%`.
> The diff coverage is `0.37%`.
[](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7830 +/- ##
=============================================
- Coverage 71.63% 14.38% -57.26%
+ Complexity 4088 80 -4008
=============================================
Files 1580 1542 -38
Lines 81100 80186 -914
Branches 12068 12066 -2
=============================================
- Hits 58093 11531 -46562
- Misses 19092 67805 +48713
+ Partials 3915 850 -3065
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `?` | |
| unittests1 | `?` | |
| unittests2 | `14.38% <0.37%> (-0.18%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...a/org/apache/pinot/core/operator/BaseOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9CYXNlT3BlcmF0b3IuamF2YQ==) | `0.00% <0.00%> (-38.47%)` | :arrow_down: |
| [...g/apache/pinot/core/operator/DocIdSetOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9Eb2NJZFNldE9wZXJhdG9yLmphdmE=) | `0.00% <0.00%> (-93.11%)` | :arrow_down: |
| [...apache/pinot/core/operator/ProjectionOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9Qcm9qZWN0aW9uT3BlcmF0b3IuamF2YQ==) | `0.00% <0.00%> (-84.62%)` | :arrow_down: |
| [...t/core/operator/blocks/CombinedTransformBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvQ29tYmluZWRUcmFuc2Zvcm1CbG9jay5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ache/pinot/core/operator/blocks/DocIdSetBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvRG9jSWRTZXRCbG9jay5qYXZh) | `0.00% <0.00%> (-60.00%)` | :arrow_down: |
| [...he/pinot/core/operator/blocks/ProjectionBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvUHJvamVjdGlvbkJsb2NrLmphdmE=) | `0.00% <0.00%> (-60.00%)` | :arrow_down: |
| [...che/pinot/core/operator/blocks/TransformBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvVHJhbnNmb3JtQmxvY2suamF2YQ==) | `0.00% <0.00%> (-69.24%)` | :arrow_down: |
| [.../operator/filter/BlockDrivenAndFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvQmxvY2tEcml2ZW5BbmRGaWx0ZXJPcGVyYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...pinot/core/operator/query/AggregationOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9xdWVyeS9BZ2dyZWdhdGlvbk9wZXJhdG9yLmphdmE=) | `0.00% <0.00%> (-92.00%)` | :arrow_down: |
| [.../operator/transform/CombinedTransformOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vQ29tYmluZWRUcmFuc2Zvcm1PcGVyYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| ... and [1282 more](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [41c98d7...763c5c0](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#issuecomment-979136818
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7830](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (763c5c0) into [master](https://codecov.io/gh/apache/pinot/commit/47e49ecd6e11aebe74f8868cdac22051b175d4c5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47e49ec) will **decrease** coverage by `35.39%`.
> The diff coverage is `30.56%`.
[](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7830 +/- ##
=============================================
- Coverage 71.63% 36.23% -35.40%
+ Complexity 4088 80 -4008
=============================================
Files 1580 1587 +7
Lines 81100 82056 +956
Branches 12068 12269 +201
=============================================
- Hits 58093 29730 -28363
- Misses 19092 49968 +30876
+ Partials 3915 2358 -1557
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `?` | |
| integration2 | `27.62% <30.56%> (-0.28%)` | :arrow_down: |
| unittests1 | `?` | |
| unittests2 | `14.38% <0.37%> (-0.18%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...g/apache/pinot/core/operator/DocIdSetOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9Eb2NJZFNldE9wZXJhdG9yLmphdmE=) | `87.09% <0.00%> (-6.01%)` | :arrow_down: |
| [...t/core/operator/blocks/CombinedTransformBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvQ29tYmluZWRUcmFuc2Zvcm1CbG9jay5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ache/pinot/core/operator/blocks/DocIdSetBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvRG9jSWRTZXRCbG9jay5qYXZh) | `60.00% <0.00%> (ø)` | |
| [...che/pinot/core/operator/blocks/TransformBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvVHJhbnNmb3JtQmxvY2suamF2YQ==) | `69.23% <0.00%> (ø)` | |
| [.../operator/filter/BlockDrivenAndFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvQmxvY2tEcml2ZW5BbmRGaWx0ZXJPcGVyYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [.../operator/transform/CombinedTransformOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vQ29tYmluZWRUcmFuc2Zvcm1PcGVyYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...erator/transform/PassThroughTransformOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vUGFzc1Rocm91Z2hUcmFuc2Zvcm1PcGVyYXRvci5qYXZh) | `70.00% <0.00%> (-17.50%)` | :arrow_down: |
| [...t/core/plan/AggregationGroupByOrderByPlanNode.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0FnZ3JlZ2F0aW9uR3JvdXBCeU9yZGVyQnlQbGFuTm9kZS5qYXZh) | `45.45% <0.00%> (ø)` | |
| [...he/pinot/core/plan/AggregationGroupByPlanNode.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0FnZ3JlZ2F0aW9uR3JvdXBCeVBsYW5Ob2RlLmphdmE=) | `45.45% <0.00%> (-36.37%)` | :arrow_down: |
| [...aggregation/FilteredClauseAggregationExecutor.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9GaWx0ZXJlZENsYXVzZUFnZ3JlZ2F0aW9uRXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| ... and [980 more](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [41c98d7...763c5c0](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] atris commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
atris commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r763700805
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BlockDrivenAndFilterOperator.java
##########
@@ -0,0 +1,126 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.filter;
+
+import java.util.ArrayList;
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.core.common.BlockDocIdSet;
+import org.apache.pinot.core.common.Operator;
+import org.apache.pinot.core.operator.VisitableOperator;
+import org.apache.pinot.core.operator.blocks.FilterBlock;
+import org.apache.pinot.core.operator.blocks.TransformBlock;
+import org.apache.pinot.core.operator.dociditerators.ArrayBasedDocIdIterator;
+import org.apache.pinot.core.operator.docidsets.AndDocIdSet;
+import org.apache.pinot.core.operator.docidsets.ArrayBasedDocIdSet;
+import org.apache.pinot.core.operator.docidsets.BitmapDocIdSet;
+import org.apache.pinot.core.operator.docidsets.FilterBlockDocIdSet;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
+
+/**
+ * Performs an AND operation on top of a Filter Block DocIDSet
+ * and a block from the given filter operator.
+ */
+public class BlockDrivenAndFilterOperator extends BaseFilterOperator
+ implements VisitableOperator {
+ private static final String OPERATOR_NAME = "BlockDrivenAndFilterOperator";
+
+ private final BaseFilterOperator _filterOperator;
+ private FilterBlockDocIdSet _filterBlockDocIdSet;
+ private final int _numDocs;
+
+ public BlockDrivenAndFilterOperator(BaseFilterOperator filterOperator, int numDocs) {
+ _filterOperator = filterOperator;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public FilterBlock getNextBlock() {
+
+ if (_filterBlockDocIdSet != null) {
+ List<FilterBlockDocIdSet> filterBlockDocIdSets = new ArrayList<>(2);
+
+ filterBlockDocIdSets.add(_filterBlockDocIdSet);
+ filterBlockDocIdSets.add(_filterOperator.nextBlock().getBlockDocIdSet());
Review comment:
I am not sure if I understand. Referring to AndFilterOperator, all child FilterOperators are invoked on each call of nextBlock() invocation of the parent operator, hence each child operator generates a block on a single call of getNextBlock() for the parent. Isn't that exactly what this operator is doing as well?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
Jackie-Jiang commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r763594347
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BlockDrivenAndFilterOperator.java
##########
@@ -0,0 +1,126 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.filter;
+
+import java.util.ArrayList;
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.core.common.BlockDocIdSet;
+import org.apache.pinot.core.common.Operator;
+import org.apache.pinot.core.operator.VisitableOperator;
+import org.apache.pinot.core.operator.blocks.FilterBlock;
+import org.apache.pinot.core.operator.blocks.TransformBlock;
+import org.apache.pinot.core.operator.dociditerators.ArrayBasedDocIdIterator;
+import org.apache.pinot.core.operator.docidsets.AndDocIdSet;
+import org.apache.pinot.core.operator.docidsets.ArrayBasedDocIdSet;
+import org.apache.pinot.core.operator.docidsets.BitmapDocIdSet;
+import org.apache.pinot.core.operator.docidsets.FilterBlockDocIdSet;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
+
+/**
+ * Performs an AND operation on top of a Filter Block DocIDSet
+ * and a block from the given filter operator.
+ */
+public class BlockDrivenAndFilterOperator extends BaseFilterOperator
+ implements VisitableOperator {
+ private static final String OPERATOR_NAME = "BlockDrivenAndFilterOperator";
+
+ private final BaseFilterOperator _filterOperator;
+ private FilterBlockDocIdSet _filterBlockDocIdSet;
+ private final int _numDocs;
+
+ public BlockDrivenAndFilterOperator(BaseFilterOperator filterOperator, int numDocs) {
+ _filterOperator = filterOperator;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public FilterBlock getNextBlock() {
+
+ if (_filterBlockDocIdSet != null) {
+ List<FilterBlockDocIdSet> filterBlockDocIdSets = new ArrayList<>(2);
+
+ filterBlockDocIdSets.add(_filterBlockDocIdSet);
+ filterBlockDocIdSets.add(_filterOperator.nextBlock().getBlockDocIdSet());
Review comment:
`FilterOperator.nextBlock()` should only be called once. Currently it is called per block from the `TransformOperator` which can cause problems
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/function/FilterableAggregation.java
##########
@@ -0,0 +1,31 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation.function;
+
+public class FilterableAggregation {
Review comment:
Why do we need this class? Whether an aggregation is filtered or not should not be passed via the `AggregationFunction`
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/DocIdSetBlock.java
##########
@@ -56,7 +57,7 @@ public BlockDocIdValueSet getBlockDocIdValueSet() {
@Override
public BlockDocIdSet getBlockDocIdSet() {
- return new ArrayBasedDocIdSet(_docIdArray, _searchableLength);
+ return new BitmapDocIdSet(ImmutableRoaringBitmap.bitmapOf(_docIdArray), _searchableLength);
Review comment:
This can potentially cause performance regression for non-filtered queries
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r764500110
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/operator/filter/BlockDrivenAndFilterOperator.java
##########
@@ -0,0 +1,126 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.operator.filter;
+
+import java.util.ArrayList;
+import java.util.List;
+import javax.annotation.Nullable;
+import org.apache.pinot.core.common.BlockDocIdSet;
+import org.apache.pinot.core.common.Operator;
+import org.apache.pinot.core.operator.VisitableOperator;
+import org.apache.pinot.core.operator.blocks.FilterBlock;
+import org.apache.pinot.core.operator.blocks.TransformBlock;
+import org.apache.pinot.core.operator.dociditerators.ArrayBasedDocIdIterator;
+import org.apache.pinot.core.operator.docidsets.AndDocIdSet;
+import org.apache.pinot.core.operator.docidsets.ArrayBasedDocIdSet;
+import org.apache.pinot.core.operator.docidsets.BitmapDocIdSet;
+import org.apache.pinot.core.operator.docidsets.FilterBlockDocIdSet;
+import org.apache.pinot.segment.spi.Constants;
+import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
+
+/**
+ * Performs an AND operation on top of a Filter Block DocIDSet
+ * and a block from the given filter operator.
+ */
+public class BlockDrivenAndFilterOperator extends BaseFilterOperator
+ implements VisitableOperator {
+ private static final String OPERATOR_NAME = "BlockDrivenAndFilterOperator";
+
+ private final BaseFilterOperator _filterOperator;
+ private FilterBlockDocIdSet _filterBlockDocIdSet;
+ private final int _numDocs;
+
+ public BlockDrivenAndFilterOperator(BaseFilterOperator filterOperator, int numDocs) {
+ _filterOperator = filterOperator;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public FilterBlock getNextBlock() {
+
+ if (_filterBlockDocIdSet != null) {
+ List<FilterBlockDocIdSet> filterBlockDocIdSets = new ArrayList<>(2);
Review comment:
Why 2 ? One for the outer WHERE clause and there should be 1 each for the inner/child FILTER clauses and all of these need to be intersected together
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] amrishlal commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
amrishlal commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r765327106
##########
File path: pinot-core/src/test/java/org/apache/pinot/queries/InnerSegmentAggregationSingleValueQueriesTest.java
##########
@@ -49,27 +49,49 @@
" GROUP BY column1, column3, column6, column7, column9, column11, column12, column17, column18";
@Test
- public void testAggregationOnly() {
- String query = "SELECT" + AGGREGATION + " FROM testTable";
Review comment:
Why are we modifying an existing test case. Is the existing test case no longer valid? If not, I would suggest leaving the existing test case as is and adding a new test case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] codecov-commenter edited a comment on pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#issuecomment-979136818
# [Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#7830](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (763c5c0) into [master](https://codecov.io/gh/apache/pinot/commit/47e49ecd6e11aebe74f8868cdac22051b175d4c5?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (47e49ec) will **decrease** coverage by `33.48%`.
> The diff coverage is `31.32%`.
[](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #7830 +/- ##
=============================================
- Coverage 71.63% 38.14% -33.49%
+ Complexity 4088 80 -4008
=============================================
Files 1580 1587 +7
Lines 81100 82056 +956
Branches 12068 12269 +201
=============================================
- Hits 58093 31302 -26791
- Misses 19092 48349 +29257
+ Partials 3915 2405 -1510
```
| Flag | Coverage Δ | |
|---|---|---|
| integration1 | `29.20% <31.32%> (-0.20%)` | :arrow_down: |
| integration2 | `27.62% <30.56%> (-0.28%)` | :arrow_down: |
| unittests1 | `?` | |
| unittests2 | `14.38% <0.37%> (-0.18%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...g/apache/pinot/core/operator/DocIdSetOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9Eb2NJZFNldE9wZXJhdG9yLmphdmE=) | `87.09% <0.00%> (-6.01%)` | :arrow_down: |
| [...t/core/operator/blocks/CombinedTransformBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvQ29tYmluZWRUcmFuc2Zvcm1CbG9jay5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ache/pinot/core/operator/blocks/DocIdSetBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvRG9jSWRTZXRCbG9jay5qYXZh) | `60.00% <0.00%> (ø)` | |
| [...che/pinot/core/operator/blocks/TransformBlock.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvVHJhbnNmb3JtQmxvY2suamF2YQ==) | `69.23% <0.00%> (ø)` | |
| [.../operator/filter/BlockDrivenAndFilterOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvQmxvY2tEcml2ZW5BbmRGaWx0ZXJPcGVyYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [.../operator/transform/CombinedTransformOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vQ29tYmluZWRUcmFuc2Zvcm1PcGVyYXRvci5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...erator/transform/PassThroughTransformOperator.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vUGFzc1Rocm91Z2hUcmFuc2Zvcm1PcGVyYXRvci5qYXZh) | `70.00% <0.00%> (-17.50%)` | :arrow_down: |
| [...t/core/plan/AggregationGroupByOrderByPlanNode.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0FnZ3JlZ2F0aW9uR3JvdXBCeU9yZGVyQnlQbGFuTm9kZS5qYXZh) | `45.45% <0.00%> (ø)` | |
| [...aggregation/FilteredClauseAggregationExecutor.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9GaWx0ZXJlZENsYXVzZUFnZ3JlZ2F0aW9uRXhlY3V0b3IuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...aggregation/function/AggregationFunctionUtils.java](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9xdWVyeS9hZ2dyZWdhdGlvbi9mdW5jdGlvbi9BZ2dyZWdhdGlvbkZ1bmN0aW9uVXRpbHMuamF2YQ==) | `70.90% <0.00%> (-16.33%)` | :arrow_down: |
| ... and [913 more](https://codecov.io/gh/apache/pinot/pull/7830/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [41c98d7...763c5c0](https://codecov.io/gh/apache/pinot/pull/7830?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r764467965
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/aggregation/FilteredClauseAggregationExecutor.java
##########
@@ -0,0 +1,87 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pinot.core.query.aggregation;
+
+import java.util.ArrayList;
+import java.util.List;
+import org.apache.pinot.core.operator.blocks.CombinedTransformBlock;
+import org.apache.pinot.core.operator.blocks.TransformBlock;
+import org.apache.pinot.core.query.aggregation.function.AggregationFunction;
+import org.apache.pinot.core.query.aggregation.function.AggregationFunctionUtils;
+
+
+public class FilteredClauseAggregationExecutor implements AggregationExecutor {
+ protected final AggregationFunction[] _aggregationFunctions;
+ protected final AggregationResultHolder[] _aggregationResultHolders;
+
+ public FilteredClauseAggregationExecutor(AggregationFunction[] aggregationFunctions) {
+ _aggregationFunctions = aggregationFunctions;
+ int numAggregationFunctions = aggregationFunctions.length;
+ _aggregationResultHolders = new AggregationResultHolder[numAggregationFunctions];
+ for (int i = 0; i < numAggregationFunctions; i++) {
+ _aggregationResultHolders[i] = aggregationFunctions[i].createAggregationResultHolder();
+ }
+ }
+
+ @Override
+ public void aggregate(TransformBlock transformBlock) {
+ if (!(transformBlock instanceof CombinedTransformBlock)) {
+ throw new IllegalArgumentException("FilteredClauseAggregationExecutor only works"
+ + "with CombinedTransformBlock");
+ }
+
+ CombinedTransformBlock combinedTransformBlock = (CombinedTransformBlock) transformBlock;
+ List<TransformBlock> transformBlockList = combinedTransformBlock.getTransformBlockList();
+ int numAggregations = _aggregationFunctions.length;
+ int transformListOffset = 0;
+
+ for (int i = 0; i < numAggregations; i++) {
+ AggregationFunction aggregationFunction = _aggregationFunctions[i];
+
+ if (aggregationFunction.isFilteredAggregation()) {
+ TransformBlock innerTransformBlock = transformBlockList.get(transformListOffset++);
+
+ if (innerTransformBlock != null) {
+ int length = innerTransformBlock.getNumDocs();
+ aggregationFunction.aggregate(length, _aggregationResultHolders[i],
+ AggregationFunctionUtils.getBlockValSetMap(aggregationFunction, innerTransformBlock));
+ }
+ } else {
+ TransformBlock innerTransformBlock = combinedTransformBlock.getNonFilteredAggBlock();
+
+ if (innerTransformBlock != null) {
Review comment:
Not sure when the innerBlock can be null ? If the top call to `aggregate()` for this executor was called by `AggregationOperator`, then there are non-zero records to aggregate (whether filtered or non-filtered). `AggregationOperator` should check for EOF based on whether CombinedTransformBlock is null or not ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] siddharthteotia commented on a change in pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
siddharthteotia commented on a change in pull request #7830:
URL: https://github.com/apache/pinot/pull/7830#discussion_r764475919
##########
File path: pinot-core/src/main/java/org/apache/pinot/core/query/request/context/QueryContext.java
##########
@@ -441,34 +442,40 @@ public QueryContext build() {
*/
private void generateAggregationFunctions(QueryContext queryContext) {
List<AggregationFunction> aggregationFunctions = new ArrayList<>();
- List<Pair<AggregationFunction, FilterContext>> filteredAggregationFunctions = new ArrayList<>();
+ List<FilterContext> filteredAggregationFunctions = new ArrayList<>();
Map<FunctionContext, Integer> aggregationFunctionIndexMap = new HashMap<>();
// Add aggregation functions in the SELECT clause
// NOTE: DO NOT deduplicate the aggregation functions in the SELECT clause because that involves protocol change.
- List<FunctionContext> aggregationsInSelect = new ArrayList<>();
- List<Pair<FunctionContext, FilterContext>> filteredAggregations = new ArrayList<>();
+ List<Pair<Boolean, FunctionContext>> aggregationsInSelect = new ArrayList<>();
+ List<FilterContext> filteredAggregationContexts = new ArrayList<>();
for (ExpressionContext selectExpression : queryContext._selectExpressions) {
- getAggregations(selectExpression, aggregationsInSelect, filteredAggregations);
+ getAggregations(selectExpression, aggregationsInSelect,
+ filteredAggregationContexts);
}
- for (FunctionContext function : aggregationsInSelect) {
+ for (Pair<Boolean, FunctionContext> pair : aggregationsInSelect) {
+ FunctionContext function = pair.getRight();
int functionIndex = aggregationFunctions.size();
AggregationFunction aggregationFunction =
AggregationFunctionFactory.getAggregationFunction(function, queryContext);
+
+
+ aggregationFunction.setFilteredAggregation(pair.getLeft());
+
aggregationFunctions.add(aggregationFunction);
+
aggregationFunctionIndexMap.put(function, functionIndex);
}
- for (Pair<FunctionContext, FilterContext> pair : filteredAggregations) {
- AggregationFunction aggregationFunction =
- aggregationFunctions.get(aggregationFunctionIndexMap.get(pair.getLeft()));
- filteredAggregationFunctions.add(Pair.of(aggregationFunction, pair.getRight()));
+ for (FilterContext filterContext : filteredAggregationContexts) {
Review comment:
Why do we need to create two lists for filtered aggregations ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
[GitHub] [pinot] atris closed pull request #7830: Implement Multiple Predicate Execution (FILTER Clauses)
Posted by GitBox <gi...@apache.org>.
atris closed pull request #7830:
URL: https://github.com/apache/pinot/pull/7830
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org