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