From 4538097924759d6aa22285027d33822dff699492 Mon Sep 17 00:00:00 2001 From: lichi Date: Mon, 17 Feb 2025 21:21:18 +0800 Subject: [PATCH 1/2] [fix](nereids)check if correlated filter exists before converting apply to join --- .../rules/rewrite/ExistsApplyToJoin.java | 2 +- .../nereids/rules/rewrite/InApplyToJoin.java | 2 +- .../rules/rewrite/ScalarApplyToJoin.java | 2 +- .../test_correlated_filter_removed.groovy | 54 +++++++++++++++++++ 4 files changed, 57 insertions(+), 3 deletions(-) create mode 100644 regression-test/suites/nereids_p0/subquery/test_correlated_filter_removed.groovy diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ExistsApplyToJoin.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ExistsApplyToJoin.java index 003aa1028602e9..daef2a780dce44 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ExistsApplyToJoin.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ExistsApplyToJoin.java @@ -81,7 +81,7 @@ public class ExistsApplyToJoin extends OneRewriteRuleFactory { @Override public Rule build() { return logicalApply().when(LogicalApply::isExist).then(apply -> { - if (apply.isCorrelated()) { + if (apply.isCorrelated() && apply.getCorrelationFilter().isPresent()) { return correlatedToJoin(apply); } else { return unCorrelatedToJoin(apply); diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InApplyToJoin.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InApplyToJoin.java index fdf6efb81675d5..999c28780d6e9b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InApplyToJoin.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InApplyToJoin.java @@ -122,7 +122,7 @@ select t1.k1 from bigtable t1 left semi join (select bitmap_union(k2) x from bit new DistributeHint(DistributeType.NONE), apply.getMarkJoinSlotReference(), apply.children(), null); } else { - if (apply.isCorrelated()) { + if (apply.isCorrelated() && apply.getCorrelationFilter().isPresent()) { if (inSubquery.isNot()) { predicate = ExpressionUtils.and(ExpressionUtils.or(new EqualTo(left, right), new IsNull(left), new IsNull(right)), diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ScalarApplyToJoin.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ScalarApplyToJoin.java index a7b2b9a2045a38..da4d5261f368f8 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ScalarApplyToJoin.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ScalarApplyToJoin.java @@ -45,7 +45,7 @@ public class ScalarApplyToJoin extends OneRewriteRuleFactory { @Override public Rule build() { return logicalApply().when(LogicalApply::isScalar).then(apply -> { - if (apply.isCorrelated()) { + if (apply.isCorrelated() && apply.getCorrelationFilter().isPresent()) { return correlatedToJoin(apply); } else { return unCorrelatedToJoin(apply); diff --git a/regression-test/suites/nereids_p0/subquery/test_correlated_filter_removed.groovy b/regression-test/suites/nereids_p0/subquery/test_correlated_filter_removed.groovy new file mode 100644 index 00000000000000..2a3b5124048a42 --- /dev/null +++ b/regression-test/suites/nereids_p0/subquery/test_correlated_filter_removed.groovy @@ -0,0 +1,54 @@ +// 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_correlated_filter_removed") { + multi_sql """ + drop table if exists table_6_undef_partitions2_keys3_properties4_distributed_by5; + + create table table_6_undef_partitions2_keys3_properties4_distributed_by5 ( + col_int_undef_signed int/*agg_type_placeholder*/ , + col_varchar_10__undef_signed varchar(10)/*agg_type_placeholder*/ , + pk int/*agg_type_placeholder*/ + ) engine=olap + distributed by hash(pk) buckets 10 + properties("replication_num" = "1"); + + insert into table_6_undef_partitions2_keys3_properties4_distributed_by5(pk,col_int_undef_signed,col_varchar_10__undef_signed) values (0,null,'think'),(1,null,''),(2,2,''),(3,null,'r'),(4,null,null),(5,8,'here'); + + drop table if exists table_100_undef_partitions2_keys3_properties4_distributed_by5; + + create table table_100_undef_partitions2_keys3_properties4_distributed_by5 ( + col_int_undef_signed int/*agg_type_placeholder*/ , + col_varchar_10__undef_signed varchar(10)/*agg_type_placeholder*/ , + pk int/*agg_type_placeholder*/ + ) engine=olap + distributed by hash(pk) buckets 10 + properties("replication_num" = "1"); + + insert into table_100_undef_partitions2_keys3_properties4_distributed_by5(pk,col_int_undef_signed,col_varchar_10__undef_signed) values (0,null,''),(1,null,''),(2,null,''),(3,0,null),(4,7,null),(5,9,'d'),(6,9,null),(7,null,null),(8,null,null),(9,null,''),(10,null,'are'),(11,null,'were'),(12,2,''),(13,null,'one'),(14,null,'ok'),(15,null,'your'),(16,null,''),(17,null,null),(18,4,''),(19,null,null),(20,null,null),(21,null,null),(22,3,''),(23,null,null),(24,8,''),(25,2,'I''m'),(26,null,'e'),(27,3,'will'),(28,null,null),(29,3,'q'),(30,1,null),(31,3,null),(32,null,'p'),(33,null,'but'),(34,null,'v'),(35,null,'the'),(36,1,''),(37,null,'t'),(38,1,''),(39,null,''),(40,null,''),(41,4,null),(42,null,'just'),(43,4,null),(44,null,null),(45,null,null),(46,8,null),(47,5,''),(48,2,null),(49,null,'n'),(50,null,'p'),(51,1,'we'),(52,3,''),(53,null,'about'),(54,0,''),(55,2,'be'),(56,0,''),(57,null,null),(58,null,null),(59,null,null),(60,9,'how'),(61,null,'in'),(62,null,'me'),(63,5,null),(64,null,'I''ll'),(65,null,'j'),(66,8,null),(67,6,'g'),(68,1,null),(69,7,'if'),(70,3,null),(71,null,'hey'),(72,null,'I''m'),(73,4,''),(74,null,''),(75,6,'y'),(76,1,''),(77,3,'f'),(78,null,'c'),(79,4,'x'),(80,null,''),(81,4,null),(82,null,''),(83,4,'r'),(84,null,''),(85,null,''),(86,null,null),(87,2,''),(88,null,'f'),(89,0,null),(90,9,'g'),(91,null,'I''ll'),(92,8,'p'),(93,5,'really'),(94,null,null),(95,0,'up'),(96,4,'k'),(97,null,'if'),(98,1,'f'),(99,1,'q'); + """ + + sql """ + SELECT * + FROM table_6_undef_partitions2_keys3_properties4_distributed_by5 AS t1 + WHERE t1.`pk` + 2 IN + (SELECT 1 + FROM table_100_undef_partitions2_keys3_properties4_distributed_by5 AS t2 + WHERE t1.col_int_undef_signed is NULL + AND t1.col_int_undef_signed is NOT NULL) ; + """ +} From 3460d5fce41e38dcc4ad3bc95738dab6110c73dd Mon Sep 17 00:00:00 2001 From: lichi Date: Tue, 18 Feb 2025 12:28:34 +0800 Subject: [PATCH 2/2] add comments --- .../apache/doris/nereids/rules/rewrite/ExistsApplyToJoin.java | 3 +++ .../org/apache/doris/nereids/rules/rewrite/InApplyToJoin.java | 4 ++++ .../apache/doris/nereids/rules/rewrite/ScalarApplyToJoin.java | 3 +++ 3 files changed, 10 insertions(+) diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ExistsApplyToJoin.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ExistsApplyToJoin.java index daef2a780dce44..930e4c467dfe1a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ExistsApplyToJoin.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ExistsApplyToJoin.java @@ -81,6 +81,9 @@ public class ExistsApplyToJoin extends OneRewriteRuleFactory { @Override public Rule build() { return logicalApply().when(LogicalApply::isExist).then(apply -> { + // apply.isCorrelated() only check if correlated slot exits + // but correlation filter may be eliminated by SimplifyConflictCompound rule + // so we need check both correlated slot and correlation filter exists before creating LogicalJoin node if (apply.isCorrelated() && apply.getCorrelationFilter().isPresent()) { return correlatedToJoin(apply); } else { diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InApplyToJoin.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InApplyToJoin.java index 999c28780d6e9b..67276e92bb31d2 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InApplyToJoin.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/InApplyToJoin.java @@ -122,6 +122,10 @@ select t1.k1 from bigtable t1 left semi join (select bitmap_union(k2) x from bit new DistributeHint(DistributeType.NONE), apply.getMarkJoinSlotReference(), apply.children(), null); } else { + // apply.isCorrelated() only check if correlated slot exits + // but correlation filter may be eliminated by SimplifyConflictCompound rule + // so we need check both correlated slot and correlation filter exists + // before creating LogicalJoin node if (apply.isCorrelated() && apply.getCorrelationFilter().isPresent()) { if (inSubquery.isNot()) { predicate = ExpressionUtils.and(ExpressionUtils.or(new EqualTo(left, right), diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ScalarApplyToJoin.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ScalarApplyToJoin.java index da4d5261f368f8..5cb11914b66d6b 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ScalarApplyToJoin.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/rules/rewrite/ScalarApplyToJoin.java @@ -45,6 +45,9 @@ public class ScalarApplyToJoin extends OneRewriteRuleFactory { @Override public Rule build() { return logicalApply().when(LogicalApply::isScalar).then(apply -> { + // apply.isCorrelated() only check if correlated slot exits + // but correlation filter may be eliminated by SimplifyConflictCompound rule + // so we need check both correlated slot and correlation filter exists before creating LogicalJoin node if (apply.isCorrelated() && apply.getCorrelationFilter().isPresent()) { return correlatedToJoin(apply); } else {