From a3e2c6affeb5038229096f9e45933ddc05d77569 Mon Sep 17 00:00:00 2001 From: zy-kkk Date: Sat, 16 Dec 2023 12:54:55 +0800 Subject: [PATCH] [fix](jdbc catalog) fix JdbcScanNode `NOT` CompoundPredicate filter expr handling errors (#28497) --- .../planner/external/jdbc/JdbcScanNode.java | 31 ++++++++--- .../jdbc/test_clickhouse_jdbc_catalog.out | Bin 4955 -> 4991 bytes .../jdbc/test_mysql_jdbc_catalog.out | 18 +++++++ .../jdbc/test_oracle_jdbc_catalog.out | 50 ++++++++++++++++++ .../jdbc/test_pg_jdbc_catalog.out | 6 +++ .../jdbc/test_sqlserver_jdbc_catalog.out | 8 +++ .../jdbc/test_clickhouse_jdbc_catalog.groovy | 2 + .../jdbc/test_mysql_jdbc_catalog.groovy | 2 + .../jdbc/test_oracle_jdbc_catalog.groovy | 14 +++++ .../jdbc/test_pg_jdbc_catalog.groovy | 2 + .../jdbc/test_sqlserver_jdbc_catalog.groovy | 2 + 11 files changed, 129 insertions(+), 6 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/planner/external/jdbc/JdbcScanNode.java b/fe/fe-core/src/main/java/org/apache/doris/planner/external/jdbc/JdbcScanNode.java index 4eccaff477..1ebff21a23 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/planner/external/jdbc/JdbcScanNode.java +++ b/fe/fe-core/src/main/java/org/apache/doris/planner/external/jdbc/JdbcScanNode.java @@ -21,6 +21,7 @@ import org.apache.doris.analysis.Analyzer; import org.apache.doris.analysis.BinaryPredicate; import org.apache.doris.analysis.BoolLiteral; import org.apache.doris.analysis.CompoundPredicate; +import org.apache.doris.analysis.CompoundPredicate.Operator; import org.apache.doris.analysis.DateLiteral; import org.apache.doris.analysis.Expr; import org.apache.doris.analysis.ExprSubstitutionMap; @@ -328,13 +329,31 @@ public class JdbcScanNode extends ExternalScanNode { if (expr instanceof CompoundPredicate) { StringBuilder result = new StringBuilder(); CompoundPredicate compoundPredicate = (CompoundPredicate) expr; - for (Expr child : compoundPredicate.getChildren()) { - result.append(conjunctExprToString(tableType, child, tbl)); - result.append(" ").append(compoundPredicate.getOp().toString()).append(" "); + + // If the operator is 'NOT', prepend 'NOT' to the start of the string + if (compoundPredicate.getOp() == Operator.NOT) { + result.append("NOT "); } - // Remove the last operator - result.setLength(result.length() - compoundPredicate.getOp().toString().length() - 2); - return result.toString(); + + // Iterate through all children of the CompoundPredicate + for (Expr child : compoundPredicate.getChildren()) { + // Recursively call conjunctExprToString for each child and append to the result + result.append(conjunctExprToString(tableType, child, tbl)); + + // If the operator is not 'NOT', append the operator after each child expression + if (!(compoundPredicate.getOp() == Operator.NOT)) { + result.append(" ").append(compoundPredicate.getOp().toString()).append(" "); + } + } + + // For operators other than 'NOT', remove the extra appended operator at the end + // This is necessary for operators like 'AND' or 'OR' that appear between child expressions + if (!(compoundPredicate.getOp() == Operator.NOT)) { + result.setLength(result.length() - compoundPredicate.getOp().toString().length() - 2); + } + + // Return the processed string trimmed of any extra spaces + return result.toString().trim(); } if (expr.contains(DateLiteral.class) && expr instanceof BinaryPredicate) { diff --git a/regression-test/data/external_table_p0/jdbc/test_clickhouse_jdbc_catalog.out b/regression-test/data/external_table_p0/jdbc/test_clickhouse_jdbc_catalog.out index 30401d56d4c24750162dae177dbcb67be389f5f6..4eba3e43641d804a791c8009feab7236f8487c01 100644 GIT binary patch delta 40 wcmcbu_Frwo0inqaLM##{3c9*nT)MgnifNfSC8 20) OR (SCORE < 90 AND NOT (NAME = 'alice' OR AGE <= 18)) order by ID; """ order_qt_date1 """ select * from TEST_DATE where T1 > '2022-01-21 00:00:00' or T1 < '2022-01-22 00:00:00'; """ order_qt_date2 """ select * from TEST_DATE where T1 > '2022-01-21 00:00:00' and T1 < '2022-01-22 00:00:00'; """ order_qt_date3 """ select * from TEST_DATE where (T1 > '2022-01-21 00:00:00' and T1 < '2022-01-22 00:00:00') or T1 > '2022-01-20 00:00:00'; """ @@ -128,6 +134,14 @@ suite("test_oracle_jdbc_catalog", "p0,external,oracle,external_docker,external_d order_qt_date6 """ select * from TEST_DATE where (T1 < '2022-01-22 00:00:00' or T1 > '2022-01-20 00:00:00') and (T1 < '2022-01-23 00:00:00' or T1 > '2022-01-19 00:00:00'); """ order_qt_date7 """select * from TEST_TIMESTAMP where T2 < str_to_date('2020-12-21 12:34:56', '%Y-%m-%d %H:%i:%s');""" + // for old planner + order_qt_filter4_old_plan """ select /*+ SET_VAR(enable_nereids_planner=false) */ * from STUDENT where NAME NOT like '%bob%' order by ID; """ + order_qt_filter5_old_plan """ select /*+ SET_VAR(enable_nereids_planner=false) */ * from STUDENT where NAME NOT like '%bob%' or NAME NOT LIKE '%jerry%' order by ID; """ + order_qt_filter6_old_plan """ select /*+ SET_VAR(enable_nereids_planner=false) */ * from STUDENT where NAME NOT like '%bob%' and NAME NOT LIKE '%jerry%' order by ID; """ + order_qt_filter7_old_plan """ select /*+ SET_VAR(enable_nereids_planner=false) */ * from STUDENT where NAME NOT like '%bob%' and NAME LIKE '%jerry%' order by ID; """ + order_qt_filter8_old_plan """ select /*+ SET_VAR(enable_nereids_planner=false) */ * from STUDENT where NAME NOT like '%bob%' and ID = 4 order by ID; """ + order_qt_filter9_old_plan """ SELECT /*+ SET_VAR(enable_nereids_planner=false) */ * FROM STUDENT WHERE (NAME NOT LIKE '%bob%' AND AGE > 20) OR (SCORE < 90 AND NOT (NAME = 'alice' OR AGE <= 18)) order by ID; """ + // The result of TEST_RAW will change // So instead of qt, we're using sql here. sql """ select * from TEST_RAW order by ID; """ diff --git a/regression-test/suites/external_table_p0/jdbc/test_pg_jdbc_catalog.groovy b/regression-test/suites/external_table_p0/jdbc/test_pg_jdbc_catalog.groovy index d44c24a479..dad935acdc 100644 --- a/regression-test/suites/external_table_p0/jdbc/test_pg_jdbc_catalog.groovy +++ b/regression-test/suites/external_table_p0/jdbc/test_pg_jdbc_catalog.groovy @@ -105,6 +105,8 @@ suite("test_pg_jdbc_catalog", "p0,external,pg,external_docker,external_docker_pg order_qt_test9 """ select * from test7 order by id; """ order_qt_test10 """ select * from test8 order by id; """ order_qt_test11 """ select * from test9 order by id1; """ + order_qt_filter4 """ select * from test3 where name not like '%abc%' order by id; """ + order_qt_filter4_old """ select /*+ SET_VAR(enable_nereids_planner=false) */ * from test3 where name not like '%abc%' order by id; """ sql """ use ${ex_schema_name2}""" order_qt_test12 """ select * from test10 order by id; """ diff --git a/regression-test/suites/external_table_p0/jdbc/test_sqlserver_jdbc_catalog.groovy b/regression-test/suites/external_table_p0/jdbc/test_sqlserver_jdbc_catalog.groovy index fde19f7a60..068f781fad 100644 --- a/regression-test/suites/external_table_p0/jdbc/test_sqlserver_jdbc_catalog.groovy +++ b/regression-test/suites/external_table_p0/jdbc/test_sqlserver_jdbc_catalog.groovy @@ -76,6 +76,8 @@ suite("test_sqlserver_jdbc_catalog", "p0,external,sqlserver,external_docker,exte order_qt_filter1 """ select * from test_char where 1 = 1 order by id; """ order_qt_filter2 """ select * from test_char where 1 = 1 and id = 1 order by id; """ order_qt_filter3 """ select * from test_char where id = 1 order by id; """ + order_qt_filter4 """ select * from student where name not like '%doris%' order by id; """ + order_qt_filter4_old """ select /*+ SET_VAR(enable_nereids_planner=false) */ * from student where name not like '%doris%' order by id; """ order_qt_id """ select count(*) from (select * from t_id) as a; """ order_qt_all_type """ select * from all_type order by id; """ sql """ drop table if exists internal.${internal_db_name}.all_type; """