github-actions[bot] commented on code in PR #65659:
URL: https://github.com/apache/doris/pull/65659#discussion_r3588173223
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java:
##########
@@ -95,15 +95,18 @@ private SelectedPartitions
pruneExternalPartitions(ExternalTable externalTable,
boolean enableBinarySearch = ctx.getConnectContext() == null
||
ctx.getConnectContext().getSessionVariable().enableBinarySearchFilteringPartitions;
if (enableBinarySearch) {
- sortedPartitionRanges = (Optional)
externalTable.getSortedPartitionRanges(scan);
+ sortedPartitionRanges =
scan.getSelectedPartitions().sortedPartitionRanges;
}
PartitionPruneResult<String> result = PartitionPruner.pruneWithResult(
partitionSlots, filter.getPredicate(), nameToPartitionItem,
ctx,
PartitionTableType.EXTERNAL, sortedPartitionRanges);
List<String> prunedPartitions = new ArrayList<>(result.partitions);
for (String name : prunedPartitions) {
- selectedPartitionItems.put(name, nameToPartitionItem.get(name));
+ PartitionItem item = nameToPartitionItem.get(name);
+ if (item != null) {
Review Comment:
At this point a missing item is an invariant failure, not a partition to
skip. The sequential path returns keys from `nameToPartitionItem`, and the
binary-search path should return keys from
`scan.getSelectedPartitions().sortedPartitionRanges`, which this PR now builds
from the same selected-partition snapshot. If `get(name)` is ever null again,
silently omitting it produces a partial or even empty selected partition map
for a positive predicate; the new test's p4 scenario demonstrates exactly that
behavior. Please fail/assert here (or fall back to sequential pruning before
constructing `SelectedPartitions`) instead of returning a scan over fewer
partitions.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/PruneFileScanPartition.java:
##########
@@ -95,15 +95,18 @@ private SelectedPartitions
pruneExternalPartitions(ExternalTable externalTable,
boolean enableBinarySearch = ctx.getConnectContext() == null
||
ctx.getConnectContext().getSessionVariable().enableBinarySearchFilteringPartitions;
if (enableBinarySearch) {
- sortedPartitionRanges = (Optional)
externalTable.getSortedPartitionRanges(scan);
+ sortedPartitionRanges =
scan.getSelectedPartitions().sortedPartitionRanges;
}
PartitionPruneResult<String> result = PartitionPruner.pruneWithResult(
partitionSlots, filter.getPredicate(), nameToPartitionItem,
ctx,
PartitionTableType.EXTERNAL, sortedPartitionRanges);
List<String> prunedPartitions = new ArrayList<>(result.partitions);
for (String name : prunedPartitions) {
- selectedPartitionItems.put(name, nameToPartitionItem.get(name));
+ PartitionItem item = nameToPartitionItem.get(name);
Review Comment:
At this point a missing item is an invariant failure, not a partition to
skip. The sequential path returns keys from `nameToPartitionItem`, and the
binary-search path should return keys from
`scan.getSelectedPartitions().sortedPartitionRanges`, which this PR now builds
from the same selected-partition snapshot. If `get(name)` is ever null again,
silently omitting it produces a partial or even empty selected partition map
for a positive predicate; the new test's p4 scenario demonstrates exactly that
behavior. Please fail/assert here (or fall back to sequential pruning before
constructing `SelectedPartitions`) instead of returning a scan over fewer
partitions.
--
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]
