xiaokang commented on code in PR #15491:
URL: https://github.com/apache/doris/pull/15491#discussion_r1070872042


##########
fe/fe-core/src/main/cup/sql_parser.cup:
##########
@@ -6113,6 +6160,8 @@ literal ::=
   {: RESULT = new BoolLiteral(false); :}
   | KW_NULL
   {: RESULT = new NullLiteral(); :}
+  | PLACEHOLDER

Review Comment:
   what's the usage of PLACEHOLDER



##########
gensrc/proto/internal_service.proto:
##########
@@ -232,6 +232,38 @@ message PFetchDataResult {
     optional bool empty_batch = 6;
 };
 
+message KeyTuple {
+    repeated string key_column_rep = 1;
+}
+
+message UUID {
+    required int64 uuid_high = 1;
+    required int64 uuid_low = 2;
+}
+
+// We use thrift definition for some structure, since TExpr,
+// list<Exprs.TExpr>, Descriptors.TDescriptorTable are all thrift format.
+// Modify them to protobuf is a redundant work.
+message PTabletKeyLookupRequest {
+    required int64 tablet_id = 1;
+    repeated KeyTuple key_tuples = 2;
+
+    // reusable structures
+    // serilized from Descriptors.TDescriptorTable
+    optional UUID uuid = 3;
+    optional bytes desc_tbl = 4;
+    // serilized from TExprList 
+    optional bytes output_expr = 5;
+    // return binary mysql row format if true
+    optional bool is_binary_row = 6;
+}
+
+message PTabletKeyLookupResponse {
+    required PStatus status = 1;
+    optional bytes row_batch = 5;

Review Comment:
   sequence 5 is strange



##########
gensrc/proto/internal_service.proto:
##########
@@ -232,6 +232,38 @@ message PFetchDataResult {
     optional bool empty_batch = 6;
 };
 
+message KeyTuple {
+    repeated string key_column_rep = 1;
+}
+
+message UUID {
+    required int64 uuid_high = 1;
+    required int64 uuid_low = 2;
+}
+
+// We use thrift definition for some structure, since TExpr,
+// list<Exprs.TExpr>, Descriptors.TDescriptorTable are all thrift format.
+// Modify them to protobuf is a redundant work.
+message PTabletKeyLookupRequest {
+    required int64 tablet_id = 1;
+    repeated KeyTuple key_tuples = 2;
+
+    // reusable structures
+    // serilized from Descriptors.TDescriptorTable
+    optional UUID uuid = 3;
+    optional bytes desc_tbl = 4;

Review Comment:
   why use bytes instead of real data structure for desc_tbl and output_expr?



##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -1648,10 +1721,37 @@ private void sendMetaData(ResultSetMetaData metaData) 
throws IOException {
         context.getMysqlChannel().sendOnePacket(serializer.toByteBuffer());
     }
 
+    private void sendStmtPrepareOK() throws IOException {
+        // 
https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_com_stmt_prepare.html#sect_protocol_com_stmt_prepare_response
+        serializer.reset();
+        // 0x00 OK
+        serializer.writeInt1(0);
+        // statement_id
+        serializer.writeInt4(Integer.valueOf(prepareStmt.getName()));

Review Comment:
   Is name guaranteed to be an int string?



##########
fe/fe-core/src/main/java/org/apache/doris/planner/OriginalPlanner.java:
##########
@@ -338,7 +339,6 @@ private void pushSortToOlapScan() {
             PlanNode node = fragment.getPlanRoot();
             PlanNode parent = null;
 
-            // OlapScanNode is the last node.

Review Comment:
   why delete this comment?



##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Env.java:
##########
@@ -3308,6 +3317,11 @@ public long getNextId() {
         return idGenerator.getNextId();
     }
 
+    // counter for prepared statement id
+    public long getNextStmtId() {
+        return this.stmtIdCounter.getAndIncrement();

Review Comment:
   Is stmtIdCounter reset to zero on fe restart?



##########
fe/fe-core/src/main/java/org/apache/doris/analysis/LiteralExpr.java:
##########
@@ -262,4 +262,106 @@ public boolean isNullable() {
     public void finalizeImplForNereids() throws AnalysisException {
 
     }
+
+    // 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) {
+        Preconditions.checkState(false,
+                "should implement this in derived class. " + 
this.type.toSql());
+    }
+
+    public static LiteralExpr getLiteralByMysqlType(int mysqlType) throws 
AnalysisException {
+        switch (mysqlType) {
+            // MYSQL_TYPE_TINY
+            case 1:
+                return LiteralExpr.create("0", Type.TINYINT);
+            // MYSQL_TYPE_SHORT
+            case 2:
+                return LiteralExpr.create("0", Type.SMALLINT);
+            // MYSQL_TYPE_LONG
+            case 3:
+                return LiteralExpr.create("0", Type.INT);
+            // MYSQL_TYPE_LONGLONG
+            case 8:
+                return LiteralExpr.create("0", Type.BIGINT);
+            // MYSQL_TYPE_FLOAT
+            case 4:
+                return LiteralExpr.create("0", Type.FLOAT);
+            // MYSQL_TYPE_DOUBLE
+            case 5:
+                return LiteralExpr.create("0", Type.DOUBLE);
+            // MYSQL_TYPE_DECIMAL
+            case 0:
+            // MYSQL_TYPE_NEWDECIMAL
+            case 246:
+                return LiteralExpr.create("0", Type.DECIMAL32);
+            // MYSQL_TYPE_TIME
+            case 11:
+                return LiteralExpr.create("", Type.TIME);
+            // MYSQL_TYPE_DATE
+            case 10:
+                return LiteralExpr.create("1970-01-01", Type.DATE);

Review Comment:
   DATE or DATEV2 ?



##########
fe/fe-core/src/main/java/org/apache/doris/analysis/DateLiteral.java:
##########
@@ -1658,4 +1658,49 @@ public void setMinValue() {
         second = 0;
         microsecond = 0;
     }
+
+    @Override
+    public void setupParamFromBinary(ByteBuffer data) {
+        int len = getParmLen(data);
+        if (type.getPrimitiveType() == PrimitiveType.DATE) {
+            if (len >= 4) {
+                year = (int) data.getChar();
+                month = (int) data.get();
+                day = (int) data.get();
+                hour = 0;
+                minute = 0;
+                second = 0;
+                microsecond = 0;
+            } else {
+                copy(MIN_DATE);
+            }
+            return;
+        }
+        if (type.getPrimitiveType() == PrimitiveType.DATETIME) {

Review Comment:
   Is DATEV2 and DATETIMEV2 need to be processed?



##########
fe/fe-core/src/main/java/org/apache/doris/qe/ConnectContext.java:
##########
@@ -155,6 +157,7 @@ public void setUserQueryTimeout(long queryTimeout) {
     }
 
     private StatementContext statementContext;
+    private Map<String, PrepareStmtContext> preparedStmtCtxs = 
Maps.newHashMap();

Review Comment:
   If key shoud be a int string, using Integer as key is better.



##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -853,10 +908,13 @@ private void analyzeAndGenerateQueryPlan(TQueryOptions 
tQueryOptions) throws Use
                 }
                 List<String> origColLabels =
                         Lists.newArrayList(parsedStmt.getColLabels());
-
                 // Re-analyze the stmt with a new analyzer.
                 analyzer = new Analyzer(context.getEnv(), context);
 
+                if (prepareStmt != null) {

Review Comment:
   add comment



##########
gensrc/proto/internal_service.proto:
##########
@@ -232,6 +232,38 @@ message PFetchDataResult {
     optional bool empty_batch = 6;
 };
 
+message KeyTuple {
+    repeated string key_column_rep = 1;
+}
+
+message UUID {
+    required int64 uuid_high = 1;
+    required int64 uuid_low = 2;
+}
+
+// We use thrift definition for some structure, since TExpr,
+// list<Exprs.TExpr>, Descriptors.TDescriptorTable are all thrift format.
+// Modify them to protobuf is a redundant work.
+message PTabletKeyLookupRequest {
+    required int64 tablet_id = 1;
+    repeated KeyTuple key_tuples = 2;
+
+    // reusable structures
+    // serilized from Descriptors.TDescriptorTable
+    optional UUID uuid = 3;
+    optional bytes desc_tbl = 4;
+    // serilized from TExprList 
+    optional bytes output_expr = 5;

Review Comment:
   Is output_expr for two purpose, expr for produce output and  type of output 
columns?



##########
fe/fe-core/src/main/java/org/apache/doris/qe/PointQueryExec.java:
##########
@@ -0,0 +1,221 @@
+// 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.qe;
+
+import org.apache.doris.analysis.DescriptorTable;
+import org.apache.doris.analysis.Expr;
+import org.apache.doris.analysis.LiteralExpr;
+import org.apache.doris.analysis.SlotRef;
+import org.apache.doris.common.Status;
+import org.apache.doris.proto.InternalService;
+import org.apache.doris.proto.InternalService.KeyTuple;
+import org.apache.doris.rpc.BackendServiceProxy;
+import org.apache.doris.rpc.RpcException;
+import org.apache.doris.thrift.TExpr;
+import org.apache.doris.thrift.TExprList;
+import org.apache.doris.thrift.TNetworkAddress;
+import org.apache.doris.thrift.TResultBatch;
+import org.apache.doris.thrift.TStatusCode;
+
+import com.google.protobuf.ByteString;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.apache.thrift.TDeserializer;
+import org.apache.thrift.TException;
+import org.apache.thrift.TSerializer;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Map;
+import java.util.UUID;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.TimeoutException;
+
+public class PointQueryExec {
+    private static final Logger LOG = 
LogManager.getLogger(PointQueryExec.class);
+    private TNetworkAddress address;
+    // SlotRef sorted by column id
+    private Map<SlotRef, Expr> equalPredicats;
+    // ByteString serialized for prepared statement
+    private ByteString serializedDescTable;
+    private ByteString serializedOutputExpr;
+    private ArrayList<Expr> outputExprs;
+    private DescriptorTable descriptorTable;
+    private long tabletID = 0;
+    private long timeoutMs = 1000; // default 1s
+
+    private boolean isCancel = false;
+    private boolean isBinaryProtocol = false;
+    private long backendID;
+    // For parepared statement cached structure,
+    // there are some pre caculated structure in Backend TabletFetch service
+    // using this ID to find for this prepared statement
+    private UUID cacheID;
+
+    public PointQueryExec(Map<SlotRef, Expr> equalPredicats, DescriptorTable 
descTable,
+                    ArrayList<Expr> outputExprs) {
+        this.equalPredicats = equalPredicats;
+        this.descriptorTable = descTable;
+        this.outputExprs = outputExprs;
+    }
+
+    void setAddressAndBackendID(TNetworkAddress addr, long backendID) {
+        this.address = addr;
+        this.backendID = backendID;
+    }
+
+    public void setSerializedDescTable(ByteString serializedDescTable) {
+        this.serializedDescTable = serializedDescTable;
+    }
+
+    public void setSerializedOutputExpr(ByteString serializedOutputExpr) {
+        this.serializedOutputExpr = serializedOutputExpr;
+    }
+
+    public void setCacheID(UUID cacheID) {
+        this.cacheID = cacheID;
+    }
+
+    public void setTabletId(long tabletID) {
+        this.tabletID = tabletID;
+    }
+
+    public void setTimeout(long timeoutMs) {
+        this.timeoutMs = timeoutMs;
+    }
+
+    public void setBinaryProtocol(boolean isBinaryProtocol) {
+        this.isBinaryProtocol = isBinaryProtocol;
+    }
+
+    void addKeyTuples(
+                InternalService.PTabletKeyLookupRequest.Builder 
requestBuilder) {
+        // TODO handle IN predicates
+        KeyTuple.Builder kBuilder = KeyTuple.newBuilder();
+        for (Expr expr : equalPredicats.values()) {
+            LiteralExpr lexpr = (LiteralExpr) expr;
+            kBuilder.addKeyColumnRep(lexpr.getStringValue());
+        }
+        requestBuilder.addKeyTuples(kBuilder);
+    }
+
+    public RowBatch getNext(Status status) throws TException {
+        long timeoutTs = System.currentTimeMillis() + timeoutMs;
+        RowBatch rowBatch = new RowBatch();
+        InternalService.PTabletKeyLookupResponse pResult = null;
+        try {
+            if (serializedDescTable == null) {

Review Comment:
   Is serialization repeteated for every getNext call?



##########
fe/fe-core/src/main/java/org/apache/doris/analysis/PlaceHolderExpr.java:
##########
@@ -0,0 +1,180 @@
+// 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.analysis;
+
+import org.apache.doris.catalog.PrimitiveType;
+import org.apache.doris.catalog.Type;
+import org.apache.doris.common.AnalysisException;
+import org.apache.doris.common.NotImplementedException;
+import org.apache.doris.thrift.TExprNode;
+
+import com.google.common.base.Preconditions;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.io.DataInput;
+import java.io.DataOutput;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+
+// PlaceHolderExpr is a reference class point to real LiteralExpr
+public class PlaceHolderExpr extends LiteralExpr {
+    private static final Logger LOG = LogManager.getLogger(LiteralExpr.class);
+    private LiteralExpr lExpr;
+    int mysqlTypeCode = -1;
+
+    public PlaceHolderExpr() {
+
+    }
+
+    public void setTypeCode(int mysqlTypeCode) {
+        this.mysqlTypeCode = mysqlTypeCode;
+    }
+
+    protected PlaceHolderExpr(LiteralExpr literal) {
+        this.lExpr = literal;
+    }
+
+    protected PlaceHolderExpr(PlaceHolderExpr other) {
+        this.lExpr = other.lExpr;
+    }
+
+    public void setLiteral(LiteralExpr literal) {
+        this.lExpr = literal;
+        this.type = literal.getType();
+    }
+
+    public LiteralExpr createLiteralFromType() throws AnalysisException {
+        Preconditions.checkState(mysqlTypeCode > 0);
+        return LiteralExpr.getLiteralByMysqlType(mysqlTypeCode);
+    }
+
+    public static PlaceHolderExpr create(String value, Type type) throws 
AnalysisException {
+        Preconditions.checkArgument(!type.equals(Type.INVALID));
+        return new PlaceHolderExpr(LiteralExpr.create(value, type));
+    }
+
+    @Override
+    protected void toThrift(TExprNode msg) {
+        lExpr.toThrift(msg);
+    }
+
+    /*
+     * return real value
+     */
+    public Object getRealValue() {
+        // implemented: 
TINYINT/SMALLINT/INT/BIGINT/LARGEINT/DATE/DATETIME/CHAR/VARCHAR/BOOLEAN
+        Preconditions.checkState(false, "not implement this in derived class. 
" + this.type.toSql());

Review Comment:
   always false?



##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -1648,10 +1721,37 @@ private void sendMetaData(ResultSetMetaData metaData) 
throws IOException {
         context.getMysqlChannel().sendOnePacket(serializer.toByteBuffer());
     }
 
+    private void sendStmtPrepareOK() throws IOException {
+        // 
https://dev.mysql.com/doc/dev/mysql-server/latest/page_protocol_com_stmt_prepare.html#sect_protocol_com_stmt_prepare_response
+        serializer.reset();
+        // 0x00 OK
+        serializer.writeInt1(0);
+        // statement_id
+        serializer.writeInt4(Integer.valueOf(prepareStmt.getName()));
+        // num_columns
+        int numColumns = 0;
+        serializer.writeInt2(numColumns);
+        // num_params
+        int numParams = prepareStmt.getColLabelsOfPlaceHolders().size();
+        serializer.writeInt2(numParams);
+        // reserved_1
+        // serializer.writeInt1(0);

Review Comment:
   useless code?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to