[BugFix] fix bad plan cache with invalid sql (backport #37047) (#37080)

Co-authored-by: Murphy <[email protected]>
StarRocks · Dec 16, 2023 · 3f669b6 · 3f669b6
1 parent 5502fcf
commit 3f669b6
Show file tree

Hide file tree

Showing 4 changed files with 38 additions and 19 deletions.
diff --git a/fe/fe-core/src/main/java/com/starrocks/catalog/Database.java b/fe/fe-core/src/main/java/com/starrocks/catalog/Database.java
@@ -610,6 +610,10 @@ public Table getTable(long tableId) {
         return idToTable.get(tableId);
     }
 
+    public Optional<Table> mayGetTable(long tableId) {
+        return Optional.ofNullable(getTable(tableId));
+    }
+
     public static Database read(DataInput in) throws IOException {
         Database db = new Database();
         db.readFields(in);

diff --git a/fe/fe-core/src/main/java/com/starrocks/catalog/MaterializedView.java b/fe/fe-core/src/main/java/com/starrocks/catalog/MaterializedView.java
@@ -615,6 +615,7 @@ public void copyOnlyForQuery(OlapTable olapTable) {
         mv.active = this.active;
         mv.refreshScheme = this.refreshScheme.copy();
         mv.maxMVRewriteStaleness = this.maxMVRewriteStaleness;
+        mv.viewDefineSql = this.viewDefineSql;
         if (this.baseTableIds != null) {
             mv.baseTableIds = Sets.newHashSet(this.baseTableIds);
         }

diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/CachingMvPlanContextBuilder.java b/fe/fe-core/src/main/java/com/starrocks/sql/optimizer/CachingMvPlanContextBuilder.java
@@ -18,49 +18,63 @@
 import com.github.benmanes.caffeine.cache.Caffeine;
 import com.google.common.annotations.VisibleForTesting;
 import com.starrocks.catalog.MaterializedView;
-import com.starrocks.catalog.MvId;
 import com.starrocks.catalog.MvPlanContext;
 import com.starrocks.common.Config;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 import java.util.concurrent.TimeUnit;
 
 public class CachingMvPlanContextBuilder {
+
+    private static final Logger LOG = LogManager.getLogger(CachingMvPlanContextBuilder.class);
     private static final CachingMvPlanContextBuilder INSTANCE = new CachingMvPlanContextBuilder();
 
-    private Cache<MvId, MvPlanContext> mvPlanContextCache = Caffeine.newBuilder()
-            .expireAfterAccess(Config.mv_plan_cache_expire_interval_sec, TimeUnit.SECONDS)
-            .maximumSize(Config.mv_plan_cache_max_size)
-            .build();
+    private Cache<MaterializedView, MvPlanContext> mvPlanContextCache = buildCache();
 
     private CachingMvPlanContextBuilder() {
-
     }
 
     public static CachingMvPlanContextBuilder getInstance() {
         return INSTANCE;
     }
 
+    private Cache<MaterializedView, MvPlanContext> buildCache() {
+        return Caffeine.newBuilder()
+                .expireAfterAccess(Config.mv_plan_cache_expire_interval_sec, TimeUnit.SECONDS)
+                .maximumSize(Config.mv_plan_cache_max_size)
+                .build();
+    }
+
+    @VisibleForTesting
+    public void rebuildCache() {
+        mvPlanContextCache = buildCache();
+    }
+
     public MvPlanContext getPlanContext(MaterializedView mv, boolean useCache) {
-        MvId mvId = new MvId(mv.getDbId(), mv.getId());
-        MvPlanContext result = useCache ? mvPlanContextCache.getIfPresent(mvId) : null;
-        if (result == null) {
-            MvPlanContextBuilder builder = new MvPlanContextBuilder();
-            result = builder.getPlanContext(mv);
-            if (result.isValidMvPlan()) {
-                mvPlanContextCache.put(mvId, result);
-                mv.setPlanMode(MaterializedView.PlanMode.VALID);
-            }
+        if (useCache) {
+            return mvPlanContextCache.get(mv, this::loadMvPlanContext);
+        } else {
+            return loadMvPlanContext(mv);
         }
+    }
 
-        return result;
+    private MvPlanContext loadMvPlanContext(MaterializedView mv) {
+        MvPlanContextBuilder builder = new MvPlanContextBuilder();
+        try {
+            return builder.getPlanContext(mv);
+        } catch (Throwable e) {
+            LOG.warn("load mv plan cache failed: {}", mv.getName(), e);
+            return null;
+        }
     }
 
     @VisibleForTesting
     public boolean contains(MaterializedView mv) {
-        return mvPlanContextCache.asMap().containsKey(new MvId(mv.getDbId(), mv.getId()));
+        return mvPlanContextCache.asMap().containsKey(mv);
     }
 
     public void invalidateFromCache(MaterializedView mv) {
-        mvPlanContextCache.invalidate(new MvId(mv.getDbId(), mv.getId()));
+        mvPlanContextCache.invalidate(mv);
     }
 }
diff --git a/...t/java/com/starrocks/sql/optimizer/rule/transformation/materialization/MvRewriteTest.java b/...t/java/com/starrocks/sql/optimizer/rule/transformation/materialization/MvRewriteTest.java
@@ -46,7 +46,7 @@ public void testPlanCache() throws Exception {
             MaterializedView mv = getMv("test", "mv_with_window");
             MvPlanContext planContext = CachingMvPlanContextBuilder.getInstance().getPlanContext(mv, true);
             Assert.assertNotNull(planContext);
-            Assert.assertFalse(CachingMvPlanContextBuilder.getInstance().contains(mv));
+            Assert.assertTrue(CachingMvPlanContextBuilder.getInstance().contains(mv));
             starRocksAssert.dropMaterializedView("mv_with_window");
         }