feat(mito): Implement skeleton for alteration (#2343)

* feat: impl handle_alter wip

* refactor: move send_result to worker.rs

* feat: skeleton for handle_alter_request

* feat: write requests should wait for alteration

* feat: define alter request

* chore: no warnings

* fix: remove memtables after flush

* chore: update comments and impl add_write_request_to_pending

* feat: add schema version to RegionMetadata

* feat: impl alter_schema/can_alter_directly

* chore: use send_result

* test: pull next_batch again

* feat: convert pb AlterRequest to RegionAlterRequest

* feat: validate alter request

* feat: validate request and alter metadata

* feat: allow none location

* test: test alter

* fix: recover files and flushed entry id from manifest

* test: test alter

* chore: change comments and variables

* chore: fix compiler errors

* feat: add is_empty() to MemtableVersion

* test: fix metadata alter test

* fix: Compaction picker doesn't notify waiters if it returns None

* chore: address CR comments

* test: add tests for alter request

* refactor: use send_result

src/store-api/src/metadata.rs

@@ -32,6 +32,7 @@ use serde::de::Error;
 use serde::{Deserialize, Deserializer, Serialize};
 use snafu::{ensure, Location, OptionExt, ResultExt, Snafu};
 
+use crate::region_request::{AddColumn, AddColumnLocation, AlterKind};
 use crate::storage::{ColumnId, RegionId};
 
 pub type Result<T> = std::result::Result<T, MetadataError>;
@@ -127,6 +128,10 @@ pub struct RegionMetadata {
 
     /// Immutable and unique id of a region.
     pub region_id: RegionId,
+    /// Current version of the region schema.
+    ///
+    /// The version starts from 0. Altering the schema bumps the version.
+    pub schema_version: u64,
 }
 
 pub type RegionMetadataRef = Arc<RegionMetadata>;
@@ -142,6 +147,7 @@ impl<'de> Deserialize<'de> for RegionMetadata {
             column_metadatas: Vec<ColumnMetadata>,
             primary_key: Vec<ColumnId>,
             region_id: RegionId,
+            schema_version: u64,
         }
 
         let without_schema = RegionMetadataWithoutSchema::deserialize(deserializer)?;
@@ -155,6 +161,7 @@ impl<'de> Deserialize<'de> for RegionMetadata {
             column_metadatas: without_schema.column_metadatas,
             primary_key: without_schema.primary_key,
             region_id: without_schema.region_id,
+            schema_version: without_schema.schema_version,
         })
     }
 }
@@ -378,6 +385,7 @@ pub struct RegionMetadataBuilder {
     region_id: RegionId,
     column_metadatas: Vec<ColumnMetadata>,
     primary_key: Vec<ColumnId>,
+    schema_version: u64,
 }
 
 impl RegionMetadataBuilder {
@@ -387,31 +395,50 @@ impl RegionMetadataBuilder {
             region_id: id,
             column_metadatas: vec![],
             primary_key: vec![],
+            schema_version: 0,
         }
     }
 
-    /// Create a builder from existing [RegionMetadata].
+    /// Creates a builder from existing [RegionMetadata].
     pub fn from_existing(existing: RegionMetadata) -> Self {
         Self {
             column_metadatas: existing.column_metadatas,
             primary_key: existing.primary_key,
             region_id: existing.region_id,
+            schema_version: existing.schema_version,
         }
     }
 
-    /// Push a new column metadata to this region's metadata.
+    /// Pushes a new column metadata to this region's metadata.
     pub fn push_column_metadata(&mut self, column_metadata: ColumnMetadata) -> &mut Self {
         self.column_metadatas.push(column_metadata);
         self
     }
 
-    /// Set the primary key of the region.
+    /// Sets the primary key of the region.
     pub fn primary_key(&mut self, key: Vec<ColumnId>) -> &mut Self {
         self.primary_key = key;
         self
     }
 
