From 8d43c503bdc36b6d4410fd7b07240cd7703d7400 Mon Sep 17 00:00:00 2001 From: HappenLee Date: Mon, 19 Oct 2020 16:26:56 +0800 Subject: [PATCH 1/2] [Bug] Do not push down limit operation when ODBC table do not push all conjunct as filter. issue:4761 --- be/src/exec/odbc_scan_node.cpp | 8 +++++++- .../java/org/apache/doris/planner/OdbcScanNode.java | 12 +++++++++--- .../java/org/apache/doris/planner/QueryPlanTest.java | 9 ++++++++- 3 files changed, 24 insertions(+), 5 deletions(-) diff --git a/be/src/exec/odbc_scan_node.cpp b/be/src/exec/odbc_scan_node.cpp index 14ea0eb1112e57..ae30d63fed4c21 100644 --- a/be/src/exec/odbc_scan_node.cpp +++ b/be/src/exec/odbc_scan_node.cpp @@ -141,6 +141,11 @@ Status OdbcScanNode::get_next(RuntimeState* state, RowBatch* row_batch, bool* eo SCOPED_TIMER(_runtime_profile->total_time_counter()); SCOPED_TIMER(materialize_tuple_timer()); + if (reached_limit()) { + *eos = true; + return Status::OK(); + } + // create new tuple buffer for row_batch int tuple_buffer_size = row_batch->capacity() * _tuple_desc->byte_size(); void* tuple_buffer = _tuple_pool->allocate(tuple_buffer_size); @@ -156,10 +161,11 @@ Status OdbcScanNode::get_next(RuntimeState* state, RowBatch* row_batch, bool* eo while (true) { RETURN_IF_CANCELLED(state); - if (row_batch->is_full()) { + if (reached_limit() || row_batch->is_full()) { // hang on to last allocated chunk in pool, we'll keep writing into it in the // next get_next() call row_batch->tuple_data_pool()->acquire_data(_tuple_pool.get(), !reached_limit()); + *eos = reached_limit(); return Status::OK(); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/planner/OdbcScanNode.java b/fe/fe-core/src/main/java/org/apache/doris/planner/OdbcScanNode.java index 6a37415b7e67f7..e7cd863ccf8944 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/planner/OdbcScanNode.java +++ b/fe/fe-core/src/main/java/org/apache/doris/planner/OdbcScanNode.java @@ -114,16 +114,22 @@ protected String getNodeExplainString(String prefix, TExplainLevel detailLevel) return output.toString(); } + // only all conjuncts be pushed down as filter, we can + // push down limit operation to ODBC table + private boolean needPushDownLimit() { + return limit != -1 && conjuncts.isEmpty(); + } + private String getOdbcQueryStr() { StringBuilder sql = new StringBuilder("SELECT "); // Oracle use the where clause to do top n - if (limit != -1 && odbcType == TOdbcTableType.ORACLE) { + if (needPushDownLimit() && odbcType == TOdbcTableType.ORACLE) { filters.add("ROWNUM <= " + limit); } // MSSQL use select top to do top n - if (limit != -1 && odbcType == TOdbcTableType.SQLSERVER) { + if (needPushDownLimit() && odbcType == TOdbcTableType.SQLSERVER) { sql.append("TOP " + limit + " "); } @@ -137,7 +143,7 @@ private String getOdbcQueryStr() { } // Other DataBase use limit do top n - if (limit != -1 && (odbcType == TOdbcTableType.MYSQL || odbcType == TOdbcTableType.POSTGRESQL || odbcType == TOdbcTableType.MONGODB) ) { + if (needPushDownLimit() && (odbcType == TOdbcTableType.MYSQL || odbcType == TOdbcTableType.POSTGRESQL || odbcType == TOdbcTableType.MONGODB) ) { sql.append(" LIMIT " + limit); } diff --git a/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java b/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java index ba3e24ce9572f9..71a8cbb5c2cdc3 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/planner/QueryPlanTest.java @@ -1223,9 +1223,16 @@ public void testLimitOfExternalTable() throws Exception { String explainString = UtFrameUtils.getSQLPlanOrErrorMsg(connectContext, queryStr); Assert.assertTrue(explainString.contains("LIMIT 10")); - // ODBC table (Oracle) + // ODBC table (Oracle) not push down limit queryStr = "explain select * from odbc_oracle where k1 > 10 and abs(k1) > 10 limit 10"; explainString = UtFrameUtils.getSQLPlanOrErrorMsg(connectContext, queryStr); + // abs is function, so Doris do not push down function except MySQL Database + // so should not push down limit operation + Assert.assertTrue(!explainString.contains("ROWNUM <= 10")); + + // ODBC table (Oracle) push down limit + queryStr = "explain select * from odbc_oracle where k1 > 10 limit 10"; + explainString = UtFrameUtils.getSQLPlanOrErrorMsg(connectContext, queryStr); Assert.assertTrue(explainString.contains("ROWNUM <= 10")); // MySQL table From c6005abf3c5979cd387863e45069bba77046dadb Mon Sep 17 00:00:00 2001 From: HappenLee Date: Mon, 19 Oct 2020 20:05:35 +0800 Subject: [PATCH 2/2] refactor some function name --- .../java/org/apache/doris/planner/OdbcScanNode.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/planner/OdbcScanNode.java b/fe/fe-core/src/main/java/org/apache/doris/planner/OdbcScanNode.java index e7cd863ccf8944..0d6ce807d0cd91 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/planner/OdbcScanNode.java +++ b/fe/fe-core/src/main/java/org/apache/doris/planner/OdbcScanNode.java @@ -64,7 +64,7 @@ private static String databaseProperName(TOdbcTableType tableType, String name) // Now some database have different function call like doris, now doris do not // push down the function call except MYSQL - private static boolean needPushDown(TOdbcTableType tableType, Expr expr) { + private static boolean shouldPushDownConjunct(TOdbcTableType tableType, Expr expr) { if (!tableType.equals(TOdbcTableType.MYSQL)) { List fnExprList = Lists.newArrayList(); expr.collect(FunctionCallExpr.class, fnExprList); @@ -116,7 +116,7 @@ protected String getNodeExplainString(String prefix, TExplainLevel detailLevel) // only all conjuncts be pushed down as filter, we can // push down limit operation to ODBC table - private boolean needPushDownLimit() { + private boolean shouldPushDownLimit() { return limit != -1 && conjuncts.isEmpty(); } @@ -124,12 +124,12 @@ private String getOdbcQueryStr() { StringBuilder sql = new StringBuilder("SELECT "); // Oracle use the where clause to do top n - if (needPushDownLimit() && odbcType == TOdbcTableType.ORACLE) { + if (shouldPushDownLimit() && odbcType == TOdbcTableType.ORACLE) { filters.add("ROWNUM <= " + limit); } // MSSQL use select top to do top n - if (needPushDownLimit() && odbcType == TOdbcTableType.SQLSERVER) { + if (shouldPushDownLimit() && odbcType == TOdbcTableType.SQLSERVER) { sql.append("TOP " + limit + " "); } @@ -143,7 +143,7 @@ private String getOdbcQueryStr() { } // Other DataBase use limit do top n - if (needPushDownLimit() && (odbcType == TOdbcTableType.MYSQL || odbcType == TOdbcTableType.POSTGRESQL || odbcType == TOdbcTableType.MONGODB) ) { + if (shouldPushDownLimit() && (odbcType == TOdbcTableType.MYSQL || odbcType == TOdbcTableType.POSTGRESQL || odbcType == TOdbcTableType.MONGODB) ) { sql.append(" LIMIT " + limit); } @@ -181,7 +181,7 @@ private void createOdbcFilters(Analyzer analyzer) { } ArrayList odbcConjuncts = Expr.cloneList(conjuncts, sMap); for (Expr p : odbcConjuncts) { - if (needPushDown(odbcType, p)) { + if (shouldPushDownConjunct(odbcType, p)) { String filter = p.toMySql(); filters.add(filter); conjuncts.remove(p);