From af2b67e65a5679d710a3f82d7bf00017df0a9f34 Mon Sep 17 00:00:00 2001
From: Xiangyu Wang <dut.xiangyu@gmail.com>
Date: Sun, 25 Jun 2023 19:14:44 +0800
Subject: [PATCH] [Fix](multi-catalog) Invalidate cache when enable auto
 refresh catalog. (#21070)

The default value of RefreshCatalogStmt.invalidCache is false now, but the RefreshManager.RefreshTask does not invoke RefreshCatalogStmt.analyze() so it will not invalidate the cache. This pr mainly fix this problem
---
 .../apache/doris/analysis/RefreshCatalogStmt.java  | 14 ++++++++++----
 .../org/apache/doris/catalog/RefreshManager.java   |  5 +++++
 .../org/apache/doris/catalog/RefreshTableTest.java |  2 ++
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/RefreshCatalogStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/RefreshCatalogStmt.java
index 5cc5baa0de..006eebe44b 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/RefreshCatalogStmt.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/RefreshCatalogStmt.java
@@ -38,7 +38,12 @@ public class RefreshCatalogStmt extends DdlStmt {
 
     private final String catalogName;
     private Map<String, String> properties;
-    private boolean invalidCache = false;
+
+    /**
+     * Set default value to true, otherwise
+     * {@link org.apache.doris.catalog.RefreshManager.RefreshTask} will lost the default value
+     */
+    private boolean invalidCache = true;
 
     public RefreshCatalogStmt(String catalogName, Map<String, String> properties) {
         this.catalogName = catalogName;
@@ -66,9 +71,9 @@ public class RefreshCatalogStmt extends DdlStmt {
             ErrorReport.reportAnalysisException(ErrorCode.ERR_CATALOG_ACCESS_DENIED,
                     analyzer.getQualifiedUser(), catalogName);
         }
-        String invalidConfig = properties == null ? null : properties.get(INVALID_CACHE);
-        // Default is to invalid cache.
-        invalidCache = invalidConfig == null ? true : invalidConfig.equalsIgnoreCase("true");
+
+        // Set to false only if user set the property "invalid_cache"="false"
+        invalidCache = !(properties != null && properties.get(INVALID_CACHE).equalsIgnoreCase("false"));
     }
 
     @Override
@@ -77,4 +82,5 @@ public class RefreshCatalogStmt extends DdlStmt {
         stringBuilder.append("REFRESH CATALOG ").append("`").append(catalogName).append("`");
         return stringBuilder.toString();
     }
+
 }
diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java
index f108f0f301..c72d3ffb4d 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/RefreshManager.java
@@ -189,6 +189,11 @@ public class RefreshManager {
                     CatalogIf catalog = Env.getCurrentEnv().getCatalogMgr().getCatalog(catalogId);
                     if (catalog != null) {
                         String catalogName = catalog.getName();
+                        /**
+                         * Now do not invoke
+                         * {@link org.apache.doris.analysis.RefreshCatalogStmt#analyze(Analyzer)} is ok,
+                         * because the default value of invalidCache is true.
+                         * */
                         RefreshCatalogStmt refreshCatalogStmt = new RefreshCatalogStmt(catalogName, null);
                         try {
                             DdlExecutor.execute(Env.getCurrentEnv(), refreshCatalogStmt);
diff --git a/fe/fe-core/src/test/java/org/apache/doris/catalog/RefreshTableTest.java b/fe/fe-core/src/test/java/org/apache/doris/catalog/RefreshTableTest.java
index 1d8965c813..11e37ce238 100644
--- a/fe/fe-core/src/test/java/org/apache/doris/catalog/RefreshTableTest.java
+++ b/fe/fe-core/src/test/java/org/apache/doris/catalog/RefreshTableTest.java
@@ -76,6 +76,7 @@ public class RefreshTableTest extends TestWithFeService {
         table.makeSureInitialized();
         Assertions.assertTrue(table.isObjectCreated());
         RefreshCatalogStmt refreshCatalogStmt = new RefreshCatalogStmt("test1", null);
+        Assertions.assertTrue(refreshCatalogStmt.isInvalidCache());
         try {
             DdlExecutor.execute(Env.getCurrentEnv(), refreshCatalogStmt);
         } catch (Exception e) {
@@ -97,6 +98,7 @@ public class RefreshTableTest extends TestWithFeService {
         } catch (Exception e) {
             // Do nothing
         }
+        Assertions.assertFalse(((ExternalCatalog) test1).isInitialized());
     }
 
     @Test