From 4dd77f602fff0d02f36d4c6e48b19dc55050cd76 Mon Sep 17 00:00:00 2001
From: Mingyu Chen <morningman.cmy@gmail.com>
Date: Thu, 11 Nov 2021 15:43:16 +0800
Subject: [PATCH] [Bug] Fix bug that NPE thrown when adding partition for table
 with MV (#7069)

The `defineExpr` in `Column` must be analyzed before calling its `treeToThrift` method.
And fro CreateReplicaTask, no need to set `defineExpr` in TColumn.
---
 .../java/org/apache/doris/analysis/SlotRef.java  |  1 +
 .../java/org/apache/doris/catalog/Column.java    | 16 ++++++++--------
 .../doris/catalog/MaterializedIndexMeta.java     |  1 -
 .../org/apache/doris/task/CreateRollupTask.java  |  1 +
 .../org/apache/doris/task/SchemaChangeTask.java  |  1 +
 5 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java
index 006beffe2d..e1995c550f 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SlotRef.java
@@ -404,6 +404,7 @@ public class SlotRef extends Expr {
 
     @Override
     public boolean isNullable() {
+        Preconditions.checkNotNull(desc);
         return desc.getIsNullable();
     }
 }
diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/Column.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/Column.java
index b8755639fb..2e61df66ac 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/Column.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/Column.java
@@ -84,7 +84,7 @@ public class Column implements Writable {
     private List<Column> children;
     // Define expr may exist in two forms, one is analyzed, and the other is not analyzed.
     // Currently, analyzed define expr is only used when creating materialized views, so the define expr in RollupJob must be analyzed.
-    // In other cases, such as define expr in `MaterializedIndexMeta`, it may not be analyzed after being relayed.
+    // In other cases, such as define expr in `MaterializedIndexMeta`, it may not be analyzed after being replayed.
     private Expr defineExpr; // use to define column in materialize view
     @SerializedName(value = "visible")
     private boolean visible;
@@ -344,13 +344,13 @@ public class Column implements Writable {
         tColumn.setVisible(visible);
         tColumn.setChildrenColumn(new ArrayList<>());
         toChildrenThrift(this, tColumn);
-
-        // The define expr does not need to be serialized here for now.
-        // At present, only serialized(analyzed) define expr is directly used when creating a materialized view.
-        // It will not be used here, but through another structure `TAlterMaterializedViewParam`.
-        if (this.defineExpr != null) {
-            tColumn.setDefineExpr(this.defineExpr.treeToThrift());
-        }
+        
+        // ATTN:
+        // Currently, this `toThrift()` method is only used from CreateReplicaTask.
+        // And CreateReplicaTask does not need `defineExpr` field.
+        // The `defineExpr` is only used when creating `TAlterMaterializedViewParam`, which is in `AlterReplicaTask`.
+        // And when creating `TAlterMaterializedViewParam`, the `defineExpr` is certainly analyzed.
+        // If we need to use `defineExpr` and call defineExpr.treeToThrift(), make sure it is analyzed, or NPE will thrown.
         return tColumn;
     }
 
diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/MaterializedIndexMeta.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/MaterializedIndexMeta.java
index 0e6712d6ee..c236554e1d 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/MaterializedIndexMeta.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/MaterializedIndexMeta.java
@@ -58,7 +58,6 @@ public class MaterializedIndexMeta implements Writable, GsonPostProcessable {
     @SerializedName(value = "keysType")
     private KeysType keysType;
     @SerializedName(value = "defineStmt")
-
     private OriginStatement defineStmt;
 
     public MaterializedIndexMeta(long indexId, List<Column> schema, int schemaVersion, int schemaHash,
diff --git a/fe/fe-core/src/main/java/org/apache/doris/task/CreateRollupTask.java b/fe/fe-core/src/main/java/org/apache/doris/task/CreateRollupTask.java
index 68d00cfdd9..a4fb118d06 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/task/CreateRollupTask.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/task/CreateRollupTask.java
@@ -31,6 +31,7 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
 
+@Deprecated
 public class CreateRollupTask extends AgentTask {
 
     private long baseTableId;
diff --git a/fe/fe-core/src/main/java/org/apache/doris/task/SchemaChangeTask.java b/fe/fe-core/src/main/java/org/apache/doris/task/SchemaChangeTask.java
index 865330c6f4..59616ccaff 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/task/SchemaChangeTask.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/task/SchemaChangeTask.java
@@ -31,6 +31,7 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Set;
 
+@Deprecated
 public class SchemaChangeTask extends AgentTask {
 
     private long baseReplicaId;