EmmyMiao87 commented on code in PR #8858: URL: https://github.com/apache/incubator-doris/pull/8858#discussion_r854737773
########## fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyzeStmt.java: ########## @@ -21,71 +21,134 @@ import org.apache.doris.catalog.Column; import org.apache.doris.catalog.Database; import org.apache.doris.catalog.Table; -import org.apache.doris.cluster.ClusterNamespace; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.Config; import org.apache.doris.common.ErrorCode; import org.apache.doris.common.ErrorReport; import org.apache.doris.common.UserException; +import org.apache.doris.common.util.Util; import org.apache.doris.mysql.privilege.PaloAuth; import org.apache.doris.mysql.privilege.PrivPredicate; import org.apache.doris.qe.ConnectContext; -import com.clearspring.analytics.util.Lists; import com.google.common.base.Preconditions; import com.google.common.base.Strings; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import com.google.common.collect.Sets; -import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; -import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.function.Predicate; /** * Collect statistics about a database - * <p> + * * syntax: * ANALYZE [[ db_name.tb_name ] [( column_name [, ...] )], ...] [ PROPERTIES(...) ] - * <p> - * db_name.tb_name: collect table and column statistics from tb_name - * <p> - * column_name: collect column statistics from column_name - * <p> - * properties: properties of statistics jobs + * + * db_name.tb_name: collect table and column statistics from tb_name + * + * column_name: collect column statistics from column_name + * + * properties: properties of statistics jobs */ public class AnalyzeStmt extends DdlStmt { + private static final Logger LOG = LogManager.getLogger(CreateRoutineLoadStmt.class); + + // time to wait for collect statistics + public static final String CBO_STATISTICS_TASK_TIMEOUT_SEC = "cbo_statistics_task_timeout_sec"; + + private static final ImmutableSet<String> PROPERTIES_SET = new ImmutableSet.Builder<String>() + .add(CBO_STATISTICS_TASK_TIMEOUT_SEC) + .build(); + + public static final Predicate<Long> DESIRED_TASK_TIMEOUT_SEC = (v) -> v > 0L; + private final TableName dbTableName; private final List<String> columnNames; - private Map<String, String> properties; + private final Map<String, String> properties; // after analyzed - private Database db; - private List<Table> tables; - private final Map<Long, List<String>> tableIdToColumnName = Maps.newHashMap(); + private long dbId; + private final Set<Long> tblIds = Sets.newHashSet(); + + private long taskTimeout = 60L; Review Comment: duplicate property ########## fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyzeStmt.java: ########## @@ -96,75 +159,59 @@ public Map<String, String> getProperties() { public void analyze(Analyzer analyzer) throws UserException { super.analyze(analyzer); - // step1: analyze database and table + // step1: analyze db, table and column if (this.dbTableName != null) { - String dbName; - if (Strings.isNullOrEmpty(this.dbTableName.getDb())) { - dbName = analyzer.getDefaultDb(); - } else { - dbName = ClusterNamespace.getFullName(analyzer.getClusterName(), this.dbTableName.getDb()); - } - - // check db - if (Strings.isNullOrEmpty(dbName)) { - ErrorReport.reportAnalysisException(ErrorCode.ERR_NO_DB_ERROR); - } - this.db = analyzer.getCatalog().getDbOrAnalysisException(dbName); - - // check table + this.dbTableName.analyze(analyzer); + String dbName = this.dbTableName.getDb(); String tblName = this.dbTableName.getTbl(); - if (Strings.isNullOrEmpty(tblName)) { - this.tables = this.db.getTables(); - for (Table table : this.tables) { - checkAnalyzePriv(dbName, table.getName()); - } - } else { - Table table = this.db.getTableOrAnalysisException(tblName); - this.tables = Collections.singletonList(table); - checkAnalyzePriv(dbName, table.getName()); - } - - // check column - if (this.columnNames == null || this.columnNames.isEmpty()) { - setTableIdToColumnName(); - } else { - Table table = this.db.getTableOrAnalysisException(tblName); - for (String columnName : this.columnNames) { - Column column = table.getColumn(columnName); - if (column == null) { - ErrorReport.reportAnalysisException(ErrorCode.ERR_WRONG_COLUMN_NAME, columnName); + checkAnalyzePriv(dbName, tblName); + + Database db = analyzer.getCatalog().getDbOrAnalysisException(dbName); + Table table = db.getTableOrAnalysisException(tblName); + + if (this.columnNames != null && !this.columnNames.isEmpty()) { + table.readLock(); + try { + for (String columnName : this.columnNames) { + Column column = table.getColumn(columnName); Review Comment: ```suggestion Column column = table.getBaseSchema(false); ``` ########## fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyzeStmt.java: ########## @@ -21,71 +21,134 @@ import org.apache.doris.catalog.Column; import org.apache.doris.catalog.Database; import org.apache.doris.catalog.Table; -import org.apache.doris.cluster.ClusterNamespace; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.Config; import org.apache.doris.common.ErrorCode; import org.apache.doris.common.ErrorReport; import org.apache.doris.common.UserException; +import org.apache.doris.common.util.Util; import org.apache.doris.mysql.privilege.PaloAuth; import org.apache.doris.mysql.privilege.PrivPredicate; import org.apache.doris.qe.ConnectContext; -import com.clearspring.analytics.util.Lists; import com.google.common.base.Preconditions; import com.google.common.base.Strings; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import com.google.common.collect.Sets; -import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; -import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.function.Predicate; /** * Collect statistics about a database - * <p> + * * syntax: * ANALYZE [[ db_name.tb_name ] [( column_name [, ...] )], ...] [ PROPERTIES(...) ] - * <p> - * db_name.tb_name: collect table and column statistics from tb_name - * <p> - * column_name: collect column statistics from column_name - * <p> - * properties: properties of statistics jobs + * + * db_name.tb_name: collect table and column statistics from tb_name + * + * column_name: collect column statistics from column_name + * + * properties: properties of statistics jobs */ public class AnalyzeStmt extends DdlStmt { + private static final Logger LOG = LogManager.getLogger(CreateRoutineLoadStmt.class); Review Comment: ```suggestion private static final Logger LOG = LogManager.getLogger(AnalyzeStmt.class); ``` ########## fe/fe-core/src/main/java/org/apache/doris/common/Config.java: ########## @@ -1632,10 +1632,10 @@ public class Config extends ConfigBase { @ConfField(mutable = true, masterOnly = true) public static int cbo_max_statistics_job_num = 20; /* - * the timeout of a statistics task + * the max timeout of a statistics task */ @ConfField(mutable = true, masterOnly = true) - public static int cbo_statistics_task_timeout_sec = 60; + public static int max_cbo_statistics_task_timeout_sec = 300; Review Comment: 60 or 300? ########## fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticsJob.java: ########## @@ -61,28 +63,30 @@ public enum JobState { */ private final Map<Long, List<String>> tableIdToColumnName; - private final Map<String, String> properties; + /** + * timeout of a statistics task + */ + private long taskTimeout; Review Comment: Please keep properties in here instead of only taskTimeout ########## fe/fe-core/src/main/java/org/apache/doris/analysis/AnalyzeStmt.java: ########## @@ -21,71 +21,134 @@ import org.apache.doris.catalog.Column; import org.apache.doris.catalog.Database; import org.apache.doris.catalog.Table; -import org.apache.doris.cluster.ClusterNamespace; import org.apache.doris.common.AnalysisException; import org.apache.doris.common.Config; import org.apache.doris.common.ErrorCode; import org.apache.doris.common.ErrorReport; import org.apache.doris.common.UserException; +import org.apache.doris.common.util.Util; import org.apache.doris.mysql.privilege.PaloAuth; import org.apache.doris.mysql.privilege.PrivPredicate; import org.apache.doris.qe.ConnectContext; -import com.clearspring.analytics.util.Lists; import com.google.common.base.Preconditions; import com.google.common.base.Strings; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Lists; import com.google.common.collect.Maps; +import com.google.common.collect.Sets; -import org.apache.commons.lang3.StringUtils; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; -import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Optional; +import java.util.Set; +import java.util.function.Predicate; /** * Collect statistics about a database - * <p> + * * syntax: * ANALYZE [[ db_name.tb_name ] [( column_name [, ...] )], ...] [ PROPERTIES(...) ] - * <p> - * db_name.tb_name: collect table and column statistics from tb_name - * <p> - * column_name: collect column statistics from column_name - * <p> - * properties: properties of statistics jobs + * + * db_name.tb_name: collect table and column statistics from tb_name + * + * column_name: collect column statistics from column_name + * + * properties: properties of statistics jobs */ public class AnalyzeStmt extends DdlStmt { + private static final Logger LOG = LogManager.getLogger(CreateRoutineLoadStmt.class); + + // time to wait for collect statistics + public static final String CBO_STATISTICS_TASK_TIMEOUT_SEC = "cbo_statistics_task_timeout_sec"; + + private static final ImmutableSet<String> PROPERTIES_SET = new ImmutableSet.Builder<String>() + .add(CBO_STATISTICS_TASK_TIMEOUT_SEC) + .build(); + + public static final Predicate<Long> DESIRED_TASK_TIMEOUT_SEC = (v) -> v > 0L; + private final TableName dbTableName; private final List<String> columnNames; - private Map<String, String> properties; + private final Map<String, String> properties; // after analyzed - private Database db; - private List<Table> tables; - private final Map<Long, List<String>> tableIdToColumnName = Maps.newHashMap(); + private long dbId; + private final Set<Long> tblIds = Sets.newHashSet(); + + private long taskTimeout = 60L; public AnalyzeStmt(TableName dbTableName, List<String> columns, Map<String, String> properties) { this.dbTableName = dbTableName; this.columnNames = columns; - this.properties = properties; + this.properties = properties == null ? Maps.newHashMap() : properties; + } + + public long getDbId() { + return this.dbId; + } + + public Set<Long> getTblIds() { + return this.tblIds; + } + + public long getTaskTimeout() { + return this.taskTimeout; Review Comment: ```suggestion return properties.get(xxx); ``` -- 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