Skip to content

Commit

Permalink
[Fix](Nereids)Check bounding of expression in analyze process after b…
Browse files Browse the repository at this point in the history
…ounding and before apply more complex analyze rules
  • Loading branch information
LiBinfeng-01 committed Apr 19, 2023
1 parent fb377a9 commit b1a002a
Show file tree
Hide file tree
Showing 6 changed files with 148 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.apache.doris.nereids.rules.analysis.BindExpression;
import org.apache.doris.nereids.rules.analysis.BindRelation;
import org.apache.doris.nereids.rules.analysis.CheckAnalysis;
import org.apache.doris.nereids.rules.analysis.CheckBound;
import org.apache.doris.nereids.rules.analysis.CheckPolicy;
import org.apache.doris.nereids.rules.analysis.FillUpMissingSlots;
import org.apache.doris.nereids.rules.analysis.NormalizeRepeat;
Expand Down Expand Up @@ -53,6 +54,9 @@ public class NereidsAnalyzer extends BatchRewriteJob {
new UserAuthentication(),
new BindExpression()
),
bottomUp(
new CheckBound()
),
bottomUp(
new ProjectToGlobalAggregate(),
// this rule check's the logicalProject node's isDisinct property
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ public enum RuleType {
// check analysis rule
CHECK_AGGREGATE_ANALYSIS(RuleTypeClass.CHECK),
CHECK_ANALYSIS(RuleTypeClass.CHECK),
CHECK_BOUND(RuleTypeClass.CHECK),
CHECK_DATATYPES(RuleTypeClass.CHECK),

// rewrite rules
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,17 +78,19 @@ private void checkBound(Plan plan) {
.flatMap(Set::stream)
.collect(Collectors.toSet());
if (!unbounds.isEmpty()) {
throw new AnalysisException(String.format("unbounded object %s.",
StringUtils.join(unbounds.stream()
.map(unbound -> {
if (unbound instanceof UnboundSlot) {
return ((UnboundSlot) unbound).toSql();
} else if (unbound instanceof UnboundFunction) {
return ((UnboundFunction) unbound).toSql();
}
return unbound.toString();
})
.collect(Collectors.toSet()), ", ")));
throw new AnalysisException(String.format("unbounded object %s in %s clause.",
StringUtils.join(unbounds.stream()
.map(unbound -> {
if (unbound instanceof UnboundSlot) {
return ((UnboundSlot) unbound).toSql();
} else if (unbound instanceof UnboundFunction) {
return ((UnboundFunction) unbound).toSql();
}
return unbound.toString();
})
.collect(Collectors.toSet()), ", "),
plan.getType().toString().substring("LOGICAL_".length())
));
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// 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.analysis;

import org.apache.doris.nereids.analyzer.Unbound;
import org.apache.doris.nereids.analyzer.UnboundFunction;
import org.apache.doris.nereids.analyzer.UnboundSlot;
import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.rules.Rule;
import org.apache.doris.nereids.rules.RuleType;
import org.apache.doris.nereids.trees.plans.Plan;

import com.google.common.collect.ImmutableList;
import org.apache.commons.lang3.StringUtils;

import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;

/**
* Check bound rule to check semantic correct after bounding of expression by Nereids.
* Also give operator information without LOGICAL_
*/
public class CheckBound implements AnalysisRuleFactory {

@Override
public List<Rule> buildRules() {
return ImmutableList.of(
RuleType.CHECK_BOUND.build(
any().then(plan -> {
checkBound(plan);
return null;
})
)
);
}

private void checkBound(Plan plan) {
Set<Unbound> unbounds = plan.getExpressions().stream()
.<Set<Unbound>>map(e -> e.collect(Unbound.class::isInstance))
.flatMap(Set::stream)
.collect(Collectors.toSet());
if (!unbounds.isEmpty()) {
throw new AnalysisException(String.format("unbounded object %s in %s clause.",
StringUtils.join(unbounds.stream()
.map(unbound -> {
if (unbound instanceof UnboundSlot) {
return ((UnboundSlot) unbound).toSql();
} else if (unbound instanceof UnboundFunction) {
return ((UnboundFunction) unbound).toSql();
}
return unbound.toString();
})
.collect(Collectors.toSet()), ", "),
plan.getType().toString().substring("LOGICAL_".length())
));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public void testCannotFindSlot() {
new LogicalOlapScan(RelationUtil.newRelationId(), PlanConstructor.student));
AnalysisException exception = Assertions.assertThrows(AnalysisException.class,
() -> PlanChecker.from(MemoTestUtils.createConnectContext()).analyze(project));
Assertions.assertEquals("unbounded object foo.", exception.getMessage());
Assertions.assertEquals("unbounded object foo in PROJECT clause.", exception.getMessage());
}

@Test
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// 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("test_bound_exception") {
sql "SET enable_nereids_planner=true"
sql "SET enable_fallback_to_original_planner=false"
def tbName = "test_bound_exception"
def dbName = "test_bound_db"
sql "CREATE DATABASE IF NOT EXISTS ${dbName}"
sql "USE ${dbName}"

sql """ DROP TABLE IF EXISTS ${tbName} """
sql """
create table if not exists ${tbName} (id int, name char(10))
distributed by hash(id) buckets 1 properties("replication_num"="1");
"""
test {
sql "SELECT id FROM ${tbName} GROUP BY id ORDER BY id123"
exception "errCode = 2, detailMessage = Unexpected exception: unbounded object id123 in SORT clause."
}
test {
sql "SELECT id123 FROM ${tbName} ORDER BY id"
exception "errCode = 2, detailMessage = Unexpected exception: unbounded object id123 in PROJECT clause."
}
test {
sql "SELECT id123 FROM ${tbName} GROUP BY id ORDER BY id"
exception "errCode = 2, detailMessage = Unexpected exception: unbounded object id123 in AGGREGATE clause."
}
test {
sql "SELECT id FROM ${tbName} GROUP BY id123 ORDER BY id"
exception "errCode = 2, detailMessage = Unexpected exception: cannot bind GROUP BY KEY: id123"
}
test {
sql "SELECT id FROM ${tbName} WHERE id = (SELECT id from ${tbName} ORDER BY id123 LIMIT 1) ORDER BY id"
exception "errCode = 2, detailMessage = Unexpected exception: unbounded object id123 in SORT clause."
}
test {
sql "SELECT id FROM ${tbName} WHERE id123 = 123 ORDER BY id"
exception "errCode = 2, detailMessage = Unexpected exception: Invalid call to dataType on unbound object"
}
}

0 comments on commit b1a002a

Please sign in to comment.