-    /// Consume the builder and build a [RegionMetadata].
+    /// Increases the schema version by 1.
+    pub fn bump_version(&mut self) -> &mut Self {
+        self.schema_version += 1;
+        self
+    }
+
+    /// Applies the alter `kind` to the builder.
+    ///
+    /// The `kind` should be valid.
+    pub fn alter(&mut self, kind: AlterKind) -> Result<&mut Self> {
+        match kind {
+            AlterKind::AddColumns { columns } => self.add_columns(columns)?,
+            AlterKind::DropColumns { names } => self.drop_columns(&names),
+        }
+        Ok(self)
+    }
+
+    /// Consumes the builder and build a [RegionMetadata].
     pub fn build(self) -> Result<RegionMetadata> {
         let skipped = SkippedFields::new(&self.column_metadatas)?;
 
@@ -422,12 +449,58 @@ impl RegionMetadataBuilder {
             column_metadatas: self.column_metadatas,
             primary_key: self.primary_key,
             region_id: self.region_id,
+            schema_version: self.schema_version,
         };
 
         meta.validate()?;
 
         Ok(meta)
     }
+
+    /// Adds columns to the metadata.
+    fn add_columns(&mut self, columns: Vec<AddColumn>) -> Result<()> {
+        for add_column in columns {
+            let column_id = add_column.column_metadata.column_id;
+            let semantic_type = add_column.column_metadata.semantic_type;
+            match add_column.location {
+                None => {
+                    self.column_metadatas.push(add_column.column_metadata);
+                }
+                Some(AddColumnLocation::First) => {
+                    self.column_metadatas.insert(0, add_column.column_metadata);
+                }
+                Some(AddColumnLocation::After { column_name }) => {
+                    let pos = self
+                        .column_metadatas
+                        .iter()
+                        .position(|col| col.column_schema.name == column_name)
+                        .context(InvalidRegionRequestSnafu {
+                            region_id: self.region_id,
+                            err: format!(
+                                "column {} not found, failed to add column {} after it",
+                                column_name, add_column.column_metadata.column_schema.name
+                            ),
+                        })?;
+                    // Insert after pos.
+                    self.column_metadatas
+                        .insert(pos + 1, add_column.column_metadata);
+                }
+            }
+            if semantic_type == SemanticType::Tag {
+                // For a new tag, we extend the primary key.
+                self.primary_key.push(column_id);
+            }
+        }
+
+        Ok(())
+    }
+
+    /// Drops columns from the metadata.
+    fn drop_columns(&mut self, names: &[String]) {
+        let name_set: HashSet<_> = names.iter().collect();
+        self.column_metadatas
+            .retain(|col| !name_set.contains(&col.column_schema.name));
+    }
 }
 
 /// Fields skipped in serialization.
@@ -497,7 +570,7 @@ pub enum MetadataError {
     },
 
     #[snafu(display(
-        "Failed to convert with struct from datatypes, location: {}, source: {}",
+        "Failed to convert struct from datatypes, location: {}, source: {}",
         location,
         source
     ))]
@@ -506,8 +579,20 @@ pub enum MetadataError {
         source: datatypes::error::Error,
     },
 
-    #[snafu(display("Invalid raw region request: {err}, at {location}"))]
+    #[snafu(display("Invalid raw region request, err: {}, location: {}", err, location))]
     InvalidRawRegionRequest { err: String, location: Location },
+
+    #[snafu(display(
+        "Invalid region request, region_id: {}, err: {}, location: {}",
+        region_id,
+        err,
+        location
+    ))]
+    InvalidRegionRequest {
+        region_id: RegionId,
+        err: String,
+        location: Location,
+    },
 }
 
 impl ErrorExt for MetadataError {
@@ -812,4 +897,113 @@ mod test {
             "unexpected err: {err}",
         );
     }
