xiaokang commented on code in PR #34247: URL: https://github.com/apache/doris/pull/34247#discussion_r1588661364
########## fe/fe-core/src/main/java/org/apache/doris/analysis/LiteralExpr.java: ########## @@ -438,4 +430,12 @@ public static LiteralExpr getLiteralExprFromThrift(TExprNode node) throws Analys default: throw new AnalysisException("Wrong type from thrift;"); } } + + // Parse from binary data, the format follows mysql binary protocal + // see https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_binary_resultset.html. + // Return next offset + public void setupParamFromBinary(ByteBuffer data) { Review Comment: why change position? ########## fe/fe-core/src/main/java/org/apache/doris/analysis/PrepareStmt.java: ########## @@ -17,6 +17,7 @@ package org.apache.doris.analysis; +// import org.apache.doris.catalog.Column; Review Comment: just delete ########## fe/fe-core/src/main/java/org/apache/doris/mysql/MysqlProto.java: ########## @@ -200,7 +200,6 @@ public static boolean negotiate(ConnectContext context) throws IOException { } if (handshakeResponse == null) { - // receive response failed. Review Comment: why delete comment ########## fe/fe-core/src/main/java/org/apache/doris/qe/PointQueryExec.java: ########## @@ -113,7 +113,7 @@ public PointQueryExec(Planner planner, Analyzer analyzer, int maxMessageSize) { this.cacheID = prepareStmt.getID(); this.serializedDescTable = prepareStmt.getSerializedDescTable(); this.serializedOutputExpr = prepareStmt.getSerializedOutputExprs(); - this.isBinaryProtocol = prepareStmt.isBinaryProtocol(); + this.isBinaryProtocol = prepareStmt.isPointQueryShortCircuit(); Review Comment: The change is confusing. ########## fe/fe-core/src/main/java/org/apache/doris/catalog/OlapTable.java: ########## @@ -1980,6 +1980,12 @@ public Boolean disableAutoCompaction() { return false; } + public int getBaseSchemaVersion() { + MaterializedIndexMeta baseIndexMeta = indexIdToMeta.get(baseIndexId); + return baseIndexMeta.getSchemaVersion(); + } Review Comment: why change position? ########## fe/fe-core/src/main/java/org/apache/doris/qe/PrepareStmtContext.java: ########## @@ -23,8 +23,11 @@ import org.apache.doris.planner.Planner; import com.google.common.base.Preconditions; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; public class PrepareStmtContext { + private static final Logger LOG = LogManager.getLogger(PrepareStmtContext.class); Review Comment: It's not used ########## fe/fe-core/src/main/java/org/apache/doris/qe/Coordinator.java: ########## @@ -373,6 +374,8 @@ private void initQueryOptions(ConnectContext context) { this.queryOptions.setQueryTimeout(context.getExecTimeout()); this.queryOptions.setExecutionTimeout(context.getExecTimeout()); this.queryOptions.setEnableScanNodeRunSerial(context.getSessionVariable().isEnableScanRunSerial()); + this.queryOptions.setMysqlRowBinaryFormat( Review Comment: Why ########## fe/fe-core/src/main/java/org/apache/doris/analysis/BinaryPredicate.java: ########## @@ -483,11 +483,13 @@ public Pair<SlotRef, Expr> extract() { public void analyzeImpl(Analyzer analyzer) throws AnalysisException { super.analyzeImpl(analyzer); this.checkIncludeBitmap(); - // Ignore placeholder - if (getChild(0) instanceof PlaceHolderExpr || getChild(1) instanceof PlaceHolderExpr) { + // Ignore placeholder, when it type is invalid. + // Invalid type could happen when analyze prepared point query select statement, + // since the value is occupied but not assigned + if ((getChild(0) instanceof PlaceHolderExpr && getChild(0).type == Type.UNSUPPORTED) Review Comment: Do you mean add setting type of placeholder to UNSUPPORTED to represent normal prepared statement and INVALID for point query? If yes, it's somewhat tricy. ########## fe/fe-core/src/main/java/org/apache/doris/analysis/StringLiteral.java: ########## @@ -311,5 +311,6 @@ public void setupParamFromBinary(ByteBuffer data) { if (LOG.isDebugEnabled()) { LOG.debug("parsed value '{}'", value); } + type = Type.VARCHAR; Review Comment: add comment to explain -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org