github-actions[bot] commented on code in PR #66187:
URL: https://github.com/apache/doris/pull/66187#discussion_r3701948554
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java:
##########
@@ -1478,6 +1478,67 @@ public Partition dropPartitionForTruncate(long dbId,
boolean isForceDrop,
*
*/
+ @Override
+ public Partition getMaxVisiblePartition() {
+ PartitionInfo partitionInfo = getPartitionInfo();
+ PartitionType type = partitionInfo.getType();
+ // Cloud mode: prefetch all visible versions in one batch RPC so the
loop below hits
+ // cache instead of firing N sequential meta-service RPCs under
metadata locks.
+ if (Config.isCloudMode()) {
+ try {
+ getVersionInBatchForCloudMode(nameToPartition.values().stream()
+ .map(Partition::getId).collect(Collectors.toList()));
+ } catch (RpcException e) {
+ LOG.warn("batch prefetch visible version failed, fallback to
per-partition lookup", e);
+ }
+ }
+ if (type == PartitionType.UNPARTITIONED) {
+ for (Partition partition : nameToPartition.values()) {
+ if (partition.getVisibleVersion() >
Partition.PARTITION_INIT_VERSION) {
+ return partition;
+ }
+ }
+ return null;
+ }
+ // RANGE/LIST: pick the visible partition with the greatest key (RANGE
by upper bound,
+ // LIST by max discrete key). A LIST default partition is only a
last-resort fallback.
+ Partition result = null;
+ PartitionKey maxKey = null;
+ Partition defaultFallback = null;
+ for (Partition partition : nameToPartition.values()) {
+ if (partition.getVisibleVersion() <=
Partition.PARTITION_INIT_VERSION) {
Review Comment:
[P1] Do not treat a published delete version as visible rows
`visibleVersion > PARTITION_INIT_VERSION` only proves that a version was
published, not that this partition still contains any visible row. The new Case
8b is a concrete counterexample: p1 retains `(1,10)`, p2's sole `(7,70)` is
deleted, yet this condition selects p2; `PruneEmptyPartition` uses the same
version heuristic, so the plan remains:
```text
ResultSink
LogicalOlapScan(mvp_td2, selectedPartitionIds=[p2])
```
The checked-in result is therefore empty. That contradicts the advertised
greatest partition "that has visible data" / `max(partition_key)` behavior,
which should fall back to p1. Please base selection on an authoritative
current-row signal (or constrain/redefine the feature if FE cannot know it),
make this regression require p1's row, and cover a zero-row publish 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: [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]
