allow physical region alter region options

GreptimeTeam · Nov 25, 2024 · 46113d0 · 46113d0
1 parent a6571d3
commit 46113d0
Show file tree

Hide file tree

Showing 5 changed files with 96 additions and 7 deletions.
diff --git a/src/metric-engine/src/data_region.rs b/src/metric-engine/src/data_region.rs
@@ -27,9 +27,10 @@ use store_api::storage::consts::ReservedColumnId;
 use store_api::storage::{ConcreteDataType, RegionId};
 
 use crate::error::{
-    ColumnTypeMismatchSnafu, MitoReadOperationSnafu, MitoWriteOperationSnafu, Result,
+    ColumnTypeMismatchSnafu, ForbiddenPhysicalAlterSnafu, MitoReadOperationSnafu,
+    MitoWriteOperationSnafu, Result,
 };
-use crate::metrics::MITO_DDL_DURATION;
+use crate::metrics::{FORBIDDEN_OPERATION_COUNT, MITO_DDL_DURATION};
 use crate::utils;
 
 const MAX_RETRIES: usize = 5;
@@ -186,6 +187,30 @@ impl DataRegion {
             .context(MitoReadOperationSnafu)?;
         Ok(metadata.column_metadatas.clone())
     }
+
+    pub async fn alter_region_options(
+        &self,
+        region_id: RegionId,
+        request: RegionAlterRequest,
+    ) -> Result<AffectedRows> {
+        match request.kind {
+            AlterKind::SetRegionOptions { options: _ }
+            | AlterKind::UnsetRegionOptions { keys: _ } => {
+                let region_id = utils::to_data_region_id(region_id);
+                self.mito
+                    .handle_request(region_id, RegionRequest::Alter(request))
+                    .await
+                    .context(MitoWriteOperationSnafu)
+                    .map(|result| result.affected_rows)
+            }
+            _ => {
+                info!("Metric region received alter request {request:?} on physical region {region_id:?}");
+                FORBIDDEN_OPERATION_COUNT.inc();
+
+                ForbiddenPhysicalAlterSnafu.fail()
+            }
+        }
+    }
 }
 
 #[cfg(test)]

diff --git a/src/metric-engine/src/engine.rs b/src/metric-engine/src/engine.rs
@@ -96,9 +96,10 @@ use crate::utils;
 /// |    Read    |       ✅        |        ✅        |
 /// |   Close    |       ✅        |        ✅        |
 /// |    Open    |       ✅        |        ✅        |
-/// |   Alter    |       ✅        |        ❌        |
+/// |   Alter    |       ✅        |        ❓*       |
 ///
 /// *: Physical region can be dropped only when all related logical regions are dropped.
+/// *: Alter: Physical regions only support altering region options.
 ///
 /// ## Internal Columns
 ///

diff --git a/src/metric-engine/src/engine/alter.rs b/src/metric-engine/src/engine/alter.rs
@@ -150,10 +150,10 @@ impl MetricEngineInner {
         region_id: RegionId,
         request: RegionAlterRequest,
     ) -> Result<()> {
-        info!("Metric region received alter request {request:?} on physical region {region_id:?}");
-        FORBIDDEN_OPERATION_COUNT.inc();
-
-        ForbiddenPhysicalAlterSnafu.fail()
+        self.data_region
+            .alter_region_options(region_id, request)
+            .await?;
+        Ok(())
     }
 }
 

diff --git a/tests/cases/standalone/common/alter/alter_table_options.result b/tests/cases/standalone/common/alter/alter_table_options.result
@@ -281,3 +281,54 @@ DROP TABLE ato;
 
 Affected Rows: 0
 
+CREATE TABLE phy (ts timestamp time index, val double) engine=metric with ("physical_metric_table" = "");
+
+Affected Rows: 0
+
+ALTER TABLE phy set ttl='2years';
+
+Affected Rows: 0
+
+SHOW CREATE TABLE phy;
+
++-------+------------------------------------+
+| Table | Create Table                       |
++-------+------------------------------------+
+| phy   | CREATE TABLE IF NOT EXISTS "phy" ( |
+|       |   "ts" TIMESTAMP(3) NOT NULL,      |
+|       |   "val" DOUBLE NULL,               |
+|       |   TIME INDEX ("ts")                |
+|       | )                                  |
+|       |                                    |
+|       | ENGINE=metric                      |
+|       | WITH(                              |
+|       |   physical_metric_table = '',      |
+|       |   ttl = '2years'                   |
+|       | )                                  |
++-------+------------------------------------+
+
+ALTER TABLE phy UNSET 'ttl';
+
+Affected Rows: 0
+
+SHOW CREATE TABLE phy;
+
++-------+------------------------------------+
+| Table | Create Table                       |
++-------+------------------------------------+
+| phy   | CREATE TABLE IF NOT EXISTS "phy" ( |
+|       |   "ts" TIMESTAMP(3) NOT NULL,      |
+|       |   "val" DOUBLE NULL,               |
+|       |   TIME INDEX ("ts")                |
+|       | )                                  |
+|       |                                    |
+|       | ENGINE=metric                      |
+|       | WITH(                              |
+|       |   physical_metric_table = ''       |
+|       | )                                  |
++-------+------------------------------------+
+
+DROP TABLE phy;
+
+Affected Rows: 0
+
diff --git a/tests/cases/standalone/common/alter/alter_table_options.sql b/tests/cases/standalone/common/alter/alter_table_options.sql
@@ -60,3 +60,15 @@ SHOW CREATE TABLE ato;
 SHOW CREATE TABLE ato;
 
 DROP TABLE ato;
+
+CREATE TABLE phy (ts timestamp time index, val double) engine=metric with ("physical_metric_table" = "");
+
+ALTER TABLE phy set ttl='2years';
+
+SHOW CREATE TABLE phy;
+
+ALTER TABLE phy UNSET 'ttl';
+
+SHOW CREATE TABLE phy;
+
+DROP TABLE phy;