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

Reply via email to