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: 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