This is an automated email from the ASF dual-hosted git repository.
zykkk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new d4b2515867b [improvement](catalog) Escape characters for columns in
recovery predicate pushdown in SQL (#29854)
d4b2515867b is described below
commit d4b2515867b68c58abf03012d1ad98ed330b3811
Author: zy-kkk <[email protected]>
AuthorDate: Mon Jan 15 11:36:18 2024 +0800
[improvement](catalog) Escape characters for columns in recovery predicate
pushdown in SQL (#29854)
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 | 31 +++++++++++++----
.../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, 88 insertions(+), 52 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 68a6da391af..2a96609c27c 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 c087107e5e0..869ace0ef14 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;
@@ -88,7 +90,9 @@ public abstract class Expr extends TreeNode<Expr> implements
ParseNode, Cloneabl
public static final String DEFAULT_EXPR_NAME = "expr";
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;
@@ -973,10 +977,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);
}
}
@@ -1006,10 +1013,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 a1175a5bff6..c27eebeb669 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;
@@ -246,12 +249,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) {
@@ -296,6 +295,26 @@ 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) inputTable).getProperRealColumnName(
+ ((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 1ebff21a234..76dbd40abab 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;
@@ -325,7 +327,7 @@ public class JdbcScanNode extends ExternalScanNode {
return !fnExprList.isEmpty();
}
- public static String conjunctExprToString(TOdbcTableType tableType, Expr
expr, JdbcTable tbl) {
+ public static String conjunctExprToString(TOdbcTableType tableType, Expr
expr, TableIf tbl) {
if (expr instanceof CompoundPredicate) {
StringBuilder result = new StringBuilder();
CompoundPredicate compoundPredicate = (CompoundPredicate) expr;
@@ -358,54 +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) {
- if (tbl != null) {
- filter = tbl.getProperRealColumnName(tableType,
children.get(0).toMySql());
- } else {
- 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() + "'";
@@ -413,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 d827ec4a4eb..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,
null);
+ 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 70957b94e2e..4a7c9a9035d 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 56c9d097cd1..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]
