From f5232e5c0173048afe1b27a9fa94d00c5afa9402 Mon Sep 17 00:00:00 2001 From: zhangstar333 <87313068+zhangstar333@users.noreply.github.com> Date: Fri, 3 Mar 2023 10:30:20 +0800 Subject: [PATCH] [vectorized](bug) fix some open enable_fold_constant_by_be failed cases (#17240) --- be/src/runtime/fold_constant_executor.cpp | 23 +++++----- be/src/runtime/fold_constant_executor.h | 4 +- be/src/vec/functions/function_utility.cpp | 2 +- .../org/apache/doris/analysis/InsertStmt.java | 5 ++- .../apache/doris/analysis/JsonLiteral.java | 2 +- .../org/apache/doris/analysis/QueryStmt.java | 5 ++- .../apache/doris/analysis/StatementBase.java | 3 +- .../org/apache/doris/qe/StmtExecutor.java | 2 +- .../apache/doris/rewrite/ExprRewriter.java | 6 ++- .../doris/rewrite/FoldConstantsRule.java | 43 ++++++++++++++++--- .../apache/doris/analysis/QueryStmtTest.java | 11 +++-- gensrc/thrift/PaloInternalService.thrift | 2 + .../correctness_p0/test_select_decimal.groovy | 8 ++-- 13 files changed, 78 insertions(+), 38 deletions(-) diff --git a/be/src/runtime/fold_constant_executor.cpp b/be/src/runtime/fold_constant_executor.cpp index bd28c35de9..1efd33238b 100644 --- a/be/src/runtime/fold_constant_executor.cpp +++ b/be/src/runtime/fold_constant_executor.cpp @@ -38,16 +38,15 @@ using std::map; namespace doris { -TUniqueId FoldConstantExecutor::_dummy_id; - Status FoldConstantExecutor::fold_constant_vexpr(const TFoldConstantParams& params, PConstantExprResult* response) { const auto& expr_map = params.expr_map; auto expr_result_map = response->mutable_expr_result_map(); TQueryGlobals query_globals = params.query_globals; + _query_id = params.query_id; // init - RETURN_IF_ERROR(_init(query_globals)); + RETURN_IF_ERROR(_init(query_globals, params.query_options)); // only after init operation, _mem_tracker is ready SCOPED_CONSUME_MEM_TRACKER(_mem_tracker.get()); @@ -71,10 +70,9 @@ Status FoldConstantExecutor::fold_constant_vexpr(const TFoldConstantParams& para // calc vexpr RETURN_IF_ERROR(ctx->execute(&tmp_block, &result_column)); DCHECK(result_column != -1); - PrimitiveType root_type = ctx->root()->type().type; // covert to thrift type - TPrimitiveType::type t_type = doris::to_thrift(root_type); - + const TypeDescriptor& res_type = ctx->root()->type(); + TPrimitiveType::type t_type = doris::to_thrift(res_type.type); // collect result PExprResult expr_result; string result; @@ -91,6 +89,9 @@ Status FoldConstantExecutor::fold_constant_vexpr(const TFoldConstantParams& para expr_result.set_content(std::move(result)); expr_result.mutable_type()->set_type(t_type); + expr_result.mutable_type()->set_scale(res_type.scale); + expr_result.mutable_type()->set_precision(res_type.precision); + expr_result.mutable_type()->set_len(res_type.len); pexpr_result_map.mutable_map()->insert({n.first, expr_result}); } expr_result_map->insert({m.first, pexpr_result_map}); @@ -99,15 +100,15 @@ Status FoldConstantExecutor::fold_constant_vexpr(const TFoldConstantParams& para return Status::OK(); } -Status FoldConstantExecutor::_init(const TQueryGlobals& query_globals) { +Status FoldConstantExecutor::_init(const TQueryGlobals& query_globals, + const TQueryOptions& query_options) { // init runtime state, runtime profile TPlanFragmentExecParams params; - params.fragment_instance_id = FoldConstantExecutor::_dummy_id; - params.query_id = FoldConstantExecutor::_dummy_id; + params.fragment_instance_id = _query_id; + params.query_id = _query_id; TExecPlanFragmentParams fragment_params; fragment_params.params = params; fragment_params.protocol_version = PaloInternalServiceVersion::V1; - TQueryOptions query_options; _runtime_state.reset(new RuntimeState(fragment_params.params, query_options, query_globals, ExecEnv::GetInstance())); DescriptorTbl* desc_tbl = nullptr; @@ -118,7 +119,7 @@ Status FoldConstantExecutor::_init(const TQueryGlobals& query_globals) { return status; } _runtime_state->set_desc_tbl(desc_tbl); - status = _runtime_state->init_mem_trackers(FoldConstantExecutor::_dummy_id); + status = _runtime_state->init_mem_trackers(_query_id); if (UNLIKELY(!status.ok())) { LOG(WARNING) << "Failed to init mem trackers, msg: " << status; return status; diff --git a/be/src/runtime/fold_constant_executor.h b/be/src/runtime/fold_constant_executor.h index 43628b6068..825a56de9d 100644 --- a/be/src/runtime/fold_constant_executor.h +++ b/be/src/runtime/fold_constant_executor.h @@ -41,7 +41,7 @@ public: private: // init runtime_state and mem_tracker - Status _init(const TQueryGlobals& query_globals); + Status _init(const TQueryGlobals& query_globals, const TQueryOptions& query_options); // prepare expr template Status _prepare_and_open(Context* ctx); @@ -55,6 +55,6 @@ private: std::unique_ptr _mem_tracker; RuntimeProfile* _runtime_profile = nullptr; ObjectPool _pool; - static TUniqueId _dummy_id; + TUniqueId _query_id; }; } // namespace doris diff --git a/be/src/vec/functions/function_utility.cpp b/be/src/vec/functions/function_utility.cpp index 9d529c9d01..d21f25bcf6 100644 --- a/be/src/vec/functions/function_utility.cpp +++ b/be/src/vec/functions/function_utility.cpp @@ -110,7 +110,7 @@ public: } }; -const std::string FunctionVersion::version = "5.1.0"; +const std::string FunctionVersion::version = "5.7.99"; void register_function_utility(SimpleFunctionFactory& factory) { factory.register_function(); diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/InsertStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/InsertStmt.java index 4b42d70b04..79173f42e8 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/InsertStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/InsertStmt.java @@ -51,6 +51,7 @@ import org.apache.doris.planner.OlapTableSink; import org.apache.doris.qe.ConnectContext; import org.apache.doris.rewrite.ExprRewriter; import org.apache.doris.service.FrontendOptions; +import org.apache.doris.thrift.TQueryOptions; import org.apache.doris.thrift.TUniqueId; import org.apache.doris.transaction.TransactionState; import org.apache.doris.transaction.TransactionState.LoadJobSourceType; @@ -226,9 +227,9 @@ public class InsertStmt extends DdlStmt { } @Override - public void foldConstant(ExprRewriter rewriter) throws AnalysisException { + public void foldConstant(ExprRewriter rewriter, TQueryOptions tQueryOptions) throws AnalysisException { Preconditions.checkState(isAnalyzed()); - queryStmt.foldConstant(rewriter); + queryStmt.foldConstant(rewriter, tQueryOptions); } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/JsonLiteral.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/JsonLiteral.java index d573d07985..1ef0cc72f4 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/JsonLiteral.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/JsonLiteral.java @@ -98,7 +98,7 @@ public class JsonLiteral extends LiteralExpr { @Override public String getStringValue() { - return null; + return value; } @Override diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/QueryStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/QueryStmt.java index 3564cb2927..b399477dec 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/QueryStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/QueryStmt.java @@ -28,6 +28,7 @@ import org.apache.doris.common.ErrorReport; import org.apache.doris.common.UserException; import org.apache.doris.common.util.VectorizedUtil; import org.apache.doris.rewrite.ExprRewriter; +import org.apache.doris.thrift.TQueryOptions; import com.google.common.base.Preconditions; import com.google.common.collect.Lists; @@ -517,11 +518,11 @@ public abstract class QueryStmt extends StatementBase implements Queriable { @Override - public void foldConstant(ExprRewriter rewriter) throws AnalysisException { + public void foldConstant(ExprRewriter rewriter, TQueryOptions tQueryOptions) throws AnalysisException { Preconditions.checkState(isAnalyzed()); Map exprMap = new HashMap<>(); collectExprs(exprMap); - rewriter.rewriteConstant(exprMap, analyzer); + rewriter.rewriteConstant(exprMap, analyzer, tQueryOptions); if (rewriter.changed()) { putBackExprs(exprMap); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/StatementBase.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/StatementBase.java index 66eecad529..aab18dc9f6 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/StatementBase.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/StatementBase.java @@ -27,6 +27,7 @@ import org.apache.doris.common.ErrorReport; import org.apache.doris.common.UserException; import org.apache.doris.qe.OriginStatement; import org.apache.doris.rewrite.ExprRewriter; +import org.apache.doris.thrift.TQueryOptions; import com.google.common.base.Preconditions; import com.google.common.base.Strings; @@ -197,7 +198,7 @@ public abstract class StatementBase implements ParseNode { * @throws AnalysisException * @param rewriter */ - public void foldConstant(ExprRewriter rewriter) throws AnalysisException { + public void foldConstant(ExprRewriter rewriter, TQueryOptions tQueryOptions) throws AnalysisException { throw new IllegalStateException( "foldConstant() not implemented for this stmt: " + getClass().getSimpleName()); } diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java b/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java index f09c62e03d..5c032c28db 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java +++ b/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java @@ -905,7 +905,7 @@ public class StmtExecutor implements ProfileWriter { rewriter.reset(); if (context.getSessionVariable().isEnableFoldConstantByBe()) { // fold constant expr - parsedStmt.foldConstant(rewriter); + parsedStmt.foldConstant(rewriter, tQueryOptions); } // Apply expr and subquery rewrites. diff --git a/fe/fe-core/src/main/java/org/apache/doris/rewrite/ExprRewriter.java b/fe/fe-core/src/main/java/org/apache/doris/rewrite/ExprRewriter.java index 43e9994a1e..04ec7d24e0 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/rewrite/ExprRewriter.java +++ b/fe/fe-core/src/main/java/org/apache/doris/rewrite/ExprRewriter.java @@ -26,6 +26,7 @@ import org.apache.doris.analysis.JoinOperator; import org.apache.doris.analysis.TupleId; import org.apache.doris.common.AnalysisException; import org.apache.doris.rewrite.mvrewrite.ExprToSlotRefRule; +import org.apache.doris.thrift.TQueryOptions; import com.google.common.collect.Lists; @@ -182,7 +183,8 @@ public class ExprRewriter { /** * FoldConstantsRule rewrite */ - public void rewriteConstant(Map exprMap, Analyzer analyzer) throws AnalysisException { + public void rewriteConstant(Map exprMap, Analyzer analyzer, TQueryOptions tQueryOptions) + throws AnalysisException { if (exprMap.isEmpty()) { return; } @@ -190,7 +192,7 @@ public class ExprRewriter { // rewrite constant expr for (ExprRewriteRule rule : rules) { if (rule instanceof FoldConstantsRule) { - changed = ((FoldConstantsRule) rule).apply(exprMap, analyzer, changed); + changed = ((FoldConstantsRule) rule).apply(exprMap, analyzer, changed, tQueryOptions); } } if (changed) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/rewrite/FoldConstantsRule.java b/fe/fe-core/src/main/java/org/apache/doris/rewrite/FoldConstantsRule.java index d989e31b55..08872008be 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/rewrite/FoldConstantsRule.java +++ b/fe/fe-core/src/main/java/org/apache/doris/rewrite/FoldConstantsRule.java @@ -32,12 +32,14 @@ import org.apache.doris.analysis.NullLiteral; import org.apache.doris.analysis.SysVariableDesc; import org.apache.doris.catalog.Env; import org.apache.doris.catalog.PrimitiveType; +import org.apache.doris.catalog.ScalarType; import org.apache.doris.catalog.Type; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.LoadException; import org.apache.doris.common.util.TimeUtils; import org.apache.doris.common.util.VectorizedUtil; import org.apache.doris.proto.InternalService; +import org.apache.doris.proto.Types.PScalarType; import org.apache.doris.qe.ConnectContext; import org.apache.doris.qe.VariableMgr; import org.apache.doris.rpc.BackendServiceProxy; @@ -47,6 +49,7 @@ import org.apache.doris.thrift.TFoldConstantParams; import org.apache.doris.thrift.TNetworkAddress; import org.apache.doris.thrift.TPrimitiveType; import org.apache.doris.thrift.TQueryGlobals; +import org.apache.doris.thrift.TQueryOptions; import com.google.common.base.Preconditions; import com.google.common.base.Predicates; @@ -133,7 +136,7 @@ public class FoldConstantsRule implements ExprRewriteRule { * @return * @throws AnalysisException */ - public boolean apply(Map exprMap, Analyzer analyzer, boolean changed) + public boolean apply(Map exprMap, Analyzer analyzer, boolean changed, TQueryOptions tQueryOptions) throws AnalysisException { // root_expr_id_string: // child_expr_id_string : texpr @@ -174,7 +177,8 @@ public class FoldConstantsRule implements ExprRewriteRule { } if (!paramMap.isEmpty()) { - Map> resultMap = calcConstExpr(paramMap, allConstMap, analyzer.getContext()); + Map> resultMap = calcConstExpr(paramMap, allConstMap, analyzer.getContext(), + tQueryOptions); if (!resultMap.isEmpty()) { putBackConstExpr(exprMap, resultMap); @@ -195,7 +199,7 @@ public class FoldConstantsRule implements ExprRewriteRule { */ // public only for unit test public void getConstExpr(Expr expr, Map constExprMap, Map oriConstMap, - Analyzer analyzer, Map sysVarMap, Map infoFnMap) + Analyzer analyzer, Map sysVarMap, Map infoFnMap) throws AnalysisException { if (expr.isConstant()) { // Do not constant fold cast(null as dataType) because we cannot preserve the @@ -338,8 +342,8 @@ public class FoldConstantsRule implements ExprRewriteRule { * @return */ private Map> calcConstExpr(Map> map, - Map allConstMap, - ConnectContext context) { + Map allConstMap, + ConnectContext context, TQueryOptions tQueryOptions) { TNetworkAddress brpcAddress = null; Map> resultMap = new HashMap<>(); try { @@ -364,6 +368,8 @@ public class FoldConstantsRule implements ExprRewriteRule { TFoldConstantParams tParams = new TFoldConstantParams(map, queryGlobals); tParams.setVecExec(VectorizedUtil.isVectorized()); + tParams.setQueryOptions(tQueryOptions); + tParams.setQueryId(context.queryId()); Future future = BackendServiceProxy.getInstance().foldConstantExpr(brpcAddress, tParams); @@ -375,14 +381,37 @@ public class FoldConstantsRule implements ExprRewriteRule { Map tmp = new HashMap<>(); for (Map.Entry entry1 : entry.getValue().getMapMap().entrySet()) { - TPrimitiveType type = TPrimitiveType.findByValue(entry1.getValue().getType().getType()); + PScalarType scalarType = entry1.getValue().getType(); + TPrimitiveType ttype = TPrimitiveType.findByValue(scalarType.getType()); Expr retExpr = null; if (entry1.getValue().getSuccess()) { + Type type = null; + if (ttype == TPrimitiveType.CHAR) { + Preconditions.checkState(scalarType.hasLen()); + type = ScalarType.createCharType(scalarType.getLen()); + } else if (ttype == TPrimitiveType.VARCHAR) { + Preconditions.checkState(scalarType.hasLen()); + type = ScalarType.createVarcharType(scalarType.getLen()); + } else if (ttype == TPrimitiveType.DECIMALV2) { + type = ScalarType.createDecimalType(scalarType.getPrecision(), + scalarType.getScale()); + } else if (ttype == TPrimitiveType.DATETIMEV2) { + type = ScalarType.createDatetimeV2Type(scalarType.getScale()); + } else if (ttype == TPrimitiveType.DECIMAL32 + || ttype == TPrimitiveType.DECIMAL64 + || ttype == TPrimitiveType.DECIMAL128I) { + type = ScalarType.createDecimalV3Type(scalarType.getPrecision(), + scalarType.getScale()); + } else { + type = ScalarType.createType( + PrimitiveType.fromThrift(ttype)); + } retExpr = LiteralExpr.create(entry1.getValue().getContent(), - Type.fromPrimitiveType(PrimitiveType.fromThrift(type))); + type); } else { retExpr = allConstMap.get(entry1.getKey()); } + LOG.debug("retExpr: " + retExpr.toString()); tmp.put(entry1.getKey(), retExpr); } if (!tmp.isEmpty()) { diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/QueryStmtTest.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/QueryStmtTest.java index 03acde255d..281b50eb43 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/analysis/QueryStmtTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/QueryStmtTest.java @@ -25,6 +25,7 @@ import org.apache.doris.qe.ConnectContext; import org.apache.doris.qe.SessionVariable; import org.apache.doris.rewrite.FoldConstantsRule; import org.apache.doris.thrift.TExpr; +import org.apache.doris.thrift.TQueryOptions; import org.apache.doris.utframe.DorisAssert; import org.apache.doris.utframe.UtFrameUtils; @@ -276,7 +277,9 @@ public class QueryStmtTest { + "FROM\n" + " (SELECT curdate()) a;"; StatementBase stmt = UtFrameUtils.parseAndAnalyzeStmt(sql, ctx); - stmt.foldConstant(new Analyzer(ctx.getEnv(), ctx).getExprRewriter()); + SessionVariable sessionVariable = new SessionVariable(); + TQueryOptions queryOptions = sessionVariable.getQueryOptionVariables(); + stmt.foldConstant(new Analyzer(ctx.getEnv(), ctx).getExprRewriter(), queryOptions); // reAnalyze reAnalyze(stmt, ctx); @@ -298,7 +301,7 @@ public class QueryStmtTest { + " t2.k2 = @@language\n" + ")"; stmt = UtFrameUtils.parseAndAnalyzeStmt(sql, ctx); - stmt.foldConstant(new Analyzer(ctx.getEnv(), ctx).getExprRewriter()); + stmt.foldConstant(new Analyzer(ctx.getEnv(), ctx).getExprRewriter(), queryOptions); // reAnalyze reAnalyze(stmt, ctx); Assert.assertTrue(stmt.toSql().contains("Apache License, Version 2.0")); @@ -319,7 +322,7 @@ public class QueryStmtTest { + " t2.k2 = CONNECTION_ID()\n" + ")"; stmt = UtFrameUtils.parseAndAnalyzeStmt(sql, ctx); - stmt.foldConstant(new Analyzer(ctx.getEnv(), ctx).getExprRewriter()); + stmt.foldConstant(new Analyzer(ctx.getEnv(), ctx).getExprRewriter(), queryOptions); // reAnalyze reAnalyze(stmt, ctx); Assert.assertTrue(stmt.toSql().contains("root''@''%")); @@ -332,7 +335,7 @@ public class QueryStmtTest { + " (select USER() k1, CURRENT_USER() k2, SCHEMA() k3) t1,\n" + " (select @@license k1, @@version k2) t2\n"; stmt = UtFrameUtils.parseAndAnalyzeStmt(sql, ctx); - stmt.foldConstant(new Analyzer(ctx.getEnv(), ctx).getExprRewriter()); + stmt.foldConstant(new Analyzer(ctx.getEnv(), ctx).getExprRewriter(), queryOptions); // reAnalyze reAnalyze(stmt, ctx); Assert.assertTrue(stmt.toSql().contains("root''@''%")); diff --git a/gensrc/thrift/PaloInternalService.thrift b/gensrc/thrift/PaloInternalService.thrift index d49bb72f3a..578701004e 100644 --- a/gensrc/thrift/PaloInternalService.thrift +++ b/gensrc/thrift/PaloInternalService.thrift @@ -429,6 +429,8 @@ struct TFoldConstantParams { 1: required map> expr_map 2: required TQueryGlobals query_globals 3: optional bool vec_exec + 4: optional TQueryOptions query_options + 5: optional Types.TUniqueId query_id } // TransmitData diff --git a/regression-test/suites/correctness_p0/test_select_decimal.groovy b/regression-test/suites/correctness_p0/test_select_decimal.groovy index 3ad7be9f64..c79bb478ef 100644 --- a/regression-test/suites/correctness_p0/test_select_decimal.groovy +++ b/regression-test/suites/correctness_p0/test_select_decimal.groovy @@ -16,8 +16,8 @@ // under the License. suite("test_select_decimal") { - qt_select1 'select cast(19888.8999999 as decimal(12,2));' - qt_select2 'select cast(19888.9999999 as decimal(12,2));' - qt_select2 'select cast(19888.666 as decimal(12,2));' - qt_select2 'select cast(19888.444 as decimal(12,2));' + qt_select1 'select cast(19888.8999999 as decimalv3(12,2));' + qt_select2 'select cast(19888.9999999 as decimalv3(12,2));' + qt_select2 'select cast(19888.666 as decimalv3(12,2));' + qt_select2 'select cast(19888.444 as decimalv3(12,2));' }