This is an automated email from the ASF dual-hosted git repository.
zykkk pushed a commit to branch branch-2.0
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-2.0 by this push:
new dbdc17ca32e [improvement](catalog) Escape characters for columns in
recovery predicate pushdown in SQL (#29854) (#30060)
dbdc17ca32e is described below
commit dbdc17ca32e1ee05e6c9df45feb5488558cfe6ca
Author: zy-kkk <[email protected]>
AuthorDate: Mon Jan 29 11:55:22 2024 +0800
[improvement](catalog) Escape characters for columns in recovery predicate
pushdown in SQL (#29854) (#30060)
In the previous logic, when we restored the Column in the predicate
pushdown based on the logical syntax tree for JdbcScanNode, in order to avoid
query errors caused by keywords such as `key`, we added escape characters for
it, but before we only Binary predicates are processed, which is imperfect. We
should add escape characters to all columns that appear in the predicate to
avoid errors with keywords or illegal characters.
---
.../java/org/apache/doris/analysis/CastExpr.java | 2 +-
.../main/java/org/apache/doris/analysis/Expr.java | 23 ++++++++-----
.../java/org/apache/doris/analysis/SlotRef.java | 30 +++++++++++++----
.../org/apache/doris/planner/MysqlScanNode.java | 3 +-
.../external/jdbc/JdbcFunctionPushDownRule.java | 4 ++-
.../doris/planner/external/jdbc/JdbcScanNode.java | 39 ++++++++--------------
.../doris/planner/external/odbc/OdbcScanNode.java | 2 +-
.../jdbc/test_mysql_jdbc_catalog.out | 19 ++++++++++-
.../jdbc/test_clickhouse_jdbc_catalog.groovy | 4 +--
.../jdbc/test_mysql_jdbc_catalog.groovy | 13 +++++---
10 files changed, 89 insertions(+), 50 deletions(-)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java
index dc21eff6563..d592f50f352 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/CastExpr.java
@@ -204,7 +204,7 @@ public class CastExpr extends Expr {
@Override
public String toSqlImpl() {
- if (needToMysql) {
+ if (needExternalSql) {
return getChild(0).toSql();
}
if (isAnalyzed) {
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/Expr.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/Expr.java
index bc463611a16..f1c2d90270c 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/Expr.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/Expr.java
@@ -33,6 +33,8 @@ import org.apache.doris.catalog.MaterializedIndexMeta;
import org.apache.doris.catalog.PrimitiveType;
import org.apache.doris.catalog.ScalarFunction;
import org.apache.doris.catalog.ScalarType;
+import org.apache.doris.catalog.TableIf;
+import org.apache.doris.catalog.TableIf.TableType;
import org.apache.doris.catalog.Type;
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.Config;
@@ -84,7 +86,9 @@ public abstract class Expr extends TreeNode<Expr> implements
ParseNode, Cloneabl
public static final String AGG_MERGE_SUFFIX = "_merge";
protected boolean disableTableName = false;
- protected boolean needToMysql = false;
+ protected boolean needExternalSql = false;
+ protected TableType tableType = null;
+ protected TableIf inputTable = null;
// to be used where we can't come up with a better estimate
public static final double DEFAULT_SELECTIVITY = 0.1;
@@ -959,10 +963,13 @@ public abstract class Expr extends TreeNode<Expr>
implements ParseNode, Cloneabl
}
}
- public void setNeedToMysql(boolean value) {
- needToMysql = value;
+ public void setExternalContext(boolean needExternalSql, TableType
tableType, TableIf inputTable) {
+ this.needExternalSql = needExternalSql;
+ this.tableType = tableType;
+ this.inputTable = inputTable;
+
for (Expr child : children) {
- child.setNeedToMysql(value);
+ child.setExternalContext(needExternalSql, tableType, inputTable);
}
}
@@ -992,10 +999,10 @@ public abstract class Expr extends TreeNode<Expr>
implements ParseNode, Cloneabl
return toSqlImpl();
}
- public String toMySql() {
- setNeedToMysql(true);
- String result = toSql();
- setNeedToMysql(false);
+ public String toExternalSql(TableType tableType, TableIf table) {
+ setExternalContext(true, tableType, table);
+ String result = toSql();
+ setExternalContext(false, null, null);
return result;
}
diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java
b/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java
index cf945f540d0..bab3ac1baef 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java
@@ -21,8 +21,11 @@
package org.apache.doris.analysis;
import org.apache.doris.catalog.Column;
+import org.apache.doris.catalog.JdbcTable;
import org.apache.doris.catalog.MaterializedIndexMeta;
+import org.apache.doris.catalog.OdbcTable;
import org.apache.doris.catalog.TableIf;
+import org.apache.doris.catalog.TableIf.TableType;
import org.apache.doris.catalog.Type;
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.io.Text;
@@ -232,12 +235,8 @@ public class SlotRef extends Expr {
@Override
public String toSqlImpl() {
- if (needToMysql) {
- if (col != null) {
- return col;
- } else {
- return "<slot " + Integer.toString(desc.getId().asInt()) + ">";
- }
+ if (needExternalSql) {
+ return toExternalSqlImpl();
}
if (disableTableName && label != null) {
@@ -282,6 +281,25 @@ public class SlotRef extends Expr {
}
}
+ private String toExternalSqlImpl() {
+ if (col != null) {
+ if (tableType.equals(TableType.JDBC_EXTERNAL_TABLE) ||
tableType.equals(TableType.JDBC) || tableType
+ .equals(TableType.ODBC)) {
+ if (inputTable instanceof JdbcTable) {
+ return JdbcTable.databaseProperName(((JdbcTable)
inputTable).getJdbcTableType(), col);
+ } else if (inputTable instanceof OdbcTable) {
+ return JdbcTable.databaseProperName(((OdbcTable)
inputTable).getOdbcTableType(), col);
+ } else {
+ return col;
+ }
+ } else {
+ return col;
+ }
+ } else {
+ return "<slot " + Integer.toString(desc.getId().asInt()) + ">";
+ }
+ }
+
public TableName getTableName() {
if (tblName == null) {
Preconditions.checkState(isAnalyzed);
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/planner/MysqlScanNode.java
b/fe/fe-core/src/main/java/org/apache/doris/planner/MysqlScanNode.java
index 2f3e49b8d9b..98d09d4e6e2 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/planner/MysqlScanNode.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/planner/MysqlScanNode.java
@@ -25,6 +25,7 @@ import org.apache.doris.analysis.SlotRef;
import org.apache.doris.analysis.TupleDescriptor;
import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.MysqlTable;
+import org.apache.doris.catalog.TableIf.TableType;
import org.apache.doris.common.UserException;
import org.apache.doris.planner.external.ExternalScanNode;
import org.apache.doris.statistics.StatisticalType;
@@ -144,7 +145,7 @@ public class MysqlScanNode extends ExternalScanNode {
}
ArrayList<Expr> mysqlConjuncts = Expr.cloneList(conjuncts, sMap);
for (Expr p : mysqlConjuncts) {
- filters.add(p.toMySql());
+ filters.add(p.toExternalSql(TableType.MYSQL, null));
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/planner/external/jdbc/JdbcFunctionPushDownRule.java
b/fe/fe-core/src/main/java/org/apache/doris/planner/external/jdbc/JdbcFunctionPushDownRule.java
index 35d069c12c7..27fd693ca4d 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/planner/external/jdbc/JdbcFunctionPushDownRule.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/planner/external/jdbc/JdbcFunctionPushDownRule.java
@@ -20,6 +20,7 @@ package org.apache.doris.planner.external.jdbc;
import org.apache.doris.analysis.Expr;
import org.apache.doris.analysis.FunctionCallExpr;
import org.apache.doris.analysis.FunctionName;
+import org.apache.doris.catalog.TableIf.TableType;
import org.apache.doris.thrift.TOdbcTableType;
import com.google.common.base.Preconditions;
@@ -108,7 +109,8 @@ public class JdbcFunctionPushDownRule {
Preconditions.checkArgument(!func.isEmpty(), "function can not be
empty");
if (checkFunction.test(func)) {
- String errMsg = "Unsupported function: " + func + " in expr: "
+ expr.toMySql()
+ String errMsg = "Unsupported function: " + func + " in expr: "
+ expr.toExternalSql(
+ TableType.JDBC_EXTERNAL_TABLE, null)
+ " in JDBC Table Type: " + tableType;
LOG.warn(errMsg);
errors.add(errMsg);
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 6dcbfcab00c..d2e81233fee 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
@@ -33,6 +33,8 @@ import org.apache.doris.analysis.TupleDescriptor;
import org.apache.doris.catalog.Column;
import org.apache.doris.catalog.Env;
import org.apache.doris.catalog.JdbcTable;
+import org.apache.doris.catalog.TableIf;
+import org.apache.doris.catalog.TableIf.TableType;
import org.apache.doris.catalog.external.JdbcExternalTable;
import org.apache.doris.common.AnalysisException;
import org.apache.doris.common.Config;
@@ -139,7 +141,7 @@ public class JdbcScanNode extends ExternalScanNode {
List<Expr> pushDownConjuncts =
collectConjunctsToPushDown(conjunctsList, errors);
for (Expr individualConjunct : pushDownConjuncts) {
- String filter = conjunctExprToString(jdbcType, individualConjunct);
+ String filter = conjunctExprToString(jdbcType, individualConjunct,
tbl);
filters.add(filter);
conjuncts.remove(individualConjunct);
}
@@ -325,7 +327,7 @@ public class JdbcScanNode extends ExternalScanNode {
return !fnExprList.isEmpty();
}
- public static String conjunctExprToString(TOdbcTableType tableType, Expr
expr) {
+ public static String conjunctExprToString(TOdbcTableType tableType, Expr
expr, TableIf tbl) {
if (expr instanceof CompoundPredicate) {
StringBuilder result = new StringBuilder();
CompoundPredicate compoundPredicate = (CompoundPredicate) expr;
@@ -338,7 +340,7 @@ public class JdbcScanNode extends ExternalScanNode {
// 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));
+ result.append(conjunctExprToString(tableType, child, tbl));
// If the operator is not 'NOT', append the operator after
each child expression
if (!(compoundPredicate.getOp() == Operator.NOT)) {
@@ -358,50 +360,37 @@ public class JdbcScanNode extends ExternalScanNode {
if (expr.contains(DateLiteral.class) && expr instanceof
BinaryPredicate) {
ArrayList<Expr> children = expr.getChildren();
- String filter = children.get(0).toMySql();
+ String filter =
children.get(0).toExternalSql(TableType.JDBC_EXTERNAL_TABLE, tbl);
filter += " " + ((BinaryPredicate) expr).getOp().toString() + " ";
if (tableType.equals(TOdbcTableType.ORACLE)) {
- filter += handleOracleDateFormat(children.get(1));
+ filter += handleOracleDateFormat(children.get(1), tbl);
} else if (tableType.equals(TOdbcTableType.TRINO) ||
tableType.equals(TOdbcTableType.PRESTO)) {
- filter += handleTrinoDateFormat(children.get(1));
+ filter += handleTrinoDateFormat(children.get(1), tbl);
} else {
- filter += children.get(1).toMySql();
+ filter +=
children.get(1).toExternalSql(TableType.JDBC_EXTERNAL_TABLE, tbl);
}
return filter;
}
- if (expr.contains(SlotRef.class) && expr instanceof BinaryPredicate) {
- ArrayList<Expr> children = expr.getChildren();
- String filter;
- if (children.get(0) instanceof SlotRef) {
- filter = JdbcTable.databaseProperName(tableType,
children.get(0).toMySql());
- } else {
- filter = children.get(0).toMySql();
- }
- filter += " " + ((BinaryPredicate) expr).getOp().toString() + " ";
- filter += children.get(1).toMySql();
- return filter;
- }
-
// only for old planner
if (expr.contains(BoolLiteral.class) &&
"1".equals(expr.getStringValue()) && expr.getChildren().isEmpty()) {
return "1 = 1";
}
- return expr.toMySql();
+ return expr.toExternalSql(TableType.JDBC_EXTERNAL_TABLE, tbl);
}
- private static String handleOracleDateFormat(Expr expr) {
+ private static String handleOracleDateFormat(Expr expr, TableIf tbl) {
if (expr.isConstant()
&& (expr.getType().isDatetime() ||
expr.getType().isDatetimeV2())) {
return "to_date('" + expr.getStringValue() + "', 'yyyy-mm-dd
hh24:mi:ss')";
}
- return expr.toMySql();
+ return expr.toExternalSql(TableType.JDBC_EXTERNAL_TABLE, tbl);
}
- private static String handleTrinoDateFormat(Expr expr) {
+ private static String handleTrinoDateFormat(Expr expr, TableIf tbl) {
if (expr.isConstant()) {
if (expr.getType().isDate() || expr.getType().isDateV2()) {
return "date '" + expr.getStringValue() + "'";
@@ -409,6 +398,6 @@ public class JdbcScanNode extends ExternalScanNode {
return "timestamp '" + expr.getStringValue() + "'";
}
}
- return expr.toMySql();
+ return expr.toExternalSql(TableType.JDBC_EXTERNAL_TABLE, tbl);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/planner/external/odbc/OdbcScanNode.java
b/fe/fe-core/src/main/java/org/apache/doris/planner/external/odbc/OdbcScanNode.java
index d90140438f1..f3ba2a5add8 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/planner/external/odbc/OdbcScanNode.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/planner/external/odbc/OdbcScanNode.java
@@ -216,7 +216,7 @@ public class OdbcScanNode extends ExternalScanNode {
ArrayList<Expr> odbcConjuncts = Expr.cloneList(conjuncts, sMap);
for (Expr p : odbcConjuncts) {
if (shouldPushDownConjunct(odbcType, p)) {
- String filter = JdbcScanNode.conjunctExprToString(odbcType, p);
+ String filter = JdbcScanNode.conjunctExprToString(odbcType, p,
tbl);
filters.add(filter);
conjuncts.remove(p);
}
diff --git
a/regression-test/data/external_table_p0/jdbc/test_mysql_jdbc_catalog.out
b/regression-test/data/external_table_p0/jdbc/test_mysql_jdbc_catalog.out
index c81d86828c6..500f48b77a2 100644
--- a/regression-test/data/external_table_p0/jdbc/test_mysql_jdbc_catalog.out
+++ b/regression-test/data/external_table_p0/jdbc/test_mysql_jdbc_catalog.out
@@ -165,9 +165,26 @@ bca 2022-11-02 2022-11-02 8012 vivo
1.12345 1.12345 1.12345 1.12345 1.12345 1.12345
123456789012345678901234567890123.12345
12345678901234567890123456789012.12345
1234567890123456789012345678901234.12345
123456789012345678901234567890123.12345
123456789012345678901234567890123456789012345678901234567890.12345
123456789012345678901234567890123456789012345678901234567890.12345
--- !ex_tb21 --
+-- !ex_tb21_1 --
2 2
+-- !ex_tb21_2 --
+2 2
+
+-- !ex_tb21_3 --
+1 1
+2 2
+
+-- !ex_tb21_4 --
+2 2
+
+-- !ex_tb21_5 --
+1 1
+2 2
+
+-- !ex_tb21_6 --
+1 1
+
-- !information_schema --
CHARACTER_SETS
COLLATIONS
diff --git
a/regression-test/suites/external_table_p0/jdbc/test_clickhouse_jdbc_catalog.groovy
b/regression-test/suites/external_table_p0/jdbc/test_clickhouse_jdbc_catalog.groovy
index 3870469ef53..9948c49d24a 100644
---
a/regression-test/suites/external_table_p0/jdbc/test_clickhouse_jdbc_catalog.groovy
+++
b/regression-test/suites/external_table_p0/jdbc/test_clickhouse_jdbc_catalog.groovy
@@ -87,7 +87,7 @@ suite("test_clickhouse_jdbc_catalog",
"p0,external,clickhouse,external_docker,ex
order_qt_func_push """select * from ts where
from_unixtime(ts,'yyyyMMdd') >= '2022-01-01';"""
explain {
sql("select * from ts where from_unixtime(ts,'yyyyMMdd') >=
'2022-01-01';")
- contains """QUERY: SELECT "id", "ts" FROM "doris_test"."ts"
WHERE (FROM_UNIXTIME(ts, '%Y%m%d') >= '2022-01-01')"""
+ contains """QUERY: SELECT "id", "ts" FROM "doris_test"."ts"
WHERE (FROM_UNIXTIME("ts", '%Y%m%d') >= '2022-01-01')"""
}
explain {
sql("select * from ts where nvl(ts,null) >= '2022-01-01';")
@@ -96,7 +96,7 @@ suite("test_clickhouse_jdbc_catalog",
"p0,external,clickhouse,external_docker,ex
order_qt_func_push2 """select * from ts where ts <=
unix_timestamp(from_unixtime(ts,'yyyyMMdd'));"""
explain {
sql("select * from ts where ts <=
unix_timestamp(from_unixtime(ts,'yyyy-MM-dd'));")
- contains """QUERY: SELECT "id", "ts" FROM "doris_test"."ts"
WHERE ("ts" <= toUnixTimestamp(FROM_UNIXTIME(ts, '%Y-%m-%d')))"""
+ contains """QUERY: SELECT "id", "ts" FROM "doris_test"."ts"
WHERE ("ts" <= toUnixTimestamp(FROM_UNIXTIME("ts", '%Y-%m-%d')))"""
}
order_qt_dt_with_tz """ select * from dt_with_tz order by id; """
diff --git
a/regression-test/suites/external_table_p0/jdbc/test_mysql_jdbc_catalog.groovy
b/regression-test/suites/external_table_p0/jdbc/test_mysql_jdbc_catalog.groovy
index 2b343a8e017..ec09d2a319d 100644
---
a/regression-test/suites/external_table_p0/jdbc/test_mysql_jdbc_catalog.groovy
+++
b/regression-test/suites/external_table_p0/jdbc/test_mysql_jdbc_catalog.groovy
@@ -166,7 +166,12 @@ suite("test_mysql_jdbc_catalog",
"p0,external,mysql,external_docker,external_doc
order_qt_ex_tb18 """ select * from ${ex_tb18} order by
num_tinyint; """
order_qt_ex_tb19 """ select * from ${ex_tb19} order by
date_value; """
order_qt_ex_tb20 """ select * from ${ex_tb20} order by
decimal_normal; """
- order_qt_ex_tb21 """ select `key`, `id` from ${ex_tb21} where
`key` = 2 order by id;"""
+ order_qt_ex_tb21_1 """ select `key`, `id` from ${ex_tb21} where
`key` = 2 order by id;"""
+ order_qt_ex_tb21_2 """ select `key`, `id` from ${ex_tb21} where
`key` like 2 order by id;"""
+ order_qt_ex_tb21_3 """ select `key`, `id` from ${ex_tb21} where
`key` in (1,2) order by id;"""
+ order_qt_ex_tb21_4 """ select `key`, `id` from ${ex_tb21} where
abs(`key`) = 2 order by id;"""
+ order_qt_ex_tb21_5 """ select `key`, `id` from ${ex_tb21} where
`key` between 1 and 2 order by id;"""
+ order_qt_ex_tb21_6 """ select `key`, `id` from ${ex_tb21} where
`key` = case when id = 1 then 1 else 0 end order by id;"""
order_qt_information_schema """ show tables from
information_schema; """
order_qt_auto_default_t """insert into ${auto_default_t}(name)
values('a'); """
order_qt_dt """select * from ${dt}; """
@@ -386,17 +391,17 @@ suite("test_mysql_jdbc_catalog",
"p0,external,mysql,external_docker,external_doc
explain {
sql ("SELECT timestamp0 from dt where
DATE_TRUNC(date_sub(timestamp0,INTERVAL 9 HOUR),'hour') > '2011-03-03 17:39:05'
and timestamp0 > '2022-01-01';")
- contains "QUERY: SELECT `timestamp0` FROM `doris_test`.`dt`
WHERE (timestamp0 > '2022-01-01 00:00:00')"
+ contains "QUERY: SELECT `timestamp0` FROM `doris_test`.`dt`
WHERE (`timestamp0` > '2022-01-01 00:00:00')"
}
explain {
sql ("select k6, k8 from test1 where nvl(k6, null) = 1;")
- contains "QUERY: SELECT `k6`, `k8` FROM `doris_test`.`test1`
WHERE (ifnull(k6, NULL) = 1)"
+ contains "QUERY: SELECT `k6`, `k8` FROM `doris_test`.`test1`
WHERE (ifnull(`k6`, NULL) = 1)"
}
explain {
sql ("select k6, k8 from test1 where nvl(nvl(k6, null),null) =
1;")
- contains "QUERY: SELECT `k6`, `k8` FROM `doris_test`.`test1`
WHERE (ifnull(ifnull(k6, NULL), NULL) = 1)"
+ contains "QUERY: SELECT `k6`, `k8` FROM `doris_test`.`test1`
WHERE (ifnull(ifnull(`k6`, NULL), NULL) = 1)"
}
sql """ admin set frontend config ("enable_func_pushdown" =
"false"); """
explain {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
