morrySnow commented on code in PR #11030:
URL: https://github.com/apache/doris/pull/11030#discussion_r937306232
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AlterColumnStatsStmt.java:
##########
@@ -47,59 +60,124 @@ public class AlterColumnStatsStmt extends DdlStmt {
.add(ColumnStats.MAX_VALUE)
.build();
- private TableName tableName;
- private String columnName;
- private Map<String, String> properties;
- public final Map<StatsType, String> statsTypeToValue = Maps.newHashMap();
+ private final TableName tableName;
+ private final PartitionNames optPartitionNames;
+ private final String columnName;
+ private final Map<String, String> properties;
- public AlterColumnStatsStmt(TableName tableName, String columnName,
Map<String, String> properties) {
+ private final List<String> partitionNames = Lists.newArrayList();
+ private final Map<StatsType, String> statsTypeToValue = Maps.newHashMap();
+
+ public AlterColumnStatsStmt(TableName tableName, String columnName,
+ Map<String, String> properties, PartitionNames
optPartitionNames) {
this.tableName = tableName;
this.columnName = columnName;
- this.properties = properties;
+ this.properties = properties == null ? Maps.newHashMap() : properties;
+ this.optPartitionNames = optPartitionNames;
+ }
+
+ public TableName getTableName() {
+ return tableName;
+ }
+
+ public String getColumnName() {
+ return columnName;
+ }
+
+ public List<String> getPartitionNames() {
+ return partitionNames;
+ }
+
+ public Map<StatsType, String> getStatsTypeToValue() {
+ return statsTypeToValue;
}
@Override
public void analyze(Analyzer analyzer) throws UserException {
super.analyze(analyzer);
+
// check table name
tableName.analyze(analyzer);
+
// disallow external catalog
Util.prohibitExternalCatalog(tableName.getCtl(),
this.getClass().getSimpleName());
+
+ // check partition & column
+ checkPartitionAndColumnNames();
+
// check properties
Optional<StatsType> optional =
properties.keySet().stream().map(StatsType::fromString)
- .filter(statsType ->
!CONFIGURABLE_PROPERTIES_SET.contains(statsType)).findFirst();
+ .filter(statsType ->
!CONFIGURABLE_PROPERTIES_SET.contains(statsType))
+ .findFirst();
if (optional.isPresent()) {
- throw new AnalysisException(optional.get() + " is invalid
statistic");
+ throw new AnalysisException(optional.get() + " is invalid
statistics");
}
+
// check auth
- if (!Env.getCurrentEnv().getAuth().checkTblPriv(
- ConnectContext.get(), tableName.getDb(), tableName.getTbl(),
PrivPredicate.ALTER)) {
+ if (!Env.getCurrentEnv().getAuth()
+ .checkTblPriv(ConnectContext.get(), tableName.getDb(),
tableName.getTbl(), PrivPredicate.ALTER)) {
ErrorReport.reportAnalysisException(ErrorCode.ERR_TABLEACCESS_DENIED_ERROR,
"ALTER COLUMN STATS",
- ConnectContext.get().getQualifiedUser(),
- ConnectContext.get().getRemoteIP(),
+ ConnectContext.get().getQualifiedUser(),
ConnectContext.get().getRemoteIP(),
tableName.getDb() + ": " + tableName.getTbl());
}
+
// get statsTypeToValue
properties.forEach((key, value) -> {
StatsType statsType = StatsType.fromString(key);
statsTypeToValue.put(statsType, value);
});
}
- public TableName getTableName() {
- return tableName;
- }
+ private void checkPartitionAndColumnNames() throws AnalysisException {
Review Comment:
add a todo to support external catalog and external table
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AlterTableStatsStmt.java:
##########
@@ -35,61 +39,125 @@
import java.util.List;
import java.util.Map;
import java.util.Optional;
+import java.util.Set;
+/**
+ * Manually inject statistics for tables or partitions.
+ * Only OLAP table statistics are supported.
+ * e.g.
+ * ALTER TABLE table_name
+ * SET STATS ('k1' = 'v1', ...) [ PARTITIONS(p_name1, p_name2...) ]
+ */
public class AlterTableStatsStmt extends DdlStmt {
private static final ImmutableSet<StatsType> CONFIGURABLE_PROPERTIES_SET =
new ImmutableSet.Builder<StatsType>()
.add(TableStats.DATA_SIZE)
.add(TableStats.ROW_COUNT)
.build();
- private TableName tableName;
- private Map<String, String> properties;
- public final Map<StatsType, String> statsTypeToValue = Maps.newHashMap();
+ private final TableName tableName;
+ private final PartitionNames optPartitionNames;
+ private final Map<String, String> properties;
- public AlterTableStatsStmt(TableName tableName, Map<String, String>
properties) {
+ private final List<String> partitionNames = Lists.newArrayList();
+ private final Map<StatsType, String> statsTypeToValue = Maps.newHashMap();
+
+ public AlterTableStatsStmt(TableName tableName, Map<String, String>
properties,
+ PartitionNames optPartitionNames) {
this.tableName = tableName;
- this.properties = properties;
+ this.properties = properties == null ? Maps.newHashMap() : properties;
+ this.optPartitionNames = optPartitionNames;
+ }
+
+ public TableName getTableName() {
+ return tableName;
+ }
+
+ public List<String> getPartitionNames() {
+ return partitionNames;
+ }
+
+ public Map<StatsType, String> getStatsTypeToValue() {
+ return statsTypeToValue;
}
@Override
public void analyze(Analyzer analyzer) throws UserException {
super.analyze(analyzer);
+
// check table name
tableName.analyze(analyzer);
+
// disallow external catalog
Util.prohibitExternalCatalog(tableName.getCtl(),
this.getClass().getSimpleName());
+
+ // check partition
+ checkPartitionNames();
+
// check properties
Optional<StatsType> optional =
properties.keySet().stream().map(StatsType::fromString)
- .filter(statsType ->
!CONFIGURABLE_PROPERTIES_SET.contains(statsType)).findFirst();
+ .filter(statsType ->
!CONFIGURABLE_PROPERTIES_SET.contains(statsType))
+ .findFirst();
if (optional.isPresent()) {
- throw new AnalysisException(optional.get() + " is invalid
statistic");
+ throw new AnalysisException(optional.get() + " is invalid
statistics");
}
+
// check auth
- if (!Env.getCurrentEnv().getAuth().checkTblPriv(
- ConnectContext.get(), tableName.getDb(), tableName.getTbl(),
PrivPredicate.ALTER)) {
+ if (!Env.getCurrentEnv().getAuth()
+ .checkTblPriv(ConnectContext.get(), tableName.getDb(),
tableName.getTbl(), PrivPredicate.ALTER)) {
ErrorReport.reportAnalysisException(ErrorCode.ERR_TABLEACCESS_DENIED_ERROR,
"ALTER TABLE STATS",
- ConnectContext.get().getQualifiedUser(),
- ConnectContext.get().getRemoteIP(),
+ ConnectContext.get().getQualifiedUser(),
ConnectContext.get().getRemoteIP(),
tableName.getDb() + ": " + tableName.getTbl());
}
+
// get statsTypeToValue
properties.forEach((key, value) -> {
StatsType statsType = StatsType.fromString(key);
statsTypeToValue.put(statsType, value);
});
}
- public TableName getTableName() {
- return tableName;
- }
+ private void checkPartitionNames() throws AnalysisException {
+ if (optPartitionNames != null) {
+ optPartitionNames.analyze(analyzer);
+ Database db = analyzer.getEnv().getInternalDataSource()
+ .getDbOrAnalysisException(tableName.getDb());
+ Table table = db.getTableOrAnalysisException(tableName.getTbl());
- public Map<StatsType, String> getStatsTypeToValue() {
- return statsTypeToValue;
+ if (table.getType() != Table.TableType.OLAP) {
+ throw new AnalysisException("Only OLAP table statistics are
supported");
+ }
+
+ OlapTable olapTable = (OlapTable) table;
+
+ if (olapTable.isPartitioned()) {
+ List<String> names = optPartitionNames.getPartitionNames();
+ Set<String> olapPartitionNames = olapTable.getPartitionNames();
+ Optional<String> optional = names.stream()
+ .filter(name ->
!olapPartitionNames.contains(name)).findFirst();
+ if (optional.isPresent()) {
+ throw new AnalysisException("Partition does not exist: " +
optional.get());
+ }
+ partitionNames.addAll(optPartitionNames.getPartitionNames());
+ } else {
+ throw new AnalysisException("Not a partitioned table: " +
olapTable.getName());
+ }
+ }
Review Comment:
should we need to handle table is unpartitioned and optPartitionNames is not
empty just like AlterColumnStats?
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AlterColumnStatsStmt.java:
##########
@@ -47,59 +60,124 @@ public class AlterColumnStatsStmt extends DdlStmt {
.add(ColumnStats.MAX_VALUE)
.build();
- private TableName tableName;
- private String columnName;
- private Map<String, String> properties;
- public final Map<StatsType, String> statsTypeToValue = Maps.newHashMap();
+ private final TableName tableName;
+ private final PartitionNames optPartitionNames;
+ private final String columnName;
+ private final Map<String, String> properties;
- public AlterColumnStatsStmt(TableName tableName, String columnName,
Map<String, String> properties) {
+ private final List<String> partitionNames = Lists.newArrayList();
+ private final Map<StatsType, String> statsTypeToValue = Maps.newHashMap();
+
+ public AlterColumnStatsStmt(TableName tableName, String columnName,
+ Map<String, String> properties, PartitionNames
optPartitionNames) {
this.tableName = tableName;
this.columnName = columnName;
- this.properties = properties;
+ this.properties = properties == null ? Maps.newHashMap() : properties;
Review Comment:
```suggestion
this.properties = properties == null ? Collections.emptyMap() :
properties;
```
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AlterColumnStatsStmt.java:
##########
@@ -47,59 +60,124 @@ public class AlterColumnStatsStmt extends DdlStmt {
.add(ColumnStats.MAX_VALUE)
.build();
- private TableName tableName;
- private String columnName;
- private Map<String, String> properties;
- public final Map<StatsType, String> statsTypeToValue = Maps.newHashMap();
+ private final TableName tableName;
+ private final PartitionNames optPartitionNames;
+ private final String columnName;
+ private final Map<String, String> properties;
- public AlterColumnStatsStmt(TableName tableName, String columnName,
Map<String, String> properties) {
+ private final List<String> partitionNames = Lists.newArrayList();
+ private final Map<StatsType, String> statsTypeToValue = Maps.newHashMap();
+
+ public AlterColumnStatsStmt(TableName tableName, String columnName,
+ Map<String, String> properties, PartitionNames
optPartitionNames) {
this.tableName = tableName;
this.columnName = columnName;
- this.properties = properties;
+ this.properties = properties == null ? Maps.newHashMap() : properties;
+ this.optPartitionNames = optPartitionNames;
+ }
+
+ public TableName getTableName() {
+ return tableName;
+ }
+
+ public String getColumnName() {
+ return columnName;
+ }
+
+ public List<String> getPartitionNames() {
+ return partitionNames;
+ }
+
+ public Map<StatsType, String> getStatsTypeToValue() {
+ return statsTypeToValue;
}
@Override
public void analyze(Analyzer analyzer) throws UserException {
super.analyze(analyzer);
+
// check table name
tableName.analyze(analyzer);
+
// disallow external catalog
Util.prohibitExternalCatalog(tableName.getCtl(),
this.getClass().getSimpleName());
+
+ // check partition & column
+ checkPartitionAndColumnNames();
+
// check properties
Optional<StatsType> optional =
properties.keySet().stream().map(StatsType::fromString)
- .filter(statsType ->
!CONFIGURABLE_PROPERTIES_SET.contains(statsType)).findFirst();
+ .filter(statsType ->
!CONFIGURABLE_PROPERTIES_SET.contains(statsType))
+ .findFirst();
if (optional.isPresent()) {
- throw new AnalysisException(optional.get() + " is invalid
statistic");
+ throw new AnalysisException(optional.get() + " is invalid
statistics");
}
+
// check auth
- if (!Env.getCurrentEnv().getAuth().checkTblPriv(
- ConnectContext.get(), tableName.getDb(), tableName.getTbl(),
PrivPredicate.ALTER)) {
+ if (!Env.getCurrentEnv().getAuth()
+ .checkTblPriv(ConnectContext.get(), tableName.getDb(),
tableName.getTbl(), PrivPredicate.ALTER)) {
ErrorReport.reportAnalysisException(ErrorCode.ERR_TABLEACCESS_DENIED_ERROR,
"ALTER COLUMN STATS",
- ConnectContext.get().getQualifiedUser(),
- ConnectContext.get().getRemoteIP(),
+ ConnectContext.get().getQualifiedUser(),
ConnectContext.get().getRemoteIP(),
tableName.getDb() + ": " + tableName.getTbl());
}
+
// get statsTypeToValue
properties.forEach((key, value) -> {
StatsType statsType = StatsType.fromString(key);
statsTypeToValue.put(statsType, value);
});
}
- public TableName getTableName() {
- return tableName;
- }
+ private void checkPartitionAndColumnNames() throws AnalysisException {
+ Database db = analyzer.getEnv().getInternalDataSource()
+ .getDbOrAnalysisException(tableName.getDb());
+ Table table = db.getTableOrAnalysisException(tableName.getTbl());
- public String getColumnName() {
- return columnName;
- }
+ if (table.getType() != Table.TableType.OLAP) {
+ throw new AnalysisException("Only OLAP table statistics are
supported");
+ }
- public Map<StatsType, String> getStatsTypeToValue() {
- return statsTypeToValue;
+ OlapTable olapTable = (OlapTable) table;
+
+ if (olapTable.getColumn(columnName) == null) {
+
ErrorReport.reportAnalysisException(ErrorCode.ERR_WRONG_COLUMN_NAME,
columnName);
+ }
+
+ if (optPartitionNames != null) {
+ optPartitionNames.analyze(analyzer);
+ if (!olapTable.isPartitioned()) {
+ throw new AnalysisException("Not a partitioned table: " +
olapTable.getName());
+ }
+ Set<String> olapPartitionNames = olapTable.getPartitionNames();
+ Optional<String> optional =
optPartitionNames.getPartitionNames().stream()
+ .filter(name ->
!olapPartitionNames.contains(name)).findFirst();
Review Comment:
it is better to use anyMatch instead of findFirst and isPresent.
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/AlterColumnStatsStmt.java:
##########
@@ -47,59 +60,124 @@ public class AlterColumnStatsStmt extends DdlStmt {
.add(ColumnStats.MAX_VALUE)
.build();
- private TableName tableName;
- private String columnName;
- private Map<String, String> properties;
- public final Map<StatsType, String> statsTypeToValue = Maps.newHashMap();
+ private final TableName tableName;
+ private final PartitionNames optPartitionNames;
+ private final String columnName;
+ private final Map<String, String> properties;
- public AlterColumnStatsStmt(TableName tableName, String columnName,
Map<String, String> properties) {
+ private final List<String> partitionNames = Lists.newArrayList();
+ private final Map<StatsType, String> statsTypeToValue = Maps.newHashMap();
+
+ public AlterColumnStatsStmt(TableName tableName, String columnName,
+ Map<String, String> properties, PartitionNames
optPartitionNames) {
this.tableName = tableName;
this.columnName = columnName;
- this.properties = properties;
+ this.properties = properties == null ? Maps.newHashMap() : properties;
+ this.optPartitionNames = optPartitionNames;
+ }
+
+ public TableName getTableName() {
+ return tableName;
+ }
+
+ public String getColumnName() {
+ return columnName;
+ }
+
+ public List<String> getPartitionNames() {
+ return partitionNames;
+ }
+
+ public Map<StatsType, String> getStatsTypeToValue() {
+ return statsTypeToValue;
}
@Override
public void analyze(Analyzer analyzer) throws UserException {
super.analyze(analyzer);
+
// check table name
tableName.analyze(analyzer);
+
// disallow external catalog
Util.prohibitExternalCatalog(tableName.getCtl(),
this.getClass().getSimpleName());
+
+ // check partition & column
+ checkPartitionAndColumnNames();
+
// check properties
Optional<StatsType> optional =
properties.keySet().stream().map(StatsType::fromString)
- .filter(statsType ->
!CONFIGURABLE_PROPERTIES_SET.contains(statsType)).findFirst();
+ .filter(statsType ->
!CONFIGURABLE_PROPERTIES_SET.contains(statsType))
+ .findFirst();
if (optional.isPresent()) {
- throw new AnalysisException(optional.get() + " is invalid
statistic");
+ throw new AnalysisException(optional.get() + " is invalid
statistics");
}
+
// check auth
- if (!Env.getCurrentEnv().getAuth().checkTblPriv(
- ConnectContext.get(), tableName.getDb(), tableName.getTbl(),
PrivPredicate.ALTER)) {
+ if (!Env.getCurrentEnv().getAuth()
+ .checkTblPriv(ConnectContext.get(), tableName.getDb(),
tableName.getTbl(), PrivPredicate.ALTER)) {
ErrorReport.reportAnalysisException(ErrorCode.ERR_TABLEACCESS_DENIED_ERROR,
"ALTER COLUMN STATS",
- ConnectContext.get().getQualifiedUser(),
- ConnectContext.get().getRemoteIP(),
+ ConnectContext.get().getQualifiedUser(),
ConnectContext.get().getRemoteIP(),
tableName.getDb() + ": " + tableName.getTbl());
}
+
// get statsTypeToValue
properties.forEach((key, value) -> {
StatsType statsType = StatsType.fromString(key);
statsTypeToValue.put(statsType, value);
});
}
- public TableName getTableName() {
- return tableName;
- }
+ private void checkPartitionAndColumnNames() throws AnalysisException {
+ Database db = analyzer.getEnv().getInternalDataSource()
+ .getDbOrAnalysisException(tableName.getDb());
+ Table table = db.getTableOrAnalysisException(tableName.getTbl());
- public String getColumnName() {
- return columnName;
- }
+ if (table.getType() != Table.TableType.OLAP) {
+ throw new AnalysisException("Only OLAP table statistics are
supported");
+ }
- public Map<StatsType, String> getStatsTypeToValue() {
- return statsTypeToValue;
+ OlapTable olapTable = (OlapTable) table;
+
+ if (olapTable.getColumn(columnName) == null) {
+
ErrorReport.reportAnalysisException(ErrorCode.ERR_WRONG_COLUMN_NAME,
columnName);
+ }
+
+ if (optPartitionNames != null) {
+ optPartitionNames.analyze(analyzer);
Review Comment:
mv L147 below to L148's if statement
--
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
