github-actions[bot] commented on code in PR #65472:
URL: https://github.com/apache/doris/pull/65472#discussion_r3642738709
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/logical/LogicalOlapScan.java:
##########
@@ -1207,6 +1207,22 @@ public LogicalOlapScan
withTableScanParams(TableScanParams scanParams) {
Optional.of(scanParams)));
}
+ @Override
+ protected boolean hasSameScanState(LogicalCatalogRelation other) {
+ if (!Utils.isSameClass(this, other)) {
+ return false;
+ }
+ LogicalOlapScan that = (LogicalOlapScan) other;
+ return selectedIndexId == that.selectedIndexId
+ && indexSelected == that.indexSelected
+ && Objects.equals(selectedPartitionIds,
that.selectedPartitionIds)
+ && Objects.equals(manuallySpecifiedPartitions,
that.manuallySpecifiedPartitions)
+ && Objects.equals(selectedTabletIds, that.selectedTabletIds)
+ && Objects.equals(manuallySpecifiedTabletIds,
that.manuallySpecifiedTabletIds)
+ && Objects.equals(tableSample, that.tableSample)
Review Comment:
Two identical `TABLESAMPLE` clauses without `REPEATABLE` are not
semantically the same scan occurrence. The parser gives both `seek == -1`, so
this equality check accepts the following reduced tree:
```text
UnionAll
Project(...) -> Join(Scan(t TABLESAMPLE 10 PERCENT), o1)
Project(...) -> Join(Scan(t TABLESAMPLE 10 PERCENT), o2)
```
Without the rewrite, the two physical `OlapScanNode`s independently choose
fresh `SecureRandom` partition/tablet seeks when `seek == -1`
(`computeSampleTabletIds()`); after `constructNewJoin()` keeps the first common
scan, both arms share one sample and can return different rows. The current
test only covers *different* sample values (and uses a repeatable seek), so it
misses this case. Please reject common-side matching whenever a present sample
has `seek == -1` (or assign occurrence identity), and add a negative rewrite
test for equal unseeded samples.
--
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]
