morrySnow commented on code in PR #30817:
URL: https://github.com/apache/doris/pull/30817#discussion_r1477756798


##########
fe/fe-core/pom.xml:
##########
@@ -887,7 +887,7 @@ under the License.
                 <configuration>
                     <visitor>true</visitor>
                     <sourceDirectory>src/main/antlr4</sourceDirectory>
-                    <treatWarningsAsErrors>true</treatWarningsAsErrors>

Review Comment:
   why remove this? it is danger



##########
fe/fe-core/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4:
##########
@@ -34,6 +34,12 @@ singleStatement
     ;
 
 statement
+    : statementBase # statementBaseAlias
+    | CALL functionName=identifier LEFT_PAREN (expression (COMMA 
expression)*)? RIGHT_PAREN #callProcedure
+    | (ALTER | CREATE (OR REPLACE)? | REPLACE)? (PROCEDURE | PROC) identifier 
LEFT_PAREN .*? RIGHT_PAREN .*? #createProcedure

Review Comment:
    we need `AS` before `.*?` after `RIGHT_PAREN`?



##########
fe/fe-core/pom.xml:
##########
@@ -635,6 +635,10 @@ under the License.
             <groupId>org.mariadb.jdbc</groupId>
             <artifactId>mariadb-java-client</artifactId>
         </dependency>
+        <dependency>

Review Comment:
   GPL license



##########
fe/fe-core/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4:
##########
@@ -34,6 +34,12 @@ singleStatement
     ;
 
 statement
+    : statementBase # statementBaseAlias
+    | CALL functionName=identifier LEFT_PAREN (expression (COMMA 
expression)*)? RIGHT_PAREN #callProcedure

