krishan1390 commented on code in PR #16845:
URL: https://github.com/apache/pinot/pull/16845#discussion_r2368495143
##########
pinot-core/src/test/java/org/apache/pinot/queries/BaseMultiValueRawQueriesTest.java:
##########
@@ -85,7 +85,7 @@ public class BaseMultiValueRawQueriesTest extends
BaseQueriesTest {
protected static final TableConfig TABLE_CONFIG = new
TableConfigBuilder(TableType.OFFLINE)
.setTableName(RAW_TABLE_NAME)
.setTimeColumnName("daysSinceEpoch")
- .setNoDictionaryColumns(List.of("column5", "column6", "column7"))
+ .setNoDictionaryColumns(List.of("column5"))
Review Comment:
column6 and 7 have low cardinality and are good dictionary columns. With the
new change, marking them as no dictionary columns affects the scan order in
ScanBasedANDFilterReorderingTest and fails that test.
##########
pinot-core/src/test/java/org/apache/pinot/queries/InterSegmentAggregationMultiValueRawQueriesTest.java:
##########
@@ -294,7 +294,7 @@ public void testDistinctCountMV() {
{DataSchema.ColumnDataType.INT});
Object[] expectedResults = new Object[]{18499};
ResultTable expectedResultTable = new ResultTable(expectedDataSchema,
Collections.singletonList(expectedResults));
- QueriesTestUtils.testInterSegmentsResult(brokerResponse, 400000L, 0L,
400000L, 400000L, expectedResultTable);
+ QueriesTestUtils.testInterSegmentsResult(brokerResponse, 400000L, 0L, 0L,
400000L, expectedResultTable);
Review Comment:
without the dictionary on column6, those entries aren't scanned post 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
