From d2ea7485a856ddeb3256a50722b372c2996aaabc Mon Sep 17 00:00:00 2001 From: eldenmoon <15605149486@163.com> Date: Mon, 26 Aug 2024 18:19:54 +0800 Subject: [PATCH] [Improve](PreparedStatement) avoid record prepared statement audit log when prepared failed --- .../apache/doris/analysis/PrepareStmt.java | 10 +++--- .../common/NotSupportPreparedStmtError.java | 31 +++++++++++++++++++ .../org/apache/doris/qe/ConnectProcessor.java | 7 +++++ .../org/apache/doris/qe/StmtExecutor.java | 4 +++ 4 files changed, 47 insertions(+), 5 deletions(-) create mode 100644 fe/fe-core/src/main/java/org/apache/doris/common/NotSupportPreparedStmtError.java diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/PrepareStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/PrepareStmt.java index 8afc933aedf379c..27c07fac896bcf7 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/PrepareStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/PrepareStmt.java @@ -18,6 +18,7 @@ package org.apache.doris.analysis; import org.apache.doris.catalog.OlapTable; +import org.apache.doris.common.NotSupportPreparedStmtError; import org.apache.doris.common.UserException; import org.apache.doris.qe.ConnectContext; import org.apache.doris.thrift.TDescriptorTable; @@ -162,7 +163,7 @@ public ByteString getSerializedQueryOptions() { public void analyze(Analyzer analyzer) throws UserException { // TODO support more Statement if (!(inner instanceof SelectStmt) && !(inner instanceof NativeInsertStmt)) { - throw new UserException("Only support prepare SelectStmt or NativeInsertStmt"); + throw new NotSupportPreparedStmtError("Only support prepare SelectStmt or NativeInsertStmt"); } analyzer.setPrepareStmt(this); if (inner instanceof SelectStmt) { @@ -190,12 +191,12 @@ public void analyze(Analyzer analyzer) throws UserException { // use session var to decide whether to use full prepared or let user client handle to do fail over if (preparedType != PreparedType.FULL_PREPARED && !ConnectContext.get().getSessionVariable().enableServeSidePreparedStatement) { - throw new UserException("Failed to prepare statement"); + throw new NotSupportPreparedStmtError("Failed to prepare statement, not supported"); } } else if (inner instanceof NativeInsertStmt) { LabelName label = ((NativeInsertStmt) inner).getLoadLabel(); if (label != null && !Strings.isNullOrEmpty(label.getLabelName())) { - throw new UserException("Only support prepare InsertStmt without label now"); + throw new NotSupportPreparedStmtError("Only support prepare InsertStmt without label now"); } } preparedType = PreparedType.STATEMENT; @@ -262,12 +263,11 @@ public int argsSize() { public void asignValues(List values) throws UserException { if (values.size() != inner.getPlaceHolders().size()) { - throw new UserException("Invalid arguments size " + throw new NotSupportPreparedStmtError("Invalid arguments size " + values.size() + ", expected " + inner.getPlaceHolders().size()); } for (int i = 0; i < values.size(); ++i) { inner.getPlaceHolders().get(i).setLiteral(values.get(i)); - inner.getPlaceHolders().get(i).analysisDone(); } if (!values.isEmpty()) { LOG.debug("assign values {}", values.get(0).toSql()); diff --git a/fe/fe-core/src/main/java/org/apache/doris/common/NotSupportPreparedStmtError.java b/fe/fe-core/src/main/java/org/apache/doris/common/NotSupportPreparedStmtError.java new file mode 100644 index 000000000000000..67c39f2a8b0925e --- /dev/null +++ b/fe/fe-core/src/main/java/org/apache/doris/common/NotSupportPreparedStmtError.java @@ -0,0 +1,31 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.common; + +import com.google.common.base.Strings; + +/** + * Thrown for unhandled prepared statement. + */ +public class NotSupportPreparedStmtError extends UserException { + + public NotSupportPreparedStmtError(String msg) { + super(Strings.nullToEmpty(msg)); + setMysqlErrorCode(ErrorCode.ERR_UNSUPPORTED_PS); + } +} diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java b/fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java index 93e58709e4cbe03..171a1f5c23d8018 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java +++ b/fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java @@ -36,6 +36,7 @@ import org.apache.doris.common.DdlException; import org.apache.doris.common.ErrorCode; import org.apache.doris.common.ErrorReport; +import org.apache.doris.common.NotSupportPreparedStmtError; import org.apache.doris.common.UserException; import org.apache.doris.common.telemetry.Telemetry; import org.apache.doris.common.util.DebugUtil; @@ -377,6 +378,12 @@ private void handleQueryException(Throwable throwable, String origStmt, if (ctx.getMinidump() != null) { MinidumpUtils.saveMinidumpString(ctx.getMinidump(), DebugUtil.printId(ctx.queryId())); } + if (throwable instanceof NotSupportPreparedStmtError) { + LOG.info("Process one query failed because NotSupportPreparedStmtError: ", throwable); + ctx.getState().setError(ErrorCode.ERR_UNSUPPORTED_PS, throwable.getMessage()); + // Not audit such error, since we don't support such prepared statement + return; + } if (throwable instanceof IOException) { // Client failed. LOG.warn("Process one query failed because IOException: ", throwable); 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 02450f424c0219d..8d56271ac4fd163 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 @@ -98,6 +98,7 @@ import org.apache.doris.common.FeConstants; import org.apache.doris.common.MetaNotFoundException; import org.apache.doris.common.NereidsException; +import org.apache.doris.common.NotSupportPreparedStmtError; import org.apache.doris.common.UserException; import org.apache.doris.common.Version; import org.apache.doris.common.profile.Profile; @@ -874,6 +875,9 @@ public void executeByLegacy(TUniqueId queryId) throws Exception { // this exception shows the connection is gone context.getState().setError(ErrorCode.ERR_UNKNOWN_ERROR, e.getMessage()); throw e; + } catch (NotSupportPreparedStmtError e) { + // just throw + throw e; } catch (UserException e) { // analysis exception only print message, not print the stack LOG.warn("execute Exception. {}", context.getQueryIdentifier(), e);