+
+    #[test]
+    fn test_bump_version() {
+        let mut region_metadata = build_test_region_metadata();
+        let mut builder = RegionMetadataBuilder::from_existing(region_metadata.clone());
+        builder.bump_version();
+        let new_meta = builder.build().unwrap();
+        region_metadata.schema_version += 1;
+        assert_eq!(region_metadata, new_meta);
+    }
+
+    fn new_column_metadata(name: &str, is_tag: bool, column_id: ColumnId) -> ColumnMetadata {
+        let semantic_type = if is_tag {
+            SemanticType::Tag
+        } else {
+            SemanticType::Field
+        };
+        ColumnMetadata {
+            column_schema: ColumnSchema::new(name, ConcreteDataType::string_datatype(), true),
+            semantic_type,
+            column_id,
+        }
+    }
+
+    fn check_columns(metadata: &RegionMetadata, names: &[&str]) {
+        let actual: Vec<_> = metadata
+            .column_metadatas
+            .iter()
+            .map(|col| &col.column_schema.name)
+            .collect();
+        assert_eq!(names, actual);
+    }
+
+    #[test]
+    fn test_alter() {
+        // a (tag), b (field), c (ts)
+        let metadata = build_test_region_metadata();
+        let mut builder = RegionMetadataBuilder::from_existing(metadata);
+        builder
+            .alter(AlterKind::AddColumns {
+                columns: vec![AddColumn {
+                    column_metadata: new_column_metadata("d", true, 4),
+                    location: None,
+                }],
+            })
+            .unwrap();
+        let metadata = builder.build().unwrap();
+        check_columns(&metadata, &["a", "b", "c", "d"]);
+        assert_eq!([1, 4], &metadata.primary_key[..]);
+
+        let mut builder = RegionMetadataBuilder::from_existing(metadata);
+        builder
+            .alter(AlterKind::AddColumns {
+                columns: vec![AddColumn {
+                    column_metadata: new_column_metadata("e", false, 5),
+                    location: Some(AddColumnLocation::First),
+                }],
+            })
+            .unwrap();
+        let metadata = builder.build().unwrap();
+        check_columns(&metadata, &["e", "a", "b", "c", "d"]);
+
+        let mut builder = RegionMetadataBuilder::from_existing(metadata);
+        builder
+            .alter(AlterKind::AddColumns {
+                columns: vec![AddColumn {
+                    column_metadata: new_column_metadata("f", false, 6),
+                    location: Some(AddColumnLocation::After {
+                        column_name: "b".to_string(),
+                    }),
+                }],
+            })
+            .unwrap();
+        let metadata = builder.build().unwrap();
+        check_columns(&metadata, &["e", "a", "b", "f", "c", "d"]);
+
+        let mut builder = RegionMetadataBuilder::from_existing(metadata);
+        builder
+            .alter(AlterKind::AddColumns {
+                columns: vec![AddColumn {
+                    column_metadata: new_column_metadata("g", false, 7),
+                    location: Some(AddColumnLocation::After {
+                        column_name: "d".to_string(),
+                    }),
+                }],
+            })
+            .unwrap();
+        let metadata = builder.build().unwrap();
+        check_columns(&metadata, &["e", "a", "b", "f", "c", "d", "g"]);
+
+        let mut builder = RegionMetadataBuilder::from_existing(metadata);
+        builder
+            .alter(AlterKind::DropColumns {
+                names: vec!["g".to_string(), "e".to_string()],
+            })
+            .unwrap();
+        let metadata = builder.build().unwrap();
+        check_columns(&metadata, &["a", "b", "f", "c", "d"]);
+
+        let mut builder = RegionMetadataBuilder::from_existing(metadata);
+        builder
+            .alter(AlterKind::DropColumns {
+                names: vec!["a".to_string()],
+            })
+            .unwrap();
+        // Build returns error as the primary key has more columns.
+        let err = builder.build().unwrap_err();
+        assert_eq!(StatusCode::InvalidArguments, err.status_code());
+    }
 }

src/store-api/src/region_request.rs

@@ -12,15 +12,19 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 
-use api::v1::region::region_request;
-use api::v1::Rows;
-use snafu::OptionExt;
+use api::v1::add_column_location::LocationType;
+use api::v1::region::{alter_request, region_request, AlterRequest};
+use api::v1::{self, Rows, SemanticType};
+use snafu::{ensure, OptionExt};
 
-use crate::metadata::{ColumnMetadata, InvalidRawRegionRequestSnafu, MetadataError};
+use crate::metadata::{
+    ColumnMetadata, InvalidRawRegionRequestSnafu, InvalidRegionRequestSnafu, MetadataError,
+    RegionMetadata, Result,
+};
 use crate::path_utils::region_dir;
