morrySnow commented on code in PR #40680:
URL: https://github.com/apache/doris/pull/40680#discussion_r1948879175
##########
fe/fe-core/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4:
##########
@@ -185,6 +198,9 @@ supportedCreateStatement
| CREATE (EXTERNAL)? TABLE (IF NOT EXISTS)? name=multipartIdentifier
LIKE existedTable=multipartIdentifier
(WITH ROLLUP (rollupNames=identifierList)?)?
#createTableLike
+ | CREATE (TEMPORARY)? TABLE (IF NOT EXISTS)? name=multipartIdentifier
Review Comment:
same as create table
##########
fe/fe-core/src/main/antlr4/org/apache/doris/nereids/DorisParser.g4:
##########
@@ -177,6 +177,19 @@ supportedCreateStatement
properties=propertyClause?
(BROKER extProperties=propertyClause)?
(AS query)?
#createTable
+ | CREATE (TEMPORARY)? TABLE (IF NOT EXISTS)? name=multipartIdentifier
Review Comment:
remove this new branch
```
(EXTERNAL | TEMPORARY)?
```
##########
fe/fe-core/src/main/java/org/apache/doris/binlog/BinlogManager.java:
##########
@@ -154,7 +154,11 @@ private void addBinlog(long dbId, List<Long> tableIds,
long commitSeq, long time
if (tableIds != null) {
for (long tableId : tableIds) {
boolean tableBinlogEnable =
binlogConfigCache.isEnableTable(dbId, tableId);
- anyEnable = anyEnable || tableBinlogEnable;
+ if (tableIds.size() > 1) {
+ anyEnable = anyEnable || tableBinlogEnable;
+ } else {
+ anyEnable = tableBinlogEnable;
+ }
Review Comment:
i think this is code of temporary table, it is better to submit another PR
to solve this problem
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/DatabaseIf.java:
##########
@@ -167,6 +183,17 @@ default T getTableOrMetaException(String tableName,
TableIf.TableType tableType)
return table;
}
+ default T getNonTempTableOrMetaException(String tableName,
TableIf.TableType tableType)
+ throws MetaNotFoundException {
+ T table = getNonTempTableOrMetaException(tableName);
+ TableType type = Objects.requireNonNull(table.getType());
Review Comment:
add error message
##########
fe/fe-core/src/main/java/org/apache/doris/backup/BackupHandler.java:
##########
@@ -435,14 +436,27 @@ private void backup(Repository repository, Database db,
BackupStmt stmt) throws
if (Config.ignore_backup_not_support_table_type) {
LOG.warn("Table '{}' is a {} table, can not backup and
ignore it."
+ "Only OLAP(Doris)/ODBC/VIEW table can be backed
up",
- tblName, tbl.getType().toString());
+ tblName, tbl.isTemporary() ? "temporary" :
tbl.getType().toString());
tblRefsNotSupport.add(tblRef);
continue;
} else {
ErrorReport.reportDdlException(ErrorCode.ERR_NOT_OLAP_TABLE, tblName);
}
}
+ if (tbl.isTemporary()) {
+ if (Config.ignore_backup_not_support_table_type) {
+ LOG.warn("Table '{}' is a temporary table, can not backup
and ignore it."
+ + "Only OLAP(Doris)/ODBC/VIEW table can be backed
up",
+ Util.getTempTableDisplayName(tblName));
+ tblRefsNotSupport.add(tblRef);
+ continue;
+ } else {
+ ErrorReport.reportDdlException("Table " +
Util.getTempTableDisplayName(tblName)
Review Comment:
i think backup should always ignore temporary table and should not failed
even if has temporary table in current Cluster
##########
fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java:
##########
@@ -127,7 +127,7 @@ public void
processCreateMaterializedView(CreateMaterializedViewStmt stmt)
Database db =
Env.getCurrentInternalCatalog().getDbOrDdlException(dbName);
Env.getCurrentInternalCatalog().checkAvailableCapacity(db);
- OlapTable olapTable = (OlapTable)
db.getTableOrMetaException(tableName, TableType.OLAP);
+ OlapTable olapTable = (OlapTable)
db.getNonTempTableOrMetaException(tableName, TableType.OLAP);
Review Comment:
why temporary table could not create materialized view?
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Database.java:
##########
@@ -381,6 +382,19 @@ public void checkQuota() throws DdlException {
checkReplicaQuota();
}
+ public boolean isTableExist(String tableName, boolean isTemporary) {
+ if (Env.isTableNamesCaseInsensitive()) {
+ tableName = tableName.toLowerCase();
+ }
+
+ if (isTemporary) {
+ Set<String> tableSet =
ConnectContext.get().getDbToTempTableNamesMap().get(fullQualifiedName);
+ return tableSet != null && tableSet.contains(tableName);
+ } else {
+ return nameToTable.containsKey(tableName);
+ }
+ }
+
public boolean isTableExist(String tableName) {
Review Comment:
should call `isTableExist(String tableName, boolean isTemporary)` with false
as second parameter
##########
fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java:
##########
@@ -127,7 +127,7 @@ public void
processCreateMaterializedView(CreateMaterializedViewStmt stmt)
Database db =
Env.getCurrentInternalCatalog().getDbOrDdlException(dbName);
Env.getCurrentInternalCatalog().checkAvailableCapacity(db);
- OlapTable olapTable = (OlapTable)
db.getTableOrMetaException(tableName, TableType.OLAP);
+ OlapTable olapTable = (OlapTable)
db.getNonTempTableOrMetaException(tableName, TableType.OLAP);
Review Comment:
if not support create sync materialized view on temporary table, we should
also forbid create async materialized view on it
##########
fe/fe-core/src/main/java/org/apache/doris/statistics/AnalysisManager.java:
##########
@@ -206,6 +207,9 @@ public List<AnalysisInfo>
buildAnalysisInfosForDB(DatabaseIf<TableIf> db, Analyz
if (table instanceof View) {
continue;
}
+ if (table.isTemporary()) {
Review Comment:
not analyze temporary table maybe not a good idea
##########
fe/fe-core/src/main/java/org/apache/doris/policy/PolicyMgr.java:
##########
@@ -179,12 +180,16 @@ public void dropPolicy(DropPolicyLog dropPolicyLog,
boolean ifExists) throws Ddl
for (Table table : tables) {
if (table instanceof OlapTable) {
OlapTable olapTable = (OlapTable) table;
+ String tableName = table.getName();
+ if (table.isTemporary()) {
+ tableName =
Util.getTempTableDisplayName(tableName);
+ }
Review Comment:
why not Util provide add new function to process Table object and return
right name
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Database.java:
##########
@@ -381,6 +382,19 @@ public void checkQuota() throws DdlException {
checkReplicaQuota();
}
+ public boolean isTableExist(String tableName, boolean isTemporary) {
+ if (Env.isTableNamesCaseInsensitive()) {
+ tableName = tableName.toLowerCase();
Review Comment:
this is different with original `isTableExist`
```
if (Env.isTableNamesCaseInsensitive()) {
tableName = lowerCaseToTableName.get(tableName.toLowerCase());
if (tableName == null) {
return false;
}
}
```
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/Database.java:
##########
@@ -569,7 +589,33 @@ public Table getTableNullable(String tableName) {
return null;
}
}
- return nameToTable.get(tableName);
+
+ // return temp table first
+ Table table =
nameToTable.get(Util.generateTempTableInnerName(tableName));
Review Comment:
so temporary table's name could same with permanent table?
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/ShowPartitionIdCommand.java:
##########
@@ -84,13 +90,22 @@ private ShowResultSet handleShowPartitionId(ConnectContext
ctx, StmtExecutor exe
if (partition != null) {
List<String> row = new ArrayList<>();
row.add(database.getFullName());
- row.add(tbl.getName());
+ if (tbl.isTemporary()) {
+ if
(!Util.isTempTableInCurrentSession(tbl.getName())) {
+ continue;
+ }
+
row.add(Util.getTempTableDisplayName(tbl.getName()));
Review Comment:
could table provide a function to return display name directly?
--
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]
