github-actions[bot] commented on code in PR #65853:
URL: https://github.com/apache/doris/pull/65853#discussion_r3636179062
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/RuntimeFilterPartitionPruneClassifier.java:
##########
@@ -173,19 +173,36 @@ private static boolean isPartitionColumnSlot(SlotRef
slotRef, List<Column> parti
}
private static boolean sameColumn(Column targetColumn, Column
partitionColumn) {
+ targetColumn = getBaseColumn(targetColumn);
+ if (targetColumn == null) {
+ return false;
+ }
if (targetColumn == partitionColumn) {
return true;
}
+ // Metadata reload and deep copy do not preserve Column object
identity.
+ // Prefer stable unique IDs when present; legacy schemas without unique
+ // IDs must fall back to structural equality.
int targetUniqueId = targetColumn.getUniqueId();
int partitionUniqueId = partitionColumn.getUniqueId();
if (targetUniqueId != Column.COLUMN_UNIQUE_ID_INIT_VALUE
- && partitionUniqueId != Column.COLUMN_UNIQUE_ID_INIT_VALUE
- && targetUniqueId == partitionUniqueId) {
- return true;
+ && partitionUniqueId != Column.COLUMN_UNIQUE_ID_INIT_VALUE) {
+ return targetUniqueId == partitionUniqueId;
}
return targetColumn.equals(partitionColumn);
}
+ private static Column getBaseColumn(Column targetColumn) {
+ if (!targetColumn.isMaterializedViewColumn()) {
+ return targetColumn;
+ }
+ Expr defineExpr = targetColumn.getDefineExpr();
+ if (defineExpr instanceof SlotRef && ((SlotRef)
defineExpr).getColumn() != null) {
+ return ((SlotRef) defineExpr).getColumn();
Review Comment:
**[P1] Reject aggregate MV columns before treating `defineExpr` as alias
lineage**
A simple `SlotRef` definition is not necessarily value-preserving. For
`SUM(BIGINT k) AS sum_k`, MV construction records aggregation type `SUM` but
leaves `defineExpr` as the input `SlotRef(k)` because no cast is needed. A
direct `FROM fact INDEX mv_sum` scan can then put an RF on the stored `sum_k`
slot; this helper unwraps it to raw `k`, and `OlapScanNode` serializes raw `k`
partition ranges under the `sum_k` slot ID.
For example, a `k=[0,10)` partition containing `k=6,7` for one group stores
`sum_k=13`. An RF `{13}` has no intersection with `[0,10)`, so BE drops the
partition even though it contains a matching aggregate row. Please only unwrap
non-aggregating, value-preserving aliases, keep the classifier and serializer
on the same rule, and add a direct aggregate-MV regression whose result lies
outside the raw source range.
##########
fe/fe-core/src/main/java/org/apache/doris/planner/OlapScanNode.java:
##########
@@ -1408,22 +1413,64 @@ private void setPartitionBoundaries(TOlapScanNode
olapScanNode) {
if (item == null) {
continue;
}
- if (item instanceof RangePartitionItem) {
- addRangeBoundaries(boundaries, partitionId,
(RangePartitionItem) item,
- partColumns, partColToSlotId);
- } else if (item instanceof ListPartitionItem) {
- addListBoundaries(boundaries, partitionId, (ListPartitionItem)
item,
- partColumns, partColToSlotId);
+ for (Map.Entry<Integer, Integer> entry :
pruningSlotToPartitionColumnIndex.entrySet()) {
+ int slotId = entry.getKey();
+ int partitionColumnIndex = entry.getValue();
+ if (item instanceof RangePartitionItem) {
+ Preconditions.checkState(partitionColumnIndex == 0);
+ addRangeBoundary(boundaries, partitionId,
(RangePartitionItem) item,
+ partColumns.size(), slotId);
+ } else if (item instanceof ListPartitionItem) {
+ addListBoundary(boundaries, partitionId,
(ListPartitionItem) item,
+ partitionColumnIndex, slotId);
+ }
}
}
if (!boundaries.isEmpty()) {
olapScanNode.setPartitionBoundaries(boundaries);
}
}
- private void addRangeBoundaries(List<TPartitionBoundary> boundaries, long
partitionId,
- RangePartitionItem rangeItem, List<Column> partColumns,
- Map<String, Integer> partColToSlotId) {
+ private int findPartitionColumnIndex(Column targetColumn, List<Column>
partitionColumns) {
+ targetColumn = getBaseColumn(targetColumn);
+ if (targetColumn == null) {
+ return -1;
+ }
+ for (int i = 0; i < partitionColumns.size(); i++) {
+ Column partitionColumn = partitionColumns.get(i);
+ if (targetColumn == partitionColumn) {
+ return i;
+ }
+ // Metadata reload and deep copy do not preserve Column object
identity.
+ // Prefer stable unique IDs when present; legacy schemas without
unique
+ // IDs must fall back to structural equality.
+ int targetUniqueId = targetColumn.getUniqueId();
Review Comment:
**[P1] Do not match rollup-local IDs to base partition columns**
`uniqueId` is only unique within one index. With light schema change,
ordinary `ADD ROLLUP` schemas are renumbered from 0 and their copied base
columns have no `defineExpr`. For a base DUP table `(id uid0, k uid1, v uid2)
DUPLICATE KEY(id,k)` partitioned by `k`, the valid rollup `(id uid0, v uid1)`
makes an RF on `v` pass this lookup as if it targeted `k`. FE then serializes
`k`'s ranges with `v`'s slot ID, so BE can prune `k` partitions using RF values
from `v` and return wrong rows. The old name-based serializer emitted no `k`
boundary when this rollup omitted `k`.
Please use index-aware base-column lineage rather than comparing raw IDs
across the selected rollup and base partition schema, keep the classifier and
serializer on the same resolution rule, and add this ordinary-rollup collision
as a regression.
--
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]