-use crate::storage::{AlterRequest, ColumnId, RegionId, ScanRequest};
+use crate::storage::{ColumnId, RegionId, ScanRequest};
 
 #[derive(Debug)]
 pub enum RegionRequest {
@@ -39,11 +43,7 @@ pub enum RegionRequest {
 impl RegionRequest {
     /// Convert [Body](region_request::Body) to a group of [RegionRequest] with region id.
     /// Inserts/Deletes request might become multiple requests. Others are one-to-one.
-    // TODO: implement alter request
-    #[allow(unreachable_code)]
-    pub fn try_from_request_body(
-        body: region_request::Body,
-    ) -> Result<Vec<(RegionId, Self)>, MetadataError> {
+    pub fn try_from_request_body(body: region_request::Body) -> Result<Vec<(RegionId, Self)>> {
         match body {
             region_request::Body::Inserts(inserts) => Ok(inserts
                 .requests
@@ -68,7 +68,7 @@ impl RegionRequest {
                     .column_defs
                     .into_iter()
                     .map(ColumnMetadata::try_from_column_def)
-                    .collect::<Result<Vec<_>, _>>()?;
+                    .collect::<Result<Vec<_>>>()?;
                 let region_id = create.region_id.into();
                 let region_dir = region_dir(&create.catalog, &create.schema, region_id);
                 Ok(vec![(
@@ -103,25 +103,10 @@ impl RegionRequest {
                 close.region_id.into(),
                 Self::Close(RegionCloseRequest {}),
             )]),
-            region_request::Body::Alter(alter) => {
-                let kind = alter.kind.context(InvalidRawRegionRequestSnafu {
-                    err: "'kind' is absent",
-                })?;
-                Ok(vec![(
-                    alter.region_id.into(),
-                    Self::Alter(RegionAlterRequest {
-                        request: AlterRequest {
-                            operation: kind.try_into().map_err(|e| {
-                                InvalidRawRegionRequestSnafu {
-                                    err: format!("{e}"),
-                                }
-                                .build()
-                            })?,
-                            version: alter.schema_version as _,
-                        },
-                    }),
-                )])
-            }
+            region_request::Body::Alter(alter) => Ok(vec![(
+                alter.region_id.into(),
+                Self::Alter(RegionAlterRequest::try_from(alter)?),
+            )]),
             region_request::Body::Flush(flush) => Ok(vec![(
                 flush.region_id.into(),
                 Self::Flush(RegionFlushRequest {}),
@@ -189,9 +174,236 @@ pub struct RegionOpenRequest {
 #[derive(Debug)]
 pub struct RegionCloseRequest {}
 
-#[derive(Debug)]
+/// Alter metadata of a region.
+#[derive(Debug, PartialEq, Eq)]
 pub struct RegionAlterRequest {
-    pub request: AlterRequest,
+    /// The version of the schema before applying the alteration.
+    pub schema_version: u64,
+    /// Kind of alteration to do.
+    pub kind: AlterKind,
+}
+
+impl RegionAlterRequest {
+    /// Checks whether the request is valid, returns an error if it is invalid.
+    pub fn validate(&self, metadata: &RegionMetadata) -> Result<()> {
+        ensure!(
+            metadata.schema_version == self.schema_version,
+            InvalidRegionRequestSnafu {
+                region_id: metadata.region_id,
+                err: format!(
+                    "region schema version {} is not equal to request schema version {}",
+                    metadata.schema_version, self.schema_version
+                ),
+            }
+        );
+
+        self.kind.validate(metadata)?;
+
+        Ok(())
+    }
+}
+
+impl TryFrom<AlterRequest> for RegionAlterRequest {
+    type Error = MetadataError;
+
+    fn try_from(value: AlterRequest) -> Result<Self> {
+        let kind = value.kind.context(InvalidRawRegionRequestSnafu {
+            err: "missing kind in AlterRequest",
+        })?;
+
+        let kind = AlterKind::try_from(kind)?;
+        Ok(RegionAlterRequest {
+            schema_version: value.schema_version,
+            kind,
+        })
+    }
+}
+
+/// Kind of the alteration.
+#[derive(Debug, PartialEq, Eq)]
+pub enum AlterKind {
+    /// Add columns to the region.
+    AddColumns {
+        /// Columns to add.
+        columns: Vec<AddColumn>,
+    },
+    /// Drop columns from the region, only fields are allowed to drop.
+    DropColumns {
+        /// Name of columns to drop.
+        names: Vec<String>,
+    },
+}
+
+impl AlterKind {
+    /// Returns an error if the the alter kind is invalid.
+    pub fn validate(&self, metadata: &RegionMetadata) -> Result<()> {
+        match self {
+            AlterKind::AddColumns { columns } => {
+                let mut names = HashSet::with_capacity(columns.len());
+                for col_to_add in columns {
+                    ensure!(
+                        !names.contains(&col_to_add.column_metadata.column_schema.name),
+                        InvalidRegionRequestSnafu {
+                            region_id: metadata.region_id,
+                            err: format!(
+                                "add column {} more than once",
+                                col_to_add.column_metadata.column_schema.name
+                            ),
+                        }
+                    );
+                    col_to_add.validate(metadata)?;
+                    names.insert(&col_to_add.column_metadata.column_schema.name);
+                }
+            }
+            AlterKind::DropColumns { names } => {
+                for name in names {
+                    Self::validate_column_to_drop(name, metadata)?;
+                }
+            }
+        }
+        Ok(())
+    }
+
+    /// Returns an error if the column to drop is invalid.
+    fn validate_column_to_drop(name: &str, metadata: &RegionMetadata) -> Result<()> {
+        let column = metadata
+            .column_by_name(name)
+            .with_context(|| InvalidRegionRequestSnafu {
+                region_id: metadata.region_id,
+                err: format!("column {} does not exist", name),
+            })?;
+        ensure!(
+            column.semantic_type == SemanticType::Field,
+            InvalidRegionRequestSnafu {
+                region_id: metadata.region_id,
+                err: format!("column {} is not a field and could not be dropped", name),
+            }
+        );
+        Ok(())
+    }
+}
+
+impl TryFrom<alter_request::Kind> for AlterKind {
+    type Error = MetadataError;
+
+    fn try_from(kind: alter_request::Kind) -> Result<Self> {
+        let alter_kind = match kind {
+            alter_request::Kind::AddColumns(x) => {
+                let columns = x
+                    .add_columns
+                    .into_iter()
+                    .map(|x| x.try_into())
+                    .collect::<Result<Vec<_>>>()?;
+                AlterKind::AddColumns { columns }
+            }
+            alter_request::Kind::DropColumns(x) => {
+                let names = x.drop_columns.into_iter().map(|x| x.name).collect();
+                AlterKind::DropColumns { names }
+            }
+        };
+
+        Ok(alter_kind)
+    }
+}
+
+/// Adds a column.
+#[derive(Debug, PartialEq, Eq)]
+pub struct AddColumn {
+    /// Metadata of the column to add.
+    pub column_metadata: ColumnMetadata,
+    /// Location to add the column. If location is None, the region adds
+    /// the column to the last.
+    pub location: Option<AddColumnLocation>,
+}
+
+impl AddColumn {
+    /// Returns an error if the column to add is invalid.
+    pub fn validate(&self, metadata: &RegionMetadata) -> Result<()> {
+        ensure!(
+            self.column_metadata.column_schema.is_nullable()
+                || self
+                    .column_metadata
+                    .column_schema
+                    .default_constraint()
+                    .is_some(),
+            InvalidRegionRequestSnafu {
+                region_id: metadata.region_id,
+                err: format!(
+                    "no default value for column {}",
+                    self.column_metadata.column_schema.name
+                ),
+            }
+        );
+        ensure!(
+            metadata
+                .column_by_name(&self.column_metadata.column_schema.name)
+                .is_none(),
+            InvalidRegionRequestSnafu {
+                region_id: metadata.region_id,
+                err: format!(
+                    "column {} already exists",
+                    self.column_metadata.column_schema.name
+                ),
+            }
+        );
+
+        Ok(())
+    }
+}
+
+impl TryFrom<v1::region::AddColumn> for AddColumn {
+    type Error = MetadataError;
+
+    fn try_from(add_column: v1::region::AddColumn) -> Result<Self> {
+        let column_def = add_column
+            .column_def
+            .context(InvalidRawRegionRequestSnafu {
+                err: "missing column_def in AddColumn",
+            })?;
+
+        let column_metadata = ColumnMetadata::try_from_column_def(column_def)?;
+        let location = add_column
+            .location
+            .map(AddColumnLocation::try_from)
+            .transpose()?;
+
+        Ok(AddColumn {
+            column_metadata,
+            location,
+        })
+    }
+}
+
+/// Location to add a column.
+#[derive(Debug, PartialEq, Eq)]
+pub enum AddColumnLocation {
+    /// Add the column to the first position of columns.
+    First,
+    /// Add the column after specific column.
+    After {
+        /// Add the column after this column.
+        column_name: String,
+    },
+}
+
+impl TryFrom<v1::AddColumnLocation> for AddColumnLocation {
+    type Error = MetadataError;
+
+    fn try_from(location: v1::AddColumnLocation) -> Result<Self> {
+        let location_type = LocationType::from_i32(location.location_type).context(
+            InvalidRawRegionRequestSnafu {
+                err: format!("unknown location type {}", location.location_type),
+            },
+        )?;
+        let add_column_location = match location_type {
+            LocationType::First => AddColumnLocation::First,
+            LocationType::After => AddColumnLocation::After {
+                column_name: location.after_column_name,
+            },
+        };
+
+        Ok(add_column_location)
+    }
 }
 
 #[derive(Debug)]
@@ -199,3 +411,297 @@ pub struct RegionFlushRequest {}
 
 #[derive(Debug)]
 pub struct RegionCompactRequest {}
+
+#[cfg(test)]
+mod tests {
+    use api::v1::region::RegionColumnDef;
+    use api::v1::{ColumnDataType, ColumnDef};
+    use datatypes::prelude::ConcreteDataType;
+    use datatypes::schema::ColumnSchema;
+
+    use super::*;
+    use crate::metadata::RegionMetadataBuilder;
+
+    #[test]
+    fn test_from_proto_location() {
+        let proto_location = v1::AddColumnLocation {
+            location_type: LocationType::First as i32,
+            after_column_name: "".to_string(),
+        };
+        let location = AddColumnLocation::try_from(proto_location).unwrap();
+        assert_eq!(location, AddColumnLocation::First);
+
+        let proto_location = v1::AddColumnLocation {
+            location_type: 10,
+            after_column_name: "".to_string(),
+        };
+        AddColumnLocation::try_from(proto_location).unwrap_err();
+
+        let proto_location = v1::AddColumnLocation {
+            location_type: LocationType::After as i32,
+            after_column_name: "a".to_string(),
+        };
+        let location = AddColumnLocation::try_from(proto_location).unwrap();
+        assert_eq!(
+            location,
+            AddColumnLocation::After {
+                column_name: "a".to_string()
+            }
+        );
+    }
+
+    #[test]
+    fn test_from_none_proto_add_column() {
+        AddColumn::try_from(v1::region::AddColumn {
+            column_def: None,
+            location: None,
+        })
+        .unwrap_err();
+    }
+
+    #[test]
+    fn test_from_proto_alter_request() {
+        RegionAlterRequest::try_from(AlterRequest {
+            region_id: 0,
+            schema_version: 1,
+            kind: None,
+        })
+        .unwrap_err();
+
+        let request = RegionAlterRequest::try_from(AlterRequest {
+            region_id: 0,
+            schema_version: 1,
+            kind: Some(alter_request::Kind::AddColumns(v1::region::AddColumns {
+                add_columns: vec![v1::region::AddColumn {
+                    column_def: Some(RegionColumnDef {
+                        column_def: Some(ColumnDef {
+                            name: "a".to_string(),
+                            data_type: ColumnDataType::String as i32,
+                            is_nullable: true,
+                            default_constraint: vec![],
+                            semantic_type: SemanticType::Field as i32,
+                        }),
+                        column_id: 1,
+                    }),
+                    location: Some(v1::AddColumnLocation {
+                        location_type: LocationType::First as i32,
+                        after_column_name: "".to_string(),
+                    }),
+                }],
+            })),
+        })
+        .unwrap();
+
+        assert_eq!(
+            request,
+            RegionAlterRequest {
+                schema_version: 1,
+                kind: AlterKind::AddColumns {
+                    columns: vec![AddColumn {
+                        column_metadata: ColumnMetadata {
+                            column_schema: ColumnSchema::new(
+                                "a",
+                                ConcreteDataType::string_datatype(),
+                                true,
+                            ),
+                            semantic_type: SemanticType::Field,
+                            column_id: 1,
+                        },
+                        location: Some(AddColumnLocation::First),
+                    }]
+                },
+            }
+        );
+    }
+
+    fn new_metadata() -> RegionMetadata {
+        let mut builder = RegionMetadataBuilder::new(RegionId::new(1, 1));
+        builder
+            .push_column_metadata(ColumnMetadata {
+                column_schema: ColumnSchema::new(
+                    "ts",
+                    ConcreteDataType::timestamp_millisecond_datatype(),
+                    false,
+                ),
+                semantic_type: SemanticType::Timestamp,
+                column_id: 1,
+            })
+            .push_column_metadata(ColumnMetadata {
+                column_schema: ColumnSchema::new(
+                    "tag_0",
+                    ConcreteDataType::string_datatype(),
+                    true,
+                ),
+                semantic_type: SemanticType::Tag,
+                column_id: 2,
+            })
+            .push_column_metadata(ColumnMetadata {
+                column_schema: ColumnSchema::new(
+                    "field_0",
+                    ConcreteDataType::string_datatype(),
+                    true,
+                ),
+                semantic_type: SemanticType::Field,
+                column_id: 3,
+            })
+            .primary_key(vec![2]);
+        builder.build().unwrap()
+    }
+
+    #[test]
+    fn test_add_column_validate() {
+        let metadata = new_metadata();
+        AddColumn {
+            column_metadata: ColumnMetadata {
+                column_schema: ColumnSchema::new(
+                    "tag_1",
+                    ConcreteDataType::string_datatype(),
+                    true,
+                ),
+                semantic_type: SemanticType::Tag,
+                column_id: 4,
+            },
+            location: None,
+        }
+        .validate(&metadata)
+        .unwrap();
+
+        AddColumn {
+            column_metadata: ColumnMetadata {
+                column_schema: ColumnSchema::new(
+                    "tag_1",
+                    ConcreteDataType::string_datatype(),
+                    false,
+                ),
+                semantic_type: SemanticType::Tag,
+                column_id: 4,
+            },
+            location: None,
+        }
+        .validate(&metadata)
+        .unwrap_err();
+
+        AddColumn {
+            column_metadata: ColumnMetadata {
+                column_schema: ColumnSchema::new(
+                    "tag_0",
+                    ConcreteDataType::string_datatype(),
+                    true,
+                ),
+                semantic_type: SemanticType::Tag,
+                column_id: 4,
+            },
+            location: None,
+        }
+        .validate(&metadata)
+        .unwrap_err();
+    }
+
+    #[test]
+    fn test_add_duplicate_columns() {
+        let kind = AlterKind::AddColumns {
+            columns: vec![
+                AddColumn {
+                    column_metadata: ColumnMetadata {
+                        column_schema: ColumnSchema::new(
+                            "tag_1",
+                            ConcreteDataType::string_datatype(),
+                            true,
+                        ),
+                        semantic_type: SemanticType::Tag,
+                        column_id: 4,
+                    },
+                    location: None,
+                },
+                AddColumn {
+                    column_metadata: ColumnMetadata {
+                        column_schema: ColumnSchema::new(
+                            "tag_1",
+                            ConcreteDataType::string_datatype(),
+                            true,
+                        ),
+                        semantic_type: SemanticType::Field,
+                        column_id: 5,
+                    },
+                    location: None,
+                },
+            ],
+        };
+        let metadata = new_metadata();
+        kind.validate(&metadata).unwrap_err();
+    }
+
+    #[test]
+    fn test_validate_drop_column() {
+        let metadata = new_metadata();
+        AlterKind::DropColumns {
+            names: vec!["xxxx".to_string()],
+        }
+        .validate(&metadata)
+        .unwrap_err();
+        AlterKind::DropColumns {
+            names: vec!["tag_0".to_string()],
+        }
+        .validate(&metadata)
+        .unwrap_err();
+        AlterKind::DropColumns {
+            names: vec!["field_0".to_string()],
+        }
+        .validate(&metadata)
+        .unwrap();
+    }
+
+    #[test]
+    fn test_validate_schema_version() {
+        let mut metadata = new_metadata();
+        metadata.schema_version = 2;
+
+        RegionAlterRequest {
+            schema_version: 1,
+            kind: AlterKind::DropColumns {
+                names: vec!["field_0".to_string()],
+            },
+        }
+        .validate(&metadata)
+        .unwrap_err();
+    }
+
+    #[test]
+    fn test_validate_add_columns() {
+        let kind = AlterKind::AddColumns {
+            columns: vec![
+                AddColumn {
+                    column_metadata: ColumnMetadata {
+                        column_schema: ColumnSchema::new(
+                            "tag_1",
+                            ConcreteDataType::string_datatype(),
+                            true,
+                        ),
+                        semantic_type: SemanticType::Tag,
+                        column_id: 4,
+                    },
+                    location: None,
+                },
+                AddColumn {
+                    column_metadata: ColumnMetadata {
+                        column_schema: ColumnSchema::new(
+                            "field_1",
+                            ConcreteDataType::string_datatype(),
+                            true,
+                        ),
+                        semantic_type: SemanticType::Field,
+                        column_id: 5,
+                    },
+                    location: None,
+                },
+            ],
+        };
+        let request = RegionAlterRequest {
+            schema_version: 1,
+            kind,
+        };
+        let mut metadata = new_metadata();
+        metadata.schema_version = 1;
+        request.validate(&metadata).unwrap();
+    }
+}