morrySnow commented on code in PR #23635: URL: https://github.com/apache/doris/pull/23635#discussion_r1312473852
########## fe/fe-core/src/main/java/org/apache/doris/nereids/util/ParseSqlUtils.java: ########## @@ -0,0 +1,84 @@ +// 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.util; + +import org.apache.doris.nereids.DorisLexer; +import org.apache.doris.nereids.DorisParser; +import org.apache.doris.nereids.StatementContext; +import org.apache.doris.nereids.glue.LogicalPlanAdapter; +import org.apache.doris.nereids.parser.CaseInsensitiveStream; +import org.apache.doris.nereids.parser.LogicalPlanBuilder; +import org.apache.doris.nereids.parser.ParseErrorListener; +import org.apache.doris.nereids.parser.PostProcessor; +import org.apache.doris.nereids.trees.plans.logical.LogicalPlan; +import org.apache.doris.qe.ConnectContext; +import org.apache.doris.qe.OriginStatement; + +import org.antlr.v4.runtime.CharStreams; +import org.antlr.v4.runtime.CommonTokenStream; +import org.antlr.v4.runtime.ParserRuleContext; +import org.antlr.v4.runtime.atn.PredictionMode; +import org.antlr.v4.runtime.misc.ParseCancellationException; + +/** + * This tool is used to use nereids to parse SQL statements. + */ +public class ParseSqlUtils { + + /** + * parse sql to LogicalPlanAdapter + * @param sql one sql + * @return LogicalPlanAdapter + */ + public static LogicalPlanAdapter parseSingleStringSql(String sql) { Review Comment: why add this function? why not use `org.apache.doris.nereids.parser.NereidsParser#parseSQL` ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ExportCommand.java: ########## @@ -17,67 +17,337 @@ package org.apache.doris.nereids.trees.plans.commands; -import org.apache.doris.analysis.Analyzer; import org.apache.doris.analysis.BrokerDesc; -import org.apache.doris.analysis.ExportStmt; -import org.apache.doris.analysis.PartitionNames; +import org.apache.doris.analysis.LoadStmt; +import org.apache.doris.analysis.OutFileClause; +import org.apache.doris.analysis.Separator; +import org.apache.doris.analysis.StorageBackend; import org.apache.doris.analysis.TableName; -import org.apache.doris.analysis.TableRef; +import org.apache.doris.analysis.UserIdentity; +import org.apache.doris.catalog.BrokerMgr; +import org.apache.doris.catalog.Database; +import org.apache.doris.catalog.Env; +import org.apache.doris.catalog.Partition; +import org.apache.doris.catalog.Table; +import org.apache.doris.common.AnalysisException; +import org.apache.doris.common.Config; +import org.apache.doris.common.DdlException; +import org.apache.doris.common.ErrorCode; +import org.apache.doris.common.ErrorReport; +import org.apache.doris.common.FeNameFormat; +import org.apache.doris.common.UserException; +import org.apache.doris.common.util.PropertyAnalyzer; +import org.apache.doris.common.util.Util; +import org.apache.doris.load.ExportJob; +import org.apache.doris.mysql.privilege.PrivPredicate; import org.apache.doris.nereids.trees.plans.PlanType; import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor; -import org.apache.doris.nereids.util.Utils; +import org.apache.doris.nereids.util.RelationUtil; import org.apache.doris.qe.ConnectContext; +import org.apache.doris.qe.SessionVariable; import org.apache.doris.qe.StmtExecutor; +import org.apache.doris.qe.VariableMgr; + +import com.google.common.base.Splitter; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Maps; +import lombok.Getter; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.UUID; +import java.util.stream.Collectors; /** - * export table + * EXPORT statement, export data to dirs by broker. + * + * syntax: + * EXPORT TABLE table_name [PARTITION (name1[, ...])] + * TO 'export_target_path' + * [PROPERTIES("key"="value")] + * BY BROKER 'broker_name' [( $broker_attrs)] */ public class ExportCommand extends Command implements ForwardWithSync { + public static final String PARALLELISM = "parallelism"; + public static final String LABEL = "label"; + private static final String DEFAULT_COLUMN_SEPARATOR = "\t"; + private static final String DEFAULT_LINE_DELIMITER = "\n"; + private static final String DEFAULT_PARALLELISM = "1"; + private static final ImmutableSet<String> PROPERTIES_SET = new ImmutableSet.Builder<String>() + .add(LABEL) + .add(PARALLELISM) + .add(LoadStmt.EXEC_MEM_LIMIT) + .add(LoadStmt.TIMEOUT_PROPERTY) + .add(LoadStmt.KEY_IN_PARAM_COLUMNS) + .add(LoadStmt.TIMEOUT_PROPERTY) + .add(OutFileClause.PROP_MAX_FILE_SIZE) + .add(OutFileClause.PROP_DELETE_EXISTING_FILES) + .add(PropertyAnalyzer.PROPERTIES_COLUMN_SEPARATOR) + .add(PropertyAnalyzer.PROPERTIES_LINE_DELIMITER) + .add("format") + .build(); + private List<String> nameParts; private String whereSql; private String path; private List<String> partitionsNameList; private Map<String, String> fileProperties; private BrokerDesc brokerDesc; + // properties + private TableName tblName; + private String columnSeparator; + private String lineDelimiter; + private String columns; + private String format; + private String label; + private Integer parallelism; + private String maxFileSize; + private String deleteExistingFiles; + + // session and user identity + private SessionVariable sessionVariables; + private String qualifiedUser; + private UserIdentity userIdentity; + + // Export Job + @Getter + private ExportJob exportJob; + /** * constructor of ExportCommand */ public ExportCommand(List<String> nameParts, List<String> partitions, String whereSql, String path, Map<String, String> fileProperties, BrokerDesc brokerDesc) { super(PlanType.EXPORT_COMMAND); - this.nameParts = nameParts; + this.nameParts = ImmutableList.copyOf(Objects.requireNonNull(nameParts, "nameParts should not be null")); + this.path = Objects.requireNonNull(path.trim(), "export path should not be null"); this.partitionsNameList = partitions; this.whereSql = whereSql; - this.path = path.trim(); this.fileProperties = fileProperties; this.brokerDesc = brokerDesc; Review Comment: u should check null on all non-primitive vars. and use immutableCollection copy of on all collection vars ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ExportCommand.java: ########## @@ -17,67 +17,337 @@ package org.apache.doris.nereids.trees.plans.commands; -import org.apache.doris.analysis.Analyzer; import org.apache.doris.analysis.BrokerDesc; -import org.apache.doris.analysis.ExportStmt; -import org.apache.doris.analysis.PartitionNames; +import org.apache.doris.analysis.LoadStmt; +import org.apache.doris.analysis.OutFileClause; +import org.apache.doris.analysis.Separator; +import org.apache.doris.analysis.StorageBackend; import org.apache.doris.analysis.TableName; -import org.apache.doris.analysis.TableRef; +import org.apache.doris.analysis.UserIdentity; +import org.apache.doris.catalog.BrokerMgr; +import org.apache.doris.catalog.Database; +import org.apache.doris.catalog.Env; +import org.apache.doris.catalog.Partition; +import org.apache.doris.catalog.Table; +import org.apache.doris.common.AnalysisException; +import org.apache.doris.common.Config; +import org.apache.doris.common.DdlException; +import org.apache.doris.common.ErrorCode; +import org.apache.doris.common.ErrorReport; +import org.apache.doris.common.FeNameFormat; +import org.apache.doris.common.UserException; +import org.apache.doris.common.util.PropertyAnalyzer; +import org.apache.doris.common.util.Util; +import org.apache.doris.load.ExportJob; +import org.apache.doris.mysql.privilege.PrivPredicate; import org.apache.doris.nereids.trees.plans.PlanType; import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor; -import org.apache.doris.nereids.util.Utils; +import org.apache.doris.nereids.util.RelationUtil; import org.apache.doris.qe.ConnectContext; +import org.apache.doris.qe.SessionVariable; import org.apache.doris.qe.StmtExecutor; +import org.apache.doris.qe.VariableMgr; + +import com.google.common.base.Splitter; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Maps; +import lombok.Getter; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.UUID; +import java.util.stream.Collectors; /** - * export table + * EXPORT statement, export data to dirs by broker. + * + * syntax: + * EXPORT TABLE table_name [PARTITION (name1[, ...])] + * TO 'export_target_path' + * [PROPERTIES("key"="value")] + * BY BROKER 'broker_name' [( $broker_attrs)] */ public class ExportCommand extends Command implements ForwardWithSync { + public static final String PARALLELISM = "parallelism"; + public static final String LABEL = "label"; + private static final String DEFAULT_COLUMN_SEPARATOR = "\t"; + private static final String DEFAULT_LINE_DELIMITER = "\n"; + private static final String DEFAULT_PARALLELISM = "1"; + private static final ImmutableSet<String> PROPERTIES_SET = new ImmutableSet.Builder<String>() + .add(LABEL) + .add(PARALLELISM) + .add(LoadStmt.EXEC_MEM_LIMIT) + .add(LoadStmt.TIMEOUT_PROPERTY) + .add(LoadStmt.KEY_IN_PARAM_COLUMNS) + .add(LoadStmt.TIMEOUT_PROPERTY) + .add(OutFileClause.PROP_MAX_FILE_SIZE) + .add(OutFileClause.PROP_DELETE_EXISTING_FILES) + .add(PropertyAnalyzer.PROPERTIES_COLUMN_SEPARATOR) + .add(PropertyAnalyzer.PROPERTIES_LINE_DELIMITER) + .add("format") + .build(); + private List<String> nameParts; private String whereSql; private String path; private List<String> partitionsNameList; private Map<String, String> fileProperties; private BrokerDesc brokerDesc; + // properties + private TableName tblName; + private String columnSeparator; + private String lineDelimiter; + private String columns; + private String format; + private String label; + private Integer parallelism; + private String maxFileSize; + private String deleteExistingFiles; + + // session and user identity + private SessionVariable sessionVariables; + private String qualifiedUser; + private UserIdentity userIdentity; + + // Export Job + @Getter + private ExportJob exportJob; + /** * constructor of ExportCommand */ public ExportCommand(List<String> nameParts, List<String> partitions, String whereSql, String path, Map<String, String> fileProperties, BrokerDesc brokerDesc) { super(PlanType.EXPORT_COMMAND); - this.nameParts = nameParts; + this.nameParts = ImmutableList.copyOf(Objects.requireNonNull(nameParts, "nameParts should not be null")); + this.path = Objects.requireNonNull(path.trim(), "export path should not be null"); this.partitionsNameList = partitions; Review Comment: partitionNameList -> partitionNames ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ExportCommand.java: ########## @@ -17,67 +17,337 @@ package org.apache.doris.nereids.trees.plans.commands; -import org.apache.doris.analysis.Analyzer; import org.apache.doris.analysis.BrokerDesc; -import org.apache.doris.analysis.ExportStmt; -import org.apache.doris.analysis.PartitionNames; +import org.apache.doris.analysis.LoadStmt; +import org.apache.doris.analysis.OutFileClause; +import org.apache.doris.analysis.Separator; +import org.apache.doris.analysis.StorageBackend; import org.apache.doris.analysis.TableName; -import org.apache.doris.analysis.TableRef; +import org.apache.doris.analysis.UserIdentity; +import org.apache.doris.catalog.BrokerMgr; +import org.apache.doris.catalog.Database; +import org.apache.doris.catalog.Env; +import org.apache.doris.catalog.Partition; +import org.apache.doris.catalog.Table; +import org.apache.doris.common.AnalysisException; +import org.apache.doris.common.Config; +import org.apache.doris.common.DdlException; +import org.apache.doris.common.ErrorCode; +import org.apache.doris.common.ErrorReport; +import org.apache.doris.common.FeNameFormat; +import org.apache.doris.common.UserException; +import org.apache.doris.common.util.PropertyAnalyzer; +import org.apache.doris.common.util.Util; +import org.apache.doris.load.ExportJob; +import org.apache.doris.mysql.privilege.PrivPredicate; import org.apache.doris.nereids.trees.plans.PlanType; import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor; -import org.apache.doris.nereids.util.Utils; +import org.apache.doris.nereids.util.RelationUtil; import org.apache.doris.qe.ConnectContext; +import org.apache.doris.qe.SessionVariable; import org.apache.doris.qe.StmtExecutor; +import org.apache.doris.qe.VariableMgr; + +import com.google.common.base.Splitter; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Maps; +import lombok.Getter; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.UUID; +import java.util.stream.Collectors; /** - * export table + * EXPORT statement, export data to dirs by broker. + * + * syntax: + * EXPORT TABLE table_name [PARTITION (name1[, ...])] + * TO 'export_target_path' + * [PROPERTIES("key"="value")] + * BY BROKER 'broker_name' [( $broker_attrs)] */ public class ExportCommand extends Command implements ForwardWithSync { + public static final String PARALLELISM = "parallelism"; + public static final String LABEL = "label"; + private static final String DEFAULT_COLUMN_SEPARATOR = "\t"; + private static final String DEFAULT_LINE_DELIMITER = "\n"; + private static final String DEFAULT_PARALLELISM = "1"; + private static final ImmutableSet<String> PROPERTIES_SET = new ImmutableSet.Builder<String>() + .add(LABEL) + .add(PARALLELISM) + .add(LoadStmt.EXEC_MEM_LIMIT) + .add(LoadStmt.TIMEOUT_PROPERTY) + .add(LoadStmt.KEY_IN_PARAM_COLUMNS) + .add(LoadStmt.TIMEOUT_PROPERTY) + .add(OutFileClause.PROP_MAX_FILE_SIZE) + .add(OutFileClause.PROP_DELETE_EXISTING_FILES) + .add(PropertyAnalyzer.PROPERTIES_COLUMN_SEPARATOR) + .add(PropertyAnalyzer.PROPERTIES_LINE_DELIMITER) + .add("format") + .build(); + private List<String> nameParts; private String whereSql; private String path; private List<String> partitionsNameList; private Map<String, String> fileProperties; private BrokerDesc brokerDesc; + // properties + private TableName tblName; + private String columnSeparator; + private String lineDelimiter; + private String columns; + private String format; + private String label; + private Integer parallelism; + private String maxFileSize; + private String deleteExistingFiles; + + // session and user identity + private SessionVariable sessionVariables; + private String qualifiedUser; + private UserIdentity userIdentity; + + // Export Job + @Getter Review Comment: lombok is forbidden in Nereids ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ExportCommand.java: ########## @@ -17,67 +17,337 @@ package org.apache.doris.nereids.trees.plans.commands; -import org.apache.doris.analysis.Analyzer; import org.apache.doris.analysis.BrokerDesc; -import org.apache.doris.analysis.ExportStmt; -import org.apache.doris.analysis.PartitionNames; +import org.apache.doris.analysis.LoadStmt; +import org.apache.doris.analysis.OutFileClause; +import org.apache.doris.analysis.Separator; +import org.apache.doris.analysis.StorageBackend; import org.apache.doris.analysis.TableName; -import org.apache.doris.analysis.TableRef; +import org.apache.doris.analysis.UserIdentity; +import org.apache.doris.catalog.BrokerMgr; +import org.apache.doris.catalog.Database; +import org.apache.doris.catalog.Env; +import org.apache.doris.catalog.Partition; +import org.apache.doris.catalog.Table; +import org.apache.doris.common.AnalysisException; +import org.apache.doris.common.Config; +import org.apache.doris.common.DdlException; +import org.apache.doris.common.ErrorCode; +import org.apache.doris.common.ErrorReport; +import org.apache.doris.common.FeNameFormat; +import org.apache.doris.common.UserException; +import org.apache.doris.common.util.PropertyAnalyzer; +import org.apache.doris.common.util.Util; +import org.apache.doris.load.ExportJob; +import org.apache.doris.mysql.privilege.PrivPredicate; import org.apache.doris.nereids.trees.plans.PlanType; import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor; -import org.apache.doris.nereids.util.Utils; +import org.apache.doris.nereids.util.RelationUtil; import org.apache.doris.qe.ConnectContext; +import org.apache.doris.qe.SessionVariable; import org.apache.doris.qe.StmtExecutor; +import org.apache.doris.qe.VariableMgr; + +import com.google.common.base.Splitter; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Maps; +import lombok.Getter; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.UUID; +import java.util.stream.Collectors; /** - * export table + * EXPORT statement, export data to dirs by broker. + * + * syntax: + * EXPORT TABLE table_name [PARTITION (name1[, ...])] + * TO 'export_target_path' + * [PROPERTIES("key"="value")] + * BY BROKER 'broker_name' [( $broker_attrs)] */ public class ExportCommand extends Command implements ForwardWithSync { + public static final String PARALLELISM = "parallelism"; + public static final String LABEL = "label"; + private static final String DEFAULT_COLUMN_SEPARATOR = "\t"; + private static final String DEFAULT_LINE_DELIMITER = "\n"; + private static final String DEFAULT_PARALLELISM = "1"; + private static final ImmutableSet<String> PROPERTIES_SET = new ImmutableSet.Builder<String>() + .add(LABEL) + .add(PARALLELISM) + .add(LoadStmt.EXEC_MEM_LIMIT) + .add(LoadStmt.TIMEOUT_PROPERTY) + .add(LoadStmt.KEY_IN_PARAM_COLUMNS) + .add(LoadStmt.TIMEOUT_PROPERTY) + .add(OutFileClause.PROP_MAX_FILE_SIZE) + .add(OutFileClause.PROP_DELETE_EXISTING_FILES) + .add(PropertyAnalyzer.PROPERTIES_COLUMN_SEPARATOR) + .add(PropertyAnalyzer.PROPERTIES_LINE_DELIMITER) + .add("format") + .build(); + private List<String> nameParts; private String whereSql; private String path; private List<String> partitionsNameList; private Map<String, String> fileProperties; private BrokerDesc brokerDesc; + // properties + private TableName tblName; Review Comment: Set variables to final as much as possible ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ExportCommand.java: ########## @@ -17,67 +17,337 @@ package org.apache.doris.nereids.trees.plans.commands; -import org.apache.doris.analysis.Analyzer; import org.apache.doris.analysis.BrokerDesc; -import org.apache.doris.analysis.ExportStmt; -import org.apache.doris.analysis.PartitionNames; +import org.apache.doris.analysis.LoadStmt; +import org.apache.doris.analysis.OutFileClause; +import org.apache.doris.analysis.Separator; +import org.apache.doris.analysis.StorageBackend; import org.apache.doris.analysis.TableName; -import org.apache.doris.analysis.TableRef; +import org.apache.doris.analysis.UserIdentity; +import org.apache.doris.catalog.BrokerMgr; +import org.apache.doris.catalog.Database; +import org.apache.doris.catalog.Env; +import org.apache.doris.catalog.Partition; +import org.apache.doris.catalog.Table; +import org.apache.doris.common.AnalysisException; +import org.apache.doris.common.Config; +import org.apache.doris.common.DdlException; +import org.apache.doris.common.ErrorCode; +import org.apache.doris.common.ErrorReport; +import org.apache.doris.common.FeNameFormat; +import org.apache.doris.common.UserException; +import org.apache.doris.common.util.PropertyAnalyzer; +import org.apache.doris.common.util.Util; +import org.apache.doris.load.ExportJob; +import org.apache.doris.mysql.privilege.PrivPredicate; import org.apache.doris.nereids.trees.plans.PlanType; import org.apache.doris.nereids.trees.plans.visitor.PlanVisitor; -import org.apache.doris.nereids.util.Utils; +import org.apache.doris.nereids.util.RelationUtil; import org.apache.doris.qe.ConnectContext; +import org.apache.doris.qe.SessionVariable; import org.apache.doris.qe.StmtExecutor; +import org.apache.doris.qe.VariableMgr; + +import com.google.common.base.Splitter; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Maps; +import lombok.Getter; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.UUID; +import java.util.stream.Collectors; /** - * export table + * EXPORT statement, export data to dirs by broker. + * + * syntax: + * EXPORT TABLE table_name [PARTITION (name1[, ...])] + * TO 'export_target_path' + * [PROPERTIES("key"="value")] + * BY BROKER 'broker_name' [( $broker_attrs)] */ public class ExportCommand extends Command implements ForwardWithSync { + public static final String PARALLELISM = "parallelism"; + public static final String LABEL = "label"; + private static final String DEFAULT_COLUMN_SEPARATOR = "\t"; + private static final String DEFAULT_LINE_DELIMITER = "\n"; + private static final String DEFAULT_PARALLELISM = "1"; + private static final ImmutableSet<String> PROPERTIES_SET = new ImmutableSet.Builder<String>() + .add(LABEL) + .add(PARALLELISM) + .add(LoadStmt.EXEC_MEM_LIMIT) + .add(LoadStmt.TIMEOUT_PROPERTY) + .add(LoadStmt.KEY_IN_PARAM_COLUMNS) + .add(LoadStmt.TIMEOUT_PROPERTY) + .add(OutFileClause.PROP_MAX_FILE_SIZE) + .add(OutFileClause.PROP_DELETE_EXISTING_FILES) + .add(PropertyAnalyzer.PROPERTIES_COLUMN_SEPARATOR) + .add(PropertyAnalyzer.PROPERTIES_LINE_DELIMITER) + .add("format") + .build(); + private List<String> nameParts; private String whereSql; private String path; private List<String> partitionsNameList; private Map<String, String> fileProperties; private BrokerDesc brokerDesc; + // properties + private TableName tblName; + private String columnSeparator; + private String lineDelimiter; + private String columns; + private String format; + private String label; + private Integer parallelism; Review Comment: why use `Integer` rather than `int`? ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ExportCommand.java: ########## @@ -49,35 +100,248 @@ public class ExportCommand extends Command implements ForwardWithSync { public ExportCommand(List<String> nameParts, List<String> partitions, String whereSql, String path, Map<String, String> fileProperties, BrokerDesc brokerDesc) { super(PlanType.EXPORT_COMMAND); - this.nameParts = nameParts; + this.nameParts = ImmutableList.copyOf(Objects.requireNonNull(nameParts, "nameParts should not be null")); + this.path = Objects.requireNonNull(path.trim(), "export path should not be null"); this.partitionsNameList = partitions; this.whereSql = whereSql; - this.path = path.trim(); this.fileProperties = fileProperties; Review Comment: Map should use ImmutableMap.copyOf ########## fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java: ########## @@ -403,8 +402,12 @@ public LogicalPlan visitDelete(DeleteContext ctx) { @Override public LogicalPlan visitExport(ExportContext ctx) { List<String> tableName = visitMultipartIdentifier(ctx.tableName); - List<String> partitions = ctx.partition == null ? ImmutableList.of() : visitIdentifierList(ctx.partition); - String path = parseConstant(ctx.filePath); + List<String> partitions = ctx.partition == null ? null : visitIdentifierList(ctx.partition); Review Comment: why change empty list to null? we should never pass null as a parameter ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ExportCommand.java: ########## @@ -49,35 +100,248 @@ public class ExportCommand extends Command implements ForwardWithSync { public ExportCommand(List<String> nameParts, List<String> partitions, String whereSql, String path, Map<String, String> fileProperties, BrokerDesc brokerDesc) { super(PlanType.EXPORT_COMMAND); - this.nameParts = nameParts; + this.nameParts = ImmutableList.copyOf(Objects.requireNonNull(nameParts, "nameParts should not be null")); Review Comment: all non-primitive params should use `Objects.requireNonNull` to check not null param ########## fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java: ########## @@ -415,7 +418,7 @@ public LogicalPlan visitExport(ExportContext ctx) { whereSql = whereClauseContext.start.getInputStream().getText(interval); } - Map<String, String> filePropertiesMap = null; + Map<String, String> filePropertiesMap = Maps.newHashMap(); Review Comment: should use `ImmutableMap.of()` ########## fe/fe-core/src/main/java/org/apache/doris/nereids/parser/LogicalPlanBuilder.java: ########## @@ -435,13 +438,13 @@ public Map<String, String> visitPropertyClause(PropertyClauseContext ctx) { @Override public Map<String, String> visitPropertyItemList(PropertyItemListContext ctx) { - Builder<String, String> propertiesMap = ImmutableMap.builder(); + Map<String, String> propertiesMap = Maps.newHashMap(); for (PropertyItemContext argument : ctx.properties) { String key = parsePropertyKey(argument.key); String value = parsePropertyValue(argument.value); propertiesMap.put(key, value); } - return propertiesMap.build(); + return propertiesMap; Review Comment: must use immutable map, do not change this ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ExportCommand.java: ########## @@ -49,35 +100,248 @@ public class ExportCommand extends Command implements ForwardWithSync { public ExportCommand(List<String> nameParts, List<String> partitions, String whereSql, String path, Map<String, String> fileProperties, BrokerDesc brokerDesc) { super(PlanType.EXPORT_COMMAND); - this.nameParts = nameParts; + this.nameParts = ImmutableList.copyOf(Objects.requireNonNull(nameParts, "nameParts should not be null")); + this.path = Objects.requireNonNull(path.trim(), "export path should not be null"); this.partitionsNameList = partitions; this.whereSql = whereSql; - this.path = path.trim(); this.fileProperties = fileProperties; - this.brokerDesc = brokerDesc; + if (brokerDesc == null) { + this.brokerDesc = new BrokerDesc("local", StorageBackend.StorageType.LOCAL, null); + } else { + this.brokerDesc = brokerDesc; + } } @Override public void run(ConnectContext ctx, StmtExecutor executor) throws Exception { - ExportStmt exportStmt = generateExportStmt(); - Analyzer analyzer = new Analyzer(ctx.getEnv(), ctx); - exportStmt.analyze(analyzer); - ctx.getEnv().getExportMgr().addExportJobAndRegisterTask(exportStmt); + // get tblName + TableName tblName = getTableName(ctx); + + // check auth + if (!Env.getCurrentEnv().getAccessManager().checkTblPriv(ctx, tblName.getDb(), tblName.getTbl(), + PrivPredicate.SELECT)) { + ErrorReport.reportAnalysisException(ErrorCode.ERR_TABLEACCESS_DENIED_ERROR, "EXPORT", + ctx.getQualifiedUser(), + ctx.getRemoteIP(), + tblName.getDb() + ": " + tblName.getTbl()); + } + + // convert key to lowercase + Map<String, String> lowercaseProperties = convertPropertyKeyToLowercase(fileProperties); + + // check phases + checkAllParameters(ctx, tblName, lowercaseProperties); + + ExportJob exportJob = generateExportJob(ctx, lowercaseProperties, tblName); + // register job + ctx.getEnv().getExportMgr().addExportJobAndRegisterTask(exportJob); + } + + private void checkAllParameters(ConnectContext ctx, TableName tblName, Map<String, String> lowercaseProperties) + throws UserException { + checkPartitions(ctx.getEnv(), tblName); + checkBrokerDesc(ctx); + checkFileProperties(lowercaseProperties, tblName); + } + + // convert property key to lowercase + private Map<String, String> convertPropertyKeyToLowercase(Map<String, String> properties) throws UserException { + for (String key : properties.keySet()) { + if (!PROPERTIES_SET.contains(key.toLowerCase())) { + throw new UserException("Invalid property key: [" + key + "]"); + } + } + + // convert key to lowercase + Map<String, String> lowercaseProperties = Maps.newHashMap(); + for (String key : properties.keySet()) { + lowercaseProperties.put(key.toLowerCase(), properties.get(key)); + } + return lowercaseProperties; + } + + // check partitions specified by user are belonged to the table. + private void checkPartitions(Env env, TableName tblName) throws AnalysisException, UserException { + if (this.partitionsNameList == null) { + return; + } + + if (this.partitionsNameList.size() > Config.maximum_number_of_export_partitions) { + throw new AnalysisException("The partitions number of this export job is larger than the maximum number" + + " of partitions allowed by an export job"); + } + + Database db = env.getInternalCatalog().getDbOrAnalysisException(tblName.getDb()); + Table table = db.getTableOrAnalysisException(tblName.getTbl()); + table.readLock(); + try { + // check table + if (!table.isPartitioned()) { + throw new AnalysisException("Table[" + tblName.getTbl() + "] is not partitioned."); + } + Table.TableType tblType = table.getType(); + switch (tblType) { + case MYSQL: + case ODBC: + case JDBC: + case OLAP: + break; + case BROKER: + case SCHEMA: + case INLINE_VIEW: + case VIEW: + default: + throw new AnalysisException("Table[" + tblName.getTbl() + "] is " + + tblType + " type, do not support EXPORT."); + } + + for (String partitionName : this.partitionsNameList) { + Partition partition = table.getPartition(partitionName); + if (partition == null) { + throw new AnalysisException("Partition [" + partitionName + "] does not exist " + + "in Table[" + tblName.getTbl() + "]"); + } + } + } finally { + table.readUnlock(); + } + } + + private void checkBrokerDesc(ConnectContext ctx) throws UserException { + // check path is valid + StorageBackend.checkPath(this.path, this.brokerDesc.getStorageType()); + + if (brokerDesc.getStorageType() == StorageBackend.StorageType.BROKER) { + BrokerMgr brokerMgr = ctx.getEnv().getBrokerMgr(); + if (!brokerMgr.containsBroker(brokerDesc.getName())) { + throw new AnalysisException("broker " + brokerDesc.getName() + " does not exist"); + } + if (null == brokerMgr.getAnyBroker(brokerDesc.getName())) { + throw new AnalysisException("failed to get alive broker"); + } + } + } + + private ExportJob generateExportJob(ConnectContext ctx, Map<String, String> lowercaseProperties, TableName tblName) + throws UserException { + ExportJob exportJob = new ExportJob(); + // set export job + Database db = Env.getCurrentInternalCatalog().getDbOrDdlException(tblName.getDb()); + exportJob.setDbId(db.getId()); + exportJob.setTableName(tblName); + exportJob.setExportTable(db.getTableOrDdlException(tblName.getTbl())); + exportJob.setTableId(db.getTableOrDdlException(tblName.getTbl()).getId()); + + // set partitions + exportJob.setPartitionNames(this.partitionsNameList); + // set where sql + exportJob.setWhereSql(this.whereSql); + // set path + exportJob.setExportPath(this.path); + + // set column separator + String columnSeparator = Separator.convertSeparator(PropertyAnalyzer.analyzeColumnSeparator( + lowercaseProperties, DEFAULT_COLUMN_SEPARATOR)); + exportJob.setColumnSeparator(columnSeparator); + + // set line delimiter + String lineDelimiter = Separator.convertSeparator(PropertyAnalyzer.analyzeLineDelimiter( + lowercaseProperties, DEFAULT_LINE_DELIMITER)); + exportJob.setLineDelimiter(lineDelimiter); + + // set format + exportJob.setFormat(lowercaseProperties.getOrDefault(LoadStmt.KEY_IN_PARAM_FORMAT_TYPE, "csv") + .toLowerCase()); + + // set parallelism + Integer parallelism; Review Comment: int is better. i think u should change ExportJob.parallelism to int too ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ExportCommand.java: ########## @@ -49,35 +100,248 @@ public class ExportCommand extends Command implements ForwardWithSync { public ExportCommand(List<String> nameParts, List<String> partitions, String whereSql, String path, Map<String, String> fileProperties, BrokerDesc brokerDesc) { super(PlanType.EXPORT_COMMAND); - this.nameParts = nameParts; + this.nameParts = ImmutableList.copyOf(Objects.requireNonNull(nameParts, "nameParts should not be null")); + this.path = Objects.requireNonNull(path.trim(), "export path should not be null"); this.partitionsNameList = partitions; this.whereSql = whereSql; - this.path = path.trim(); this.fileProperties = fileProperties; - this.brokerDesc = brokerDesc; + if (brokerDesc == null) { + this.brokerDesc = new BrokerDesc("local", StorageBackend.StorageType.LOCAL, null); + } else { + this.brokerDesc = brokerDesc; + } } @Override public void run(ConnectContext ctx, StmtExecutor executor) throws Exception { - ExportStmt exportStmt = generateExportStmt(); - Analyzer analyzer = new Analyzer(ctx.getEnv(), ctx); - exportStmt.analyze(analyzer); - ctx.getEnv().getExportMgr().addExportJobAndRegisterTask(exportStmt); + // get tblName + TableName tblName = getTableName(ctx); + + // check auth + if (!Env.getCurrentEnv().getAccessManager().checkTblPriv(ctx, tblName.getDb(), tblName.getTbl(), + PrivPredicate.SELECT)) { + ErrorReport.reportAnalysisException(ErrorCode.ERR_TABLEACCESS_DENIED_ERROR, "EXPORT", + ctx.getQualifiedUser(), + ctx.getRemoteIP(), + tblName.getDb() + ": " + tblName.getTbl()); + } + + // convert key to lowercase + Map<String, String> lowercaseProperties = convertPropertyKeyToLowercase(fileProperties); Review Comment: if u need case insensitive key map, u could write `this. fileProperties = ImmutableSortedMap.copyOf(fileProperties, String.CASE_INSENSITIVE_ORDER);` in ctor ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ExportCommand.java: ########## @@ -49,35 +100,248 @@ public class ExportCommand extends Command implements ForwardWithSync { public ExportCommand(List<String> nameParts, List<String> partitions, String whereSql, String path, Map<String, String> fileProperties, BrokerDesc brokerDesc) { super(PlanType.EXPORT_COMMAND); - this.nameParts = nameParts; + this.nameParts = ImmutableList.copyOf(Objects.requireNonNull(nameParts, "nameParts should not be null")); + this.path = Objects.requireNonNull(path.trim(), "export path should not be null"); this.partitionsNameList = partitions; this.whereSql = whereSql; - this.path = path.trim(); this.fileProperties = fileProperties; - this.brokerDesc = brokerDesc; + if (brokerDesc == null) { Review Comment: should not pass `null`. u could use `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: [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]
