Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@
import org.apache.doris.nereids.rules.rewrite.ReduceAggregateChildOutputRows;
import org.apache.doris.nereids.rules.rewrite.ReorderJoin;
import org.apache.doris.nereids.rules.rewrite.RewriteCteChildren;
import org.apache.doris.nereids.rules.rewrite.RewritePartitionColumnMinMaxToConstantRule;
import org.apache.doris.nereids.rules.rewrite.RewriteSearchToSlots;
import org.apache.doris.nereids.rules.rewrite.RewriteSimpleAggToConstantRule;
import org.apache.doris.nereids.rules.rewrite.SaltJoin;
Expand Down Expand Up @@ -289,6 +290,7 @@ public class Rewriter extends AbstractBatchJobExecutor {
new NormalizeAggregate(),
new CountLiteralRewrite(),
new RewriteSimpleAggToConstantRule(),
new RewritePartitionColumnMinMaxToConstantRule(),
new NormalizeSort()
),

Expand Down Expand Up @@ -530,6 +532,7 @@ public class Rewriter extends AbstractBatchJobExecutor {
new NormalizeAggregate(),
new CountLiteralRewrite(),
new RewriteSimpleAggToConstantRule(),
new RewritePartitionColumnMinMaxToConstantRule(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Revisit this rule after project merging. For SELECT max(q) FROM (SELECT p AS q FROM ext) v, NormalizeAggregate first adds its own bottom project, so the operative whole-plan job here sees Aggregate -> Project -> Project -> FileScan; the factory matches at most one project. MergeProjectable runs only in the following traversal, which never reruns this rule, so the intended alias case still scans files. Move/rerun the fold after project merging in both pipelines, or handle the normalized project chain directly.

new NormalizeSort()
),

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ public enum RuleType {
COUNT_LITERAL_REWRITE(RuleTypeClass.REWRITE),
SUM_LITERAL_REWRITE(RuleTypeClass.REWRITE),
REWRITE_SIMPLE_AGG_TO_CONSTANT(RuleTypeClass.REWRITE),
REWRITE_PARTITION_COLUMN_MIN_MAX_TO_CONSTANT(RuleTypeClass.REWRITE),
REPLACE_SORT_EXPRESSION_BY_CHILD_OUTPUT(RuleTypeClass.REWRITE),

FILL_UP_HAVING_AGGREGATE(RuleTypeClass.REWRITE),
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
// Licensed to the Apache Software Foundation (ASF) under one
// or more contributor license agreements. See the NOTICE file
// distributed with this work for additional information
// regarding copyright ownership. The ASF licenses this file
// to you under the Apache License, Version 2.0 (the
// "License"); you may not use this file except in compliance
// with the License. You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing,
// software distributed under the License is distributed on an
// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.

package org.apache.doris.nereids.rules.rewrite;

import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.ListPartitionItem;
import org.apache.doris.catalog.PartitionItem;
import org.apache.doris.catalog.PartitionKey;
import org.apache.doris.catalog.Type;
import org.apache.doris.datasource.ExternalTable;
import org.apache.doris.nereids.StatementContext;
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.trees.expressions.Alias;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.NamedExpression;
import org.apache.doris.nereids.trees.expressions.Slot;
import org.apache.doris.nereids.trees.expressions.SlotReference;
import org.apache.doris.nereids.trees.expressions.functions.agg.AggregateFunction;
import org.apache.doris.nereids.trees.expressions.functions.agg.Max;
import org.apache.doris.nereids.trees.expressions.functions.agg.Min;
import org.apache.doris.nereids.trees.expressions.literal.Literal;
import org.apache.doris.nereids.trees.expressions.literal.NullLiteral;
import org.apache.doris.nereids.trees.plans.Plan;
import org.apache.doris.nereids.trees.plans.algebra.Project;
import org.apache.doris.nereids.trees.plans.logical.LogicalAggregate;
import org.apache.doris.nereids.trees.plans.logical.LogicalFileScan;
import org.apache.doris.nereids.trees.plans.logical.LogicalOneRowRelation;
import org.apache.doris.nereids.trees.plans.logical.LogicalProject;
import org.apache.doris.nereids.types.DataType;

import com.google.common.collect.ImmutableList;

import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

/**
* Rewrite MIN/MAX on a single external list partition column to constants from partition metadata.
*
* <p>For queries like {@code dt = (select max(dt) from hive_table)}, evaluating MAX(dt) by scanning
* every partition blocks partition pruning for the outer scan. The selected partition map already
* contains the exact list partition values, so this rule replaces the scalar aggregate with a
* one-row constant relation before file-scan partition pruning runs.
*/
public class RewritePartitionColumnMinMaxToConstantRule implements RewriteRuleFactory {

@Override

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Add tests before enabling this semantic rewrite. The PR changes only production files, and there is no existing FE or regression reference to this rule or RuleType. Please cover direct and scalar-subquery plans, the normalized alias shape, rule disabling, all applicability gates, null/default/empty partitions, connector transforms and scan selectors, and parameterized output types; assert both result value/type and whether the file scan is removed.

public List<Rule> buildRules() {
return ImmutableList.of(
logicalAggregate(logicalFileScan())
.thenApply(ctx -> {
LogicalAggregate<LogicalFileScan> agg = ctx.root;
LogicalFileScan scan = agg.child();
return tryRewrite(agg, scan, Optional.empty(), ctx.statementContext);
})
.toRule(RuleType.REWRITE_PARTITION_COLUMN_MIN_MAX_TO_CONSTANT),
logicalAggregate(logicalProject(logicalFileScan()))
.thenApply(ctx -> {
LogicalAggregate<LogicalProject<LogicalFileScan>> agg = ctx.root;
LogicalProject<LogicalFileScan> project = agg.child();
LogicalFileScan scan = project.child();
return tryRewrite(agg, scan, Optional.of(project), ctx.statementContext);
})
.toRule(RuleType.REWRITE_PARTITION_COLUMN_MIN_MAX_TO_CONSTANT)
);
}

private Plan tryRewrite(LogicalAggregate<?> agg, LogicalFileScan scan,
Optional<LogicalProject<LogicalFileScan>> project, StatementContext statementContext) {
if (scan.getTableSample().isPresent() || !agg.getGroupByExpressions().isEmpty()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Skip this fold for incremental scan params. The Paimon @incr path intentionally freezes the whole latest partition map here, then applies incremental-between* only when planning the connector scan. If the requested window contains only p=2024 while latest also has p=2025, this rule rewrites the window's MAX(p) to 2025. Reject row-set-altering scan params, or derive extrema from metadata scoped to the exact incremental relation.

return null;
}

ExternalTable table = scan.getTable();
if (!table.supportInternalPartitionPruned()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Require an exact identity-partition contract here. supportInternalPartitionPruned() is true for every plugin-driven table, including Iceberg specs such as truncate(10, id) and bucket(2, id). Iceberg publishes the source name id, but its partition map stores the transformed value. Thus Aggregate(max(id)) -> FileScan(selected={10,20}) for rows 19 and 22 is rewritten to constant 20 instead of 22. Please gate this on connector metadata proving identity values (or restrict it to a connector with that guarantee), rather than generic pruning support.

return null;
}

List<Column> partitionColumns = table.getPartitionColumns(
statementContext.getSnapshot(table, scan.getTableSnapshot(), scan.getScanParams()));
if (partitionColumns.size() != 1) {
return null;
}
Column partitionColumn = partitionColumns.get(0);

Set<AggregateFunction> funcs = agg.getAggregateFunctions();
if (funcs.isEmpty()) {
return null;
}
for (AggregateFunction func : funcs) {
if (!(func instanceof Min) && !(func instanceof Max)) {
return null;
}
}

List<NamedExpression> newOutputExprs = new ArrayList<>();
for (NamedExpression outputExpr : agg.getOutputExpressions()) {
if (!(outputExpr instanceof Alias)) {
return null;
}
Alias alias = (Alias) outputExpr;
Expression child = alias.child();
if (!(child instanceof AggregateFunction)) {
return null;
}
Optional<Literal> constant = tryGetConstant(
(AggregateFunction) child, partitionColumn, scan, project);
if (!constant.isPresent()) {
return null;
}
newOutputExprs.add(new Alias(alias.getExprId(), constant.get(), alias.getName()));
}

if (newOutputExprs.isEmpty()) {
return null;
}

LogicalOneRowRelation oneRowRelation = new LogicalOneRowRelation(
statementContext.getNextRelationId(),
ImmutableList.of(new Alias(new NullLiteral(), "__dummy__")));
return new LogicalProject<>(newOutputExprs, oneRowRelation);
}

private Optional<Literal> tryGetConstant(AggregateFunction func, Column partitionColumn, LogicalFileScan scan,
Optional<LogicalProject<LogicalFileScan>> project) {
if (func.isDistinct() || func.getArguments().size() != 1) {
return Optional.empty();
}
Optional<SlotReference> slot = resolveSlot(func.getArguments().get(0), project);
if (!slot.isPresent() || !isPartitionColumn(slot.get(), partitionColumn)) {
return Optional.empty();
}

return findPartitionMinMaxLiteral(func instanceof Min, scan, partitionColumn);
}

private Optional<SlotReference> resolveSlot(Expression expression,
Optional<LogicalProject<LogicalFileScan>> project) {
Expression resolved = expression;
if (project.isPresent() && expression instanceof Slot) {
Map<Slot, Expression> aliasToProducer = ((Project) project.get()).getAliasToProducer();
resolved = aliasToProducer.getOrDefault(expression, expression);
}
if (resolved instanceof SlotReference) {
return Optional.of((SlotReference) resolved);
}
return Optional.empty();
}

private boolean isPartitionColumn(SlotReference slot, Column partitionColumn) {
Optional<Column> originalColumn = slot.getOriginalColumn();
if (originalColumn.isPresent()) {
return originalColumn.get().getName().equalsIgnoreCase(partitionColumn.getName());
}
return slot.getName().equalsIgnoreCase(partitionColumn.getName());
}

private Optional<Literal> findPartitionMinMaxLiteral(boolean isMin, LogicalFileScan scan, Column partitionColumn) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Do not derive query results from a partition generation that the scan does not use. MaxCompute serves this map from a cross-query cache with a default 600-second TTL, but initSelectedPartitions marks it unpruned; the scan path consequently passes no required partitions and ODPS scans the current source set. If cached p=1,p=9 loses p=9 remotely, this fold returns 9 while the scan returns 1 (and a remote add misses a new extremum). Require metadata pinned to the scan's exact generation/read set, or decline this optimization for cached scan-all listings.

PartitionKey selectedKey = null;
for (PartitionItem item : scan.getSelectedPartitions().selectedPartitions.values()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Partition existence is not row-existence evidence. Hive lists every HMS partition name while its ConnectorPartitionInfo row/file counts are explicitly unknown, and an ALTER TABLE ... ADD PARTITION (p=999) may leave an empty directory. With the only row in p=1, SQL MAX(p) is 1 but this loop returns 999; if 999 is the only empty partition, SQL returns NULL while the rewrite still returns 999. Please decline the fold unless the connector supplies a snapshot-consistent guarantee that every considered identity key contributes a visible row.

if (item.isDefaultPartition() || !(item instanceof ListPartitionItem)) {
return Optional.empty();
}
for (PartitionKey key : ((ListPartitionItem) item).getItems()) {
if (key.isDefaultListPartitionKey()) {
return Optional.empty();
}
org.apache.doris.analysis.LiteralExpr literalExpr = key.getKeys().get(0);
if (literalExpr instanceof org.apache.doris.analysis.NullLiteral) {
continue;
}
if (selectedKey == null || (isMin ? key.compareTo(selectedKey) < 0 : key.compareTo(selectedKey) > 0)) {
selectedKey = key;
}
}
}

if (selectedKey == null) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Do not treat an unenumerated or unresolved partition universe as an empty relation. Explicit FOR VERSION AS OF, @tag, @branch, and snapshot-selecting options intentionally keep partition columns with an empty map because enumeration is deferred; an absent connector handle also returns {} here while scan creation would throw Table '...' does not exist. Both reach selectedKey == null, either corrupting a nonempty historical result or suppressing the expected error. Require an explicitly complete map for the exact read and preserve unresolved-handle failure.

return Optional.of(new NullLiteral(DataType.fromCatalogType(partitionColumn.getType())));
}
Type literalType = Type.fromPrimitiveType(selectedKey.getTypes().get(0));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Preserve the normalized aggregate return type instead of rebuilding it from PrimitiveType. PartitionKey has already discarded scale/precision/length: DATETIMEV2(6) is rebuilt as scale 0 and rounded, DECIMAL(10,2) is rebuilt with scale 0 and can fail literal validation, CHAR/VARCHAR lose their declared length, and DECIMAL256 has no mapping here. This can change MAX(p)'s value or schema, or fail planning. Construct and validate both non-null and typed-NULL replacements against func.getDataType(); MIN/MAX.customSignature() can normalize DecimalV2 to DecimalV3, so the partition-column type alone is not always the result type.

return Optional.of(Literal.fromLegacyLiteral(selectedKey.getKeys().get(0), literalType));
}
}
Loading