Review Comment:
   only could call proc under current database?



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java:
##########
@@ -3250,7 +3252,17 @@ public Object visitCallProcedure(CallProcedureContext 
ctx) {
         List<Expression> arguments = ctx.expression().stream()
                 .<Expression>map(this::typedVisit)
                 .collect(ImmutableList.toImmutableList());
-        UnboundFunction unboundFunction = new UnboundFunction(functionName, 
arguments);
+        UnboundFunction unboundFunction = new UnboundFunction(functionName, 
arguments, getOriginSql(ctx));

Review Comment:
   maybe call procedure should not be wrap as a function



##########
fe/fe-core/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4:
##########
@@ -34,6 +34,12 @@ singleStatement
     ;
 
 statement
+    : statementBase # statementBaseAlias
+    | CALL functionName=identifier LEFT_PAREN (expression (COMMA 
expression)*)? RIGHT_PAREN #callProcedure
+    | (ALTER | CREATE (OR REPLACE)? | REPLACE)? (PROCEDURE | PROC) identifier 
LEFT_PAREN .*? RIGHT_PAREN .*? #createProcedure

Review Comment:
   since only use identifier instead of multi-identifier, we can only create 
proc under current database?



##########
fe/fe-common/src/main/java/org/apache/doris/catalog/MysqlColType.java:
##########
@@ -21,53 +21,64 @@
 // TYPE codes are defined in the file 'mysql/include/mysql_com.h' enum 
enum_field_types
 // which is also demostrated in
 // http://dev.mysql.com/doc/internals/en/com-query-response.html
+// Name from 
https://dev.mysql.com/doc/connector-j/8.0/en/connector-j-reference-type-conversions.html
+// In plsql/Var.defineType(), Plsql Var type will be found through the Mysql 
type name string.
+// TODO, supports the correspondence between Doris type and Plsql Var.
 public enum MysqlColType {
-    MYSQL_TYPE_DECIMAL(0, "DECIMAL"),
-    MYSQL_TYPE_TINY(1, "TINY INT"),
-    MYSQL_TYPE_SHORT(2, "SMALL INT"),
-    MYSQL_TYPE_LONG(3, "INT"),
-    MYSQL_TYPE_FLOAT(4, "FLOAT"),
-    MYSQL_TYPE_DOUBLE(5, "DOUBLE"),
-    MYSQL_TYPE_NULL(6, "NULL"),
-    MYSQL_TYPE_TIMESTAMP(7, "TIMESTAMP"),
-    MYSQL_TYPE_LONGLONG(8, "LONGLONG"),
-    MYSQL_TYPE_INT24(9, "INT24"),
-    MYSQL_TYPE_DATE(10, "DATE"),
-    MYSQL_TYPE_TIME(11, "TIME"),
-    MYSQL_TYPE_DATETIME(12, "DATETIME"),
-    MYSQL_TYPE_YEAR(13, "YEAR"),
-    MYSQL_TYPE_NEWDATE(14, "NEWDATE"),
-    MYSQL_TYPE_VARCHAR(15, "VARCHAR"),
-    MYSQL_TYPE_BIT(16, "BIT"),
-    MYSQL_TYPE_TIMESTAMP2(17, "TIMESTAMP2"),
-    MYSQL_TYPE_DATETIME2(18, "DATETIME2"),
-    MYSQL_TYPE_TIME2(19, "TIME2"),
-    MYSQL_TYPE_JSON(245, "JSON"),
-    MYSQL_TYPE_NEWDECIMAL(246, "NEW DECIMAL"),
-    MYSQL_TYPE_ENUM(247, "ENUM"),
-    MYSQL_TYPE_SET(248, "SET"),
-    MYSQL_TYPE_TINY_BLOB(249, "TINY BLOB"),
-    MYSQL_TYPE_MEDIUM_BLOB(250, "MEDIUM BLOB"),
-    MYSQL_TYPE_LONG_BLOB(251, "LONG BLOB"),
-    MYSQL_TYPE_BLOB(252, "BLOB"),
-    MYSQL_TYPE_VARSTRING(253, "VAR STRING"),
-    MYSQL_TYPE_STRING(254, "STRING"),
-    MYSQL_TYPE_GEOMETRY(255, "GEOMETRY"),
-    MYSQL_TYPE_MAP(400, "MAP");
+    MYSQL_TYPE_DECIMAL(0, "DECIMAL", "DECIMAL"),
+    MYSQL_TYPE_TINY(1, "TINYINT", "TINY INT"),
+    MYSQL_TYPE_SHORT(2, "SMALLINT", "SMALL INT"),
+    MYSQL_TYPE_LONG(3, "INTEGER", "INT"),
+    MYSQL_TYPE_FLOAT(4, "FLOAT", "FLOAT"),
+    MYSQL_TYPE_DOUBLE(5, "DOUBLE", "DOUBLE"),
+    MYSQL_TYPE_NULL(6, "NULL", "NULL"),
+    MYSQL_TYPE_TIMESTAMP(7, "TIMESTAMP", "TIMESTAMP"),
+    MYSQL_TYPE_LONGLONG(8, "BIGINT", "LONGLONG"),
+    MYSQL_TYPE_INT24(9, "INT24", "INT24"),
+    MYSQL_TYPE_DATE(10, "DATE", "DATE"),
+    MYSQL_TYPE_TIME(11, "TIME", "TIME"),
+    MYSQL_TYPE_DATETIME(12, "DATETIME", "DATETIME"),
+    MYSQL_TYPE_YEAR(13, "YEAR", "YEAR"),
+    MYSQL_TYPE_NEWDATE(14, "NEWDATE", "NEWDATE"),
+    MYSQL_TYPE_VARCHAR(15, "VARCHAR", "VARCHAR"),
+    MYSQL_TYPE_BIT(16, "BIT", "BIT"),
+    MYSQL_TYPE_TIMESTAMP2(17, "TIMESTAMP2", "TIMESTAMP2"),
+    MYSQL_TYPE_DATETIME2(18, "DATETIME2", "DATETIME2"),
+    MYSQL_TYPE_TIME2(19, "TIME2", "TIME2"),
+    MYSQL_TYPE_JSON(245, "JSON", "JSON"),
+    MYSQL_TYPE_NEWDECIMAL(246, "NEWDECIMAL", "NEW DECIMAL"),
+    MYSQL_TYPE_ENUM(247, "CHAR", "ENUM"),
+    MYSQL_TYPE_SET(248, "CHAR", "SET"),
+    MYSQL_TYPE_TINY_BLOB(249, "TINYBLOB", "TINY BLOB"),
+    MYSQL_TYPE_MEDIUM_BLOB(250, "MEDIUMBLOB", "MEDIUM BLOB"),
+    MYSQL_TYPE_LONG_BLOB(251, "LONGBLOB", "LONG BLOB"),
+    MYSQL_TYPE_BLOB(252, "BLOB", "BLOB"),
+    MYSQL_TYPE_VARSTRING(253, "VARSTRING", "VAR STRING"),
+    MYSQL_TYPE_STRING(254, "CHAR", "STRING"),
+    MYSQL_TYPE_GEOMETRY(255, "GEOMETRY", "GEOMETRY"),
+    MYSQL_TYPE_MAP(400, "MAP", "MAP");
 
-    private MysqlColType(int code, String desc) {
+    private MysqlColType(int code, String name, String desc) {
         this.code = code;
+        this.name = name;
         this.desc = desc;
     }
 
     // used in network
     private int code;
+
+    private String name;

Review Comment:
   could u add some comment to explain the diff between name and desc



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/analyzer/UnboundFunction.java:
##########
@@ -38,24 +38,34 @@ public class UnboundFunction extends Function implements 
Unbound, PropagateNulla
     private final String dbName;
     private final String name;
     private final boolean isDistinct;
+    private final String source;

Review Comment:
   what this? please add some comment. and if it could be null, use 
`Optional<String>`



##########
fe/fe-core/src/main/java/org/apache/doris/persist/OperationType.java:
##########
@@ -340,6 +340,15 @@ public class OperationType {
     // change an auto increment id for a column
     public static final short OP_UPDATE_AUTO_INCREMENT_ID = 437;
 
+    // plsql 440 ~ 450
+    public static final short OP_ADD_PLSQL_STORED_PROCEDURE = 440;
+
+    public static final short OP_DROP_PLSQL_STORED_PROCEDURE = 441;
+
+    public static final short OP_ADD_PLSQL_PACKAGE = 442;
+
+    public static final short OP_DROP_PLSQL_PACKAGE = 443;
+

Review Comment:
   why use this duration? check not conflict with cloud



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/CreateProcedureCommand.java:
##########
@@ -0,0 +1,67 @@
+// 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.nereids.trees.plans.commands;
+
+import org.apache.doris.nereids.annotation.Developing;
+import org.apache.doris.nereids.trees.plans.PlanType;
+import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor;
+import org.apache.doris.plsql.metastore.PlsqlMetaClient;
+import org.apache.doris.qe.ConnectContext;
+import org.apache.doris.qe.StmtExecutor;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+/**
+ * create table procedure
+ */
+@Developing
+public class CreateProcedureCommand extends Command implements ForwardWithSync 
{
+    public static final Logger LOG = 
LogManager.getLogger(CreateProcedureCommand.class);
+
+    private PlsqlMetaClient client;

Review Comment:
   final



##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/CreateProcedureCommand.java:
##########
@@ -0,0 +1,67 @@
+// 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.nereids.trees.plans.commands;
+
+import org.apache.doris.nereids.annotation.Developing;
+import org.apache.doris.nereids.trees.plans.PlanType;
+import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor;
+import org.apache.doris.plsql.metastore.PlsqlMetaClient;
+import org.apache.doris.qe.ConnectContext;
+import org.apache.doris.qe.StmtExecutor;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+/**
+ * create table procedure
+ */
+@Developing
+public class CreateProcedureCommand extends Command implements ForwardWithSync 
{
+    public static final Logger LOG = 
LogManager.getLogger(CreateProcedureCommand.class);
+
+    private PlsqlMetaClient client;
+    private final String name;
+    private final String source;
+    private final boolean isForce;
+
+    /**
+     * constructor
+     */
+    public CreateProcedureCommand(String name, String source, boolean isForce) 
{
+        super(PlanType.CREATE_PROCEDURE_COMMAND);
+        this.client = new PlsqlMetaClient();
+        this.name = name;
+        this.source = source;
+        this.isForce = isForce;

Review Comment:
   u should use requireNonNull to check null for name and source to ensure them 
not null



##########
fe/fe-core/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4:
##########
@@ -34,6 +34,12 @@ singleStatement
     ;
 
 statement
+    : statementBase # statementBaseAlias
+    | CALL functionName=identifier LEFT_PAREN (expression (COMMA 
expression)*)? RIGHT_PAREN #callProcedure
+    | (ALTER | CREATE (OR REPLACE)? | REPLACE)? (PROCEDURE | PROC) identifier 
LEFT_PAREN .*? RIGHT_PAREN .*? #createProcedure

Review Comment:
   `(ALTER | CREATE (OR REPLACE)? | REPLACE)` why this is optional?



-- 
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

Reply via email to