Skip to content

Commit

Permalink
[fix](Nereids) eliminate union should execute before limit push down
Browse files Browse the repository at this point in the history
other wise:

push limit through union could generate plan:

```
limit
+-- union
    |-- limit
    |   +-- empty relation
    +-- limit
        +-- project
```

and then eliminate union will generate plan:

```
+-- limit
    +-  project
        +-- limit
            +-- project
```

it could not be processed by tranlator correctly
  • Loading branch information
morrySnow committed Aug 20, 2024
1 parent 3608d2c commit a1e599b
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -364,32 +364,6 @@ public class Rewriter extends AbstractBatchJobExecutor {
topDown(new EliminateGroupByKey()),
topDown(new PushDownAggThroughJoinOnPkFk())
),

topic("Limit optimization",
// TODO: the logical plan should not contains any phase information,
// we should refactor like AggregateStrategies, e.g. LimitStrategies,
// generate one PhysicalLimit if current distribution is gather or two
// PhysicalLimits with gather exchange
topDown(new LimitSortToTopN()),
topDown(new LimitAggToTopNAgg()),
topDown(new MergeTopNs()),
topDown(new SplitLimit()),
topDown(
new PushDownLimit(),
new PushDownLimitDistinctThroughJoin(),
new PushDownLimitDistinctThroughUnion(),
new PushDownTopNDistinctThroughJoin(),
new PushDownTopNDistinctThroughUnion(),
new PushDownTopNThroughJoin(),
new PushDownTopNThroughWindow(),
new PushDownTopNThroughUnion()
),
topDown(new CreatePartitionTopNFromWindow()),
topDown(
new PullUpProjectUnderTopN(),
new PullUpProjectUnderLimit()
)
),
// TODO: these rules should be implementation rules, and generate alternative physical plans.
topic("Table/Physical optimization",
topDown(
Expand Down Expand Up @@ -425,9 +399,55 @@ public class Rewriter extends AbstractBatchJobExecutor {
bottomUp(new EliminateEmptyRelation())
),
topic("agg rewrite",
// these rules should be put after mv optimization to avoid mv matching fail
topDown(new SumLiteralRewrite(),
new MergePercentileToArray())
// these rules should be put after mv optimization to avoid mv matching fail
topDown(new SumLiteralRewrite(),
new MergePercentileToArray())
),
// limit push down through union should execute after eliminate union by empty relation.
// otherwise will generate illegal plan
// think about
// limit
// +-- union
// |-- empty relation
// +-- project 1
//
// after PUSH_LIMIT_THROUGH_UNION will generate
// limit
// +-- union
// |-- limit
// | +-- empty relation
// +-- limit
// +-- project 1
// after ELIMINATE_UNION_BY_EMPTY_RELATION will generate
// +-- limit
// +- project 2
// +-- limit
// +-- project 1
// the last plan is illegal because translator will lose info in project 1
topic("Limit optimization",
// TODO: the logical plan should not contains any phase information,
// we should refactor like AggregateStrategies, e.g. LimitStrategies,
// generate one PhysicalLimit if current distribution is gather or two
// PhysicalLimits with gather exchange
topDown(new LimitSortToTopN()),
topDown(new LimitAggToTopNAgg()),
topDown(new MergeTopNs()),
topDown(new SplitLimit()),
topDown(
new PushDownLimit(),
new PushDownLimitDistinctThroughJoin(),
new PushDownLimitDistinctThroughUnion(),
new PushDownTopNDistinctThroughJoin(),
new PushDownTopNDistinctThroughUnion(),
new PushDownTopNThroughJoin(),
new PushDownTopNThroughWindow(),
new PushDownTopNThroughUnion()
),
topDown(new CreatePartitionTopNFromWindow()),
topDown(
new PullUpProjectUnderTopN(),
new PullUpProjectUnderLimit()
)
),
topic("Push project and filter on cte consumer to cte producer",
topDown(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// 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.

suite("push_limit_with_eliminate_union") {
// this should not failed for [INTERNAL_ERROR]VSlotRef have invalid slot id
sql """(select 1 limit 0 union all select count() + 1) limit 1;"""
}

0 comments on commit a1e599b

Please sign in to comment.