github-actions[bot] commented on code in PR #65445:
URL: https://github.com/apache/doris/pull/65445#discussion_r3556957300
##########
fe/fe-core/src/test/java/org/apache/doris/nereids/stats/StatsCalculatorTest.java:
##########
@@ -206,6 +212,75 @@ table1, new Column("c1", PrimitiveType.INT),
Assertions.assertNotNull(stats.columnStatistics().get(slot1));
}
+ @Test
+ public void testComputeOlapScanScalesNumNullsForSelectedPartitions() {
+ ConnectContext previousContext = ConnectContext.get();
+ ConnectContext connectContext = new ConnectContext();
+ connectContext.setThreadLocalInfo();
+
+ Env env = Mockito.mock(Env.class);
+ AnalysisManager analysisManager = Mockito.mock(AnalysisManager.class);
+ StatisticsCache statisticsCache = Mockito.mock(StatisticsCache.class);
+ StatisticsCache.OlapTableStatistics olapTableStatistics
+ = Mockito.mock(StatisticsCache.OlapTableStatistics.class);
+ OlapTable table = Mockito.mock(OlapTable.class);
+ LogicalOlapScan scan = Mockito.mock(LogicalOlapScan.class);
+ Partition selectedPartition = Mockito.mock(Partition.class);
+
+ long selectedPartitionId = 1L;
+ long baseIndexId = 10L;
+ Column column = new Column("val", PrimitiveType.INT);
+ SlotReference slot = new SlotReference(new ExprId(1), "val",
IntegerType.INSTANCE, true,
+ ImmutableList.of("test", "tbl"), table, column, table, column);
+ ColumnStatistic tableStatistic = new ColumnStatisticBuilder(12)
+ .setNdv(1)
+ .setMinValue(2)
+ .setMaxValue(2)
+ .setMinExpr(new IntLiteral(2))
+ .setMaxExpr(new IntLiteral(2))
+ .setNumNulls(3)
+ .build();
+
+ Mockito.when(env.getAnalysisManager()).thenReturn(analysisManager);
+ Mockito.when(env.getStatisticsCache()).thenReturn(statisticsCache);
+
Mockito.when(statisticsCache.getOlapTableStats(scan)).thenReturn(olapTableStatistics);
+ Mockito.when(olapTableStatistics.getColumnStatistics("val",
connectContext)).thenReturn(tableStatistic);
+ Mockito.when(scan.getTable()).thenReturn(table);
+ Mockito.when(scan.getSelectedIndexId()).thenReturn(baseIndexId);
+
Mockito.when(scan.getSelectedPartitionIds()).thenReturn(ImmutableList.of(selectedPartitionId));
+ Mockito.when(scan.getOutput()).thenReturn(ImmutableList.of(slot));
Review Comment:
The new unit test is deterministic, but it no longer covers the same path as
the deleted regression. Here we pre-stub `getSelectedPartitionIds()` to the
single selected partition and then call `computeOlapScan` directly, so a
regression that stops `where d = '2017-01-01'` from producing a pruned
`LogicalOlapScan` would still pass this test. The removed Groovy case exercised
predicate planning, partition pruning, and memo stats together through `explain
memo plan`. Please keep this unit test, but add a non-flaky planner-level check
that builds/plans the partition-filtered scan and verifies the
selected-partition stats/null-count result through that path, without depending
on asynchronous BE row-count reporting.
##########
fe/fe-core/src/test/java/org/apache/doris/nereids/stats/StatsCalculatorTest.java:
##########
@@ -206,6 +212,75 @@ table1, new Column("c1", PrimitiveType.INT),
Assertions.assertNotNull(stats.columnStatistics().get(slot1));
}
+ @Test
+ public void testComputeOlapScanScalesNumNullsForSelectedPartitions() {
+ ConnectContext previousContext = ConnectContext.get();
+ ConnectContext connectContext = new ConnectContext();
+ connectContext.setThreadLocalInfo();
+
+ Env env = Mockito.mock(Env.class);
+ AnalysisManager analysisManager = Mockito.mock(AnalysisManager.class);
+ StatisticsCache statisticsCache = Mockito.mock(StatisticsCache.class);
+ StatisticsCache.OlapTableStatistics olapTableStatistics
+ = Mockito.mock(StatisticsCache.OlapTableStatistics.class);
+ OlapTable table = Mockito.mock(OlapTable.class);
+ LogicalOlapScan scan = Mockito.mock(LogicalOlapScan.class);
+ Partition selectedPartition = Mockito.mock(Partition.class);
+
+ long selectedPartitionId = 1L;
+ long baseIndexId = 10L;
+ Column column = new Column("val", PrimitiveType.INT);
+ SlotReference slot = new SlotReference(new ExprId(1), "val",
IntegerType.INSTANCE, true,
+ ImmutableList.of("test", "tbl"), table, column, table, column);
+ ColumnStatistic tableStatistic = new ColumnStatisticBuilder(12)
+ .setNdv(1)
+ .setMinValue(2)
+ .setMaxValue(2)
+ .setMinExpr(new IntLiteral(2))
+ .setMaxExpr(new IntLiteral(2))
+ .setNumNulls(3)
+ .build();
+
+ Mockito.when(env.getAnalysisManager()).thenReturn(analysisManager);
+ Mockito.when(env.getStatisticsCache()).thenReturn(statisticsCache);
+
Mockito.when(statisticsCache.getOlapTableStats(scan)).thenReturn(olapTableStatistics);
+ Mockito.when(olapTableStatistics.getColumnStatistics("val",
connectContext)).thenReturn(tableStatistic);
+ Mockito.when(scan.getTable()).thenReturn(table);
+ Mockito.when(scan.getSelectedIndexId()).thenReturn(baseIndexId);
+
Mockito.when(scan.getSelectedPartitionIds()).thenReturn(ImmutableList.of(selectedPartitionId));
Review Comment:
The new unit test is deterministic, but it no longer covers the same path as
the deleted regression. Here we pre-stub `getSelectedPartitionIds()` to the
single selected partition and then call `computeOlapScan` directly, so a
regression that stops `where d = '2017-01-01'` from producing a pruned
`LogicalOlapScan` would still pass this test. The removed Groovy case exercised
predicate planning, partition pruning, and memo stats together through `explain
memo plan`. Please keep this unit test, but add a non-flaky planner-level check
that builds/plans the partition-filtered scan and verifies the
selected-partition stats/null-count result through that path, without depending
on asynchronous BE row-count reporting.
--
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]
