morrySnow commented on code in PR #14201:
URL: https://github.com/apache/doris/pull/14201#discussion_r1020366774
##########
fe/fe-core/src/main/cup/sql_parser.cup:
##########
@@ -3605,9 +3605,9 @@ show_param ::=
RESULT = new ShowTableStatsStmt(tbl, partitionNames);
:}
/* show column stats */
- | KW_COLUMN KW_STATS table_name:tbl opt_partition_names:partitionNames
+ | KW_COLUMN KW_STATS table_name:tbl
{:
- RESULT = new ShowColumnStatsStmt(tbl, partitionNames);
+ RESULT = new ShowColumnStatsStmt(tbl);
Review Comment:
why remove partition level column stats?
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/ShowColumnStatsStmt.java:
##########
@@ -18,58 +18,69 @@
package org.apache.doris.analysis;
import org.apache.doris.catalog.Column;
+import org.apache.doris.catalog.Database;
+import org.apache.doris.catalog.Env;
import org.apache.doris.catalog.ScalarType;
+import org.apache.doris.catalog.TableIf;
+import org.apache.doris.common.ErrorReport;
+import org.apache.doris.common.Pair;
import org.apache.doris.common.UserException;
import org.apache.doris.common.util.Util;
+import org.apache.doris.datasource.CatalogIf;
+import org.apache.doris.qe.ShowResultSet;
import org.apache.doris.qe.ShowResultSetMetaData;
-import org.apache.doris.statistics.ColumnStat;
+import org.apache.doris.statistics.ColumnStatistic;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
-import java.util.Collections;
import java.util.List;
public class ShowColumnStatsStmt extends ShowStmt {
private static final ImmutableList<String> TITLE_NAMES =
new ImmutableList.Builder<String>()
.add("column_name")
- .add(ColumnStat.NDV.getValue())
- .add(ColumnStat.AVG_SIZE.getValue())
- .add(ColumnStat.MAX_SIZE.getValue())
- .add(ColumnStat.NUM_NULLS.getValue())
- .add(ColumnStat.MIN_VALUE.getValue())
- .add(ColumnStat.MAX_VALUE.getValue())
+ .add("count")
+ .add("ndv")
+ .add("num_null")
+ .add("data_size")
+ .add("avg_size_byte")
+ .add("min")
+ .add("max")
+ .add("min_expr")
+ .add("max_expr")
.build();
private final TableName tableName;
- private final PartitionNames partitionNames;
- public ShowColumnStatsStmt(TableName tableName, PartitionNames
partitionNames) {
+ private TableIf table;
+
+ public ShowColumnStatsStmt(TableName tableName) {
this.tableName = tableName;
- this.partitionNames = partitionNames;
}
public TableName getTableName() {
return tableName;
}
- public List<String> getPartitionNames() {
- if (partitionNames == null) {
- return Collections.emptyList();
- }
- return partitionNames.getPartitionNames();
- }
-
@Override
public void analyze(Analyzer analyzer) throws UserException {
super.analyze(analyzer);
tableName.analyze(analyzer);
// disallow external catalog
Util.prohibitExternalCatalog(tableName.getCtl(),
this.getClass().getSimpleName());
-
- if (partitionNames != null) {
- partitionNames.analyze(analyzer);
+ CatalogIf catalog =
Env.getCurrentEnv().getCatalogMgr().getCatalog(tableName.getCtl());
+ if (catalog == null) {
+ ErrorReport.reportAnalysisException("Catalog: {} not exists",
tableName.getCtl());
+ }
+ Database db =
Env.getCurrentEnv().getInternalCatalog().getDb(tableName.getDb()).orElse(null);
Review Comment:
catalog not used anynore
##########
fe/fe-core/src/main/java/org/apache/doris/qe/ShowExecutor.java:
##########
@@ -2136,9 +2140,21 @@ private void handleShowTableStats() throws
AnalysisException {
private void handleShowColumnStats() throws AnalysisException {
ShowColumnStatsStmt showColumnStatsStmt = (ShowColumnStatsStmt) stmt;
- List<List<String>> results = Env.getCurrentEnv().getStatisticsManager()
- .showColumnStatsList(showColumnStatsStmt);
- resultSet = new ShowResultSet(showColumnStatsStmt.getMetaData(),
results);
+ TableName tableName = showColumnStatsStmt.getTableName();
+ TableIf tableIf = showColumnStatsStmt.getTable();
+ if (!Env.getCurrentEnv().getAuth()
+ .checkTblPriv(ConnectContext.get(), tableName.getDb(),
tableName.getTbl(), PrivPredicate.SHOW)) {
+
ErrorReport.reportAnalysisException(ErrorCode.ERR_TABLEACCESS_DENIED_ERROR,
"SHOW CREATE TABLE",
Review Comment:
show create table?
##########
fe/fe-core/src/main/java/org/apache/doris/qe/ShowExecutor.java:
##########
@@ -587,7 +591,7 @@ private void handleShowMigrations() throws
AnalysisException {
for (BaseParam param : infos) {
final int percent = (int) (param.getFloatParam(0) * 100f);
rows.add(Lists.newArrayList(param.getStringParam(0),
param.getStringParam(1), param.getStringParam(2),
- String.valueOf(percent + "%")));
+ String.valueOf(percent + "%")));
Review Comment:
do not mod these irrelevant code in these PR
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/TableIf.java:
##########
@@ -173,5 +175,17 @@ public String toMysqlType() {
}
}
}
+
+ default List<Column> getColumns() {
+ return Collections.emptyList();
+ }
+
+ default Set<String> getPartitionNames() {
+ return Collections.emptySet();
+ }
+
+ default Partition getPartition(String name) {
+ return null;
+ }
Review Comment:
i don't think Partition is a common enough struct of partition for all type
of table. PTAL @morningman
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticConstants.java:
##########
@@ -27,4 +29,27 @@ public class StatisticConstants {
public static final int MAX_NAME_LEN = 64;
public static final int ID_LEN = 4096;
+
+ public static final int STATISTIC_PARALLEL_EXEC_INSTANCE_NUM = 1;
+
+ public static final int
STATISTICS_OUTDATED_RECORD_DETECTOR_RUNNING_INTERVAL_IN_MINUTES = 5;
+
+ public static final int STATISTICS_CACHE_VALID_DURATION_IN_HOURS = 24 * 2;
+
+ public static final int STATISTICS_CACHE_REFRESH_INTERVAL = 24 * 2;
+
+ public static final int STATISTIC_TABLE_BUCKET_COUNT = 7;
+
+ public static final long STATISTICS_MAX_MEM_PER_QUERY_IN_BYTES = 2L * 1024
* 1024 * 1024;
+
+ public static final int STATISTIC_CLEAN_INTERVAL_IN_HOURS = 24 * 2;
+
+ public static final int
STATISTICS_TABLE_CREATION_RETRY_INTERVAL_IN_SECONDS = 5;
+
+ public static final int STATISTICS_RECORDS_OUTDATED_TIME_IN_MS = 2 * 24 *
3600 * 1000;
+
+ public static final long STATISTICS_RECORDS_CACHE_SIZE = 100000;
+
+ public static final long STATISTICS_TASKS_TIMEOUT_IN_MS =
TimeUnit.MINUTES.toMillis(10);
Review Comment:
add comment plz
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticStorageInitializer.java:
##########
@@ -57,7 +58,8 @@ public void run() {
}
while (true) {
try {
-
Thread.currentThread().join(Config.statistics_table_creation_retry_interval_in_seconds
* 1000L);
+ Thread.currentThread()
+
.join(StatisticConstants.STATISTICS_TABLE_CREATION_RETRY_INTERVAL_IN_SECONDS *
1000L);
createDB();
Review Comment:
@morningman do we have any other internal table need to save in future? if
the anwser is yes, i think we should
1. rename the database to a common name
2. mv this class to another place and rename it
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticStorageInitializer.java:
##########
@@ -110,7 +112,8 @@ public CreateTableStmt buildStatisticsTblStmt() throws
UserException {
KeysDesc keysDesc = new KeysDesc(KeysType.UNIQUE_KEYS,
Lists.newArrayList("id"));
- DistributionDesc distributionDesc = new
HashDistributionDesc(Config.statistic_table_bucket_count,
+ DistributionDesc distributionDesc = new
HashDistributionDesc(StatisticConstants
+ .STATISTIC_TABLE_BUCKET_COUNT,
Review Comment:
```suggestion
DistributionDesc distributionDesc = new HashDistributionDesc(
StatisticConstants.STATISTIC_TABLE_BUCKET_COUNT,
```
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticsTableCleaner.java:
##########
@@ -45,7 +45,8 @@ public class StatisticsTableCleaner extends MasterDaemon {
private static final Logger LOG =
LogManager.getLogger(StatisticsTableCleaner.class);
public StatisticsTableCleaner() {
- super("Statistics Table Cleaner", (long)
Config.statistic_clean_interval_in_hours * 3600 * 1000);
+ super("Statistics Table Cleaner",
+ (long) StatisticConstants.STATISTIC_CLEAN_INTERVAL_IN_HOURS *
3600 * 1000);
Review Comment:
```suggestion
StatisticConstants.STATISTIC_CLEAN_INTERVAL_IN_HOURS * 3600
* 1000L);
```
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/StatisticStorageInitializer.java:
##########
@@ -145,7 +148,7 @@ public CreateTableStmt buildAnalysisJobTblStmt() throws
UserException {
KeysDesc keysDesc = new KeysDesc(KeysType.UNIQUE_KEYS,
Lists.newArrayList("job_id"));
- DistributionDesc distributionDesc = new
HashDistributionDesc(Config.statistic_table_bucket_count,
+ DistributionDesc distributionDesc = new
HashDistributionDesc(StatisticConstants.STATISTIC_TABLE_BUCKET_COUNT,
Review Comment:
```suggestion
DistributionDesc distributionDesc = new HashDistributionDesc(
StatisticConstants.STATISTIC_TABLE_BUCKET_COUNT,
```
##########
fe/fe-core/src/test/java/org/apache/doris/cluster/DecommissionBackendTest.java:
##########
@@ -83,7 +84,7 @@ public void testDecommissionBackend() throws Exception {
}
Assertions.assertEquals(backendNum() - 1,
Env.getCurrentSystemInfo().getIdToBackend().size());
- Assertions.assertEquals(tabletNum +
Config.statistic_table_bucket_count * 2,
+ Assertions.assertEquals(tabletNum +
StatisticConstants.STATISTIC_TABLE_BUCKET_COUNT * 2,
Review Comment:
add comment to explain why need *2
--
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
