morningman commented on code in PR #39532:
URL: https://github.com/apache/doris/pull/39532#discussion_r1728315747
##########
fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java:
##########
@@ -453,31 +475,37 @@ private void processModifyEngineInternal(Database db,
Table externalTable,
public void processAlterTable(AlterTableStmt stmt) throws UserException {
TableName dbTableName = stmt.getTbl();
+ String ctlName = dbTableName.getCtl();
String dbName = dbTableName.getDb();
String tableName = dbTableName.getTbl();
- Database db =
Env.getCurrentInternalCatalog().getDbOrDdlException(dbName);
- Table table = db.getTableOrDdlException(tableName);
+ DatabaseIf dbIf =
Env.getCurrentEnv().getCatalogMgr().getCatalog(ctlName).getDbOrDdlException(dbName);
+ TableIf tableIf = dbIf.getTableOrDdlException(tableName);
List<AlterClause> alterClauses = Lists.newArrayList();
// some operations will take long time to process, need to be done
outside the table lock
boolean needProcessOutsideTableLock = false;
- switch (table.getType()) {
+ switch (tableIf.getType()) {
case MATERIALIZED_VIEW:
case OLAP:
- OlapTable olapTable = (OlapTable) table;
- needProcessOutsideTableLock = processAlterOlapTable(stmt,
olapTable, alterClauses, db);
+ OlapTable olapTable = (OlapTable) tableIf;
+ needProcessOutsideTableLock = processAlterOlapTable(stmt,
olapTable, alterClauses, (Database) dbIf);
break;
case ODBC:
case JDBC:
case HIVE:
case MYSQL:
case ELASTICSEARCH:
- processAlterExternalTable(stmt, table, db);
+ processAlterExternalTable(stmt, (Table) tableIf, (Database)
dbIf);
+ return;
+ case HMS_EXTERNAL_TABLE:
+ alterClauses.addAll(stmt.getOps());
Review Comment:
How to make sure that the `AlterClause` must be
`ModifyTablePropertiesClause`?
Because you check this in `setHMSExternalTableAutoAnalyzePolicy()`
##########
fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalTable.java:
##########
@@ -69,6 +69,8 @@ public class ExternalTable implements TableIf, Writable,
GsonPostProcessable {
protected long timestamp;
@SerializedName(value = "dbName")
protected String dbName;
+ @SerializedName(value = "aap")
Review Comment:
ExternalTable will not be persist anymore.
You can try it on master branch by:
1. create catalog
2. set a hive table's property
3. restart FE
4. check if that table still has that property
##########
fe/fe-core/src/main/java/org/apache/doris/analysis/ModifyTablePropertiesClause.java:
##########
@@ -350,6 +350,19 @@ public void analyze(Analyzer analyzer) throws
AnalysisException {
// do nothing, will be analyzed when creating alter job
} else if
(properties.containsKey(PropertyAnalyzer.PROPERTIES_ROW_STORE_COLUMNS)) {
// do nothing, will be analyzed when creating alter job
+ } else if
(properties.containsKey(PropertyAnalyzer.PROPERTIES_AUTO_ANALYZE_POLICY)) {
+ String analyzePolicy =
properties.getOrDefault(PropertyAnalyzer.PROPERTIES_AUTO_ANALYZE_POLICY, "");
+ if (analyzePolicy != null
+ &&
!analyzePolicy.equals(PropertyAnalyzer.ENABLE_AUTO_ANALYZE_POLICY)
+ &&
!analyzePolicy.equals(PropertyAnalyzer.DISABLE_AUTO_ANALYZE_POLICY)
+ &&
!analyzePolicy.equals(PropertyAnalyzer.NONE_AUTO_ANALYZE_POLICY)) {
+ throw new AnalysisException(
+ "Table auto analyze policy only support for " +
PropertyAnalyzer.ENABLE_AUTO_ANALYZE_POLICY
+ + " or " + PropertyAnalyzer.DISABLE_AUTO_ANALYZE_POLICY
+ + " or " + PropertyAnalyzer.NONE_AUTO_ANALYZE_POLICY);
Review Comment:
I think better change "none" to "base_on_system" or "base_on_config".
"none" is meaningless
--
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
