Repository: roapi/roapi (language: python)

Issue:
Feature Request: Support for Dynamic Table Deletion
## Current Situation
Currently, it's possible to add tables dynamically through the API.

## Feature Request
To improve resource management, it would be beneficial to also provide functionality for table deletion.

## Rationale
Implementing dynamic table deletion would allow for more effective resource management. This feature would complement the existing ability to add tables dynamically, providing users with greater control over their data structures.

## Potential Benefits

- More efficient use of memory and storage resources
- Improved flexibility in managing data structures
- Enhanced ability to clean up temporary or obsolete tables

## Questions

- Is this feature already on the roadmap?
- Are there any technical challenges that might make this difficult to implement?
- Would this feature require significant changes to the existing architecture?

Thank you for considering this feature request. Let me know if you need any additional information or clarification.


Discussion / hints:
@mrchypark this should be very easy to add, you can checkout the code that adds table dynamically at https://github.com/roapi/roapi/blob/7f0d4bf941c048f2c2214539943d34cbba2fec74/roapi/src/api/register.rs#L33.

to delete a table, we can just delete the table from the hash table. I am happy to review the PR if you are interested in giving it a try.

Fix patch:
diff --git a/columnq/src/columnq.rs b/columnq/src/columnq.rs
index f9d23774..f6b859dc 100644
--- a/columnq/src/columnq.rs
+++ b/columnq/src/columnq.rs
@@ -110,6 +110,12 @@ impl ColumnQ {
         Ok(())
     }
 
+    pub async fn drop_table(&mut self, t: &TableSource) -> Result<(), ColumnQError> {
+        self.schema_map.remove(&t.name);
+        self.dfctx.deregister_table(t.name.as_str())?;
+        Ok(())
+    }
+
     pub fn register_object_storage(
         &mut self,
         url: &Url,
diff --git a/roapi/src/api/drop.rs b/roapi/src/api/drop.rs
new file mode 100644
index 00000000..ba4ef20c
--- /dev/null
+++ b/roapi/src/api/drop.rs
@@ -0,0 +1,46 @@
+use std::{collections::HashMap, sync::Arc};
+
+use axum::extract::{Extension, Json};
+use columnq::error::ColumnQError;
+use columnq::table::TableSource;
+use log::info;
+use serde::Deserialize;
+use tokio::sync::Mutex;
+
+use crate::context::RoapiContext;
+use crate::error::ApiErrResp;
+
+#[derive(Debug, Deserialize)]
+pub struct SourceConfig {
+    #[serde(rename = "tableName")]
+    pub table_name: String,
+}
+
+pub async fn drop_table<H: RoapiContext>(
+    Extension(ctx): Extension<Arc<H>>,
+    Extension(tables): Extension<Arc<Mutex<HashMap<String, TableSource>>>>,
+    Json(body): Json<Vec<SourceConfig>>,
+) -> Result<(), ApiErrResp> {
+    let mut tables = tables.lock().await;
+    for config in body {
+        if let Some(t) = tables.get(&config.table_name) {
+            info!("dropping table `{}`", t.name);
+            ctx.drop_table(t)
+                .await
+                .map_err(ColumnQError::from)
+                .map_err(ApiErrResp::drop_table)?;
+            tables.remove(&config.table_name);
+            info!("dropped table `{}`", config.table_name);
+        } else {
+            return Err(ApiErrResp::not_found(format!(
+                "Table `{}` source does not exist",
+                config.table_name
+            )));
+        }
+    }
+    Ok(())
+}
+
+pub async fn drop_table_read_only() -> Result<(), ApiErrResp> {
+    Err(ApiErrResp::read_only_mode())
+}
diff --git a/roapi/src/api/mod.rs b/roapi/src/api/mod.rs
index 35f09d04..dedd0340 100644
--- a/roapi/src/api/mod.rs
+++ b/roapi/src/api/mod.rs
@@ -56,6 +56,7 @@ pub fn encode_record_batches(
     Ok(bytes_to_resp(payload, content_type.to_str()))
 }
 
+pub mod drop;
 pub mod graphql;
 pub mod health;
 pub mod kv;
diff --git a/roapi/src/api/routes.rs b/roapi/src/api/routes.rs
index 24503242..4e197325 100644
--- a/roapi/src/api/routes.rs
+++ b/roapi/src/api/routes.rs
@@ -20,9 +20,13 @@ pub fn register_app_routes<H: RoapiContext>() -> Router {
         );
 
     if H::read_only_mode() {
-        router = router.route("/api/table", post(api::register::register_table_read_only));
+        router = router
+            .route("/api/table", post(api::register::register_table_read_only))
+            .route("/api/tables/drop", post(api::drop::drop_table_read_only));
     } else {
-        router = router.route("/api/table", post(api::register::register_table::<H>));
+        router = router
+            .route("/api/table", post(api::register::register_table::<H>))
+            .route("/api/tables/drop", post(api::drop::drop_table::<H>));
     }
 
     router
diff --git a/roapi/src/context.rs b/roapi/src/context.rs
index 054d7b10..79a5dfe1 100644
--- a/roapi/src/context.rs
+++ b/roapi/src/context.rs
@@ -62,6 +62,8 @@ pub trait RoapiContext: Send + Sync + 'static {
 
     async fn load_table(&self, table: &TableSource) -> Result<(), ColumnQError>;
 
+    async fn drop_table(&self, table: &TableSource) -> Result<(), ColumnQError>;
+
     async fn schemas(&self) -> Result<Vec<(String, arrow::datatypes::SchemaRef)>, ApiErrResp>;
 
     async fn schemas_json_bytes(&self) -> Result<Vec<u8>, ApiErrResp>;
@@ -108,6 +110,13 @@ impl RoapiContext for RawRoapiContext {
         ))
     }
 
+    #[inline]
+    async fn drop_table(&self, _table: &TableSource) -> Result<(), ColumnQError> {
+        Err(ColumnQError::Generic(
+            "Table update not supported in read only mode".to_string(),
+        ))
+    }
+
     #[inline]
     async fn schemas(&self) -> Result<Vec<(String, arrow::datatypes::SchemaRef)>, ApiErrResp> {
         Ok(self
@@ -209,6 +218,12 @@ impl RoapiContext for ConcurrentRoapiContext {
         ctx.cq.load_table(table).await
     }
 
+    #[inline]
+    async fn drop_table(&self, table: &TableSource) -> Result<(), ColumnQError> {
+        let mut ctx = self.write().await;
+        ctx.cq.drop_table(table).await
+    }
+
     #[inline]
     async fn table_names(&self) -> Vec<String> {
         let ctx = self.read().await;
diff --git a/roapi/src/error.rs b/roapi/src/error.rs
index 35e00806..7c96fd50 100644
--- a/roapi/src/error.rs
+++ b/roapi/src/error.rs
@@ -97,6 +97,14 @@ impl ApiErrResp {
         }
     }
 
+    pub fn drop_table(error: ColumnQError) -> Self {
+        Self {
+            code: http::StatusCode::INTERNAL_SERVER_ERROR,
+            error: "drop_table".to_string(),
+            message: error.to_string(),
+        }
+    }
+
     pub fn load_table(error: ColumnQError) -> Self {
         Self {
             code: http::StatusCode::INTERNAL_SERVER_ERROR,


Test patch:
diff --git a/roapi/tests/api_test.rs b/roapi/tests/api_test.rs
index c6ffa504..94c79111 100644
--- a/roapi/tests/api_test.rs
+++ b/roapi/tests/api_test.rs
@@ -4,6 +4,7 @@ use std::collections::HashMap;
 
 use async_process::Command;
 use columnq::arrow::datatypes::Schema;
+use serde_json::json;
 
 #[tokio::test]
 async fn test_schema() {
@@ -18,6 +19,26 @@ async fn test_schema() {
     assert!(body.contains_key("spacex_launches"));
 }
 
+#[tokio::test]
+async fn test_drop_table() {
+    let json_table = helpers::get_spacex_table();
+    let (app, address) = helpers::test_api_app_with_tables(vec![json_table]).await;
+    tokio::spawn(app.run_until_stopped());
+
+    let get_response =
+        helpers::http_get(&format!("{address}/api/tables/spacex_launches"), None).await;
+    assert_eq!(get_response.status(), 200);
+    let drop_response = helpers::http_post_json(
+        &format!("{address}/api/tables/drop"),
+        json!([{"tableName": "spacex_launches"}]).to_string(),
+    )
+    .await;
+    assert_eq!(drop_response.status(), 200);
+    let failed_get_response =
+        helpers::http_get(&format!("{address}/api/tables/spaces_launches"), None).await;
+    assert_eq!(failed_get_response.status(), 400);
+}
+
 #[tokio::test]
 async fn test_uk_cities_sql_post() {
     let table = helpers::get_uk_cities_table();
diff --git a/roapi/tests/helpers.rs b/roapi/tests/helpers.rs
index 3d53a9bb..52f20d68 100644
--- a/roapi/tests/helpers.rs
+++ b/roapi/tests/helpers.rs
@@ -35,6 +35,7 @@ pub async fn test_api_app(
         },
         tables,
         reload_interval: Some(Duration::from_secs(1000)),
+        disable_read_only: true,
         kvstores,
         ..Default::default()
     };
@@ -68,6 +69,17 @@ pub async fn http_post(url: &str, payload: impl Into<reqwest::Body>) -> reqwest:
         .expect("Unable to execute POST request")
 }
 
+#[allow(dead_code)]
+pub async fn http_post_json(url: &str, payload: impl Into<reqwest::Body>) -> reqwest::Response {
+    reqwest::Client::new()
+        .post(url)
+        .header("Content-Type", "application/json")
+        .body(payload)
+        .send()
+        .await
+        .expect("Unable to execute POST request")
+}
+
 #[allow(dead_code)]
 pub fn get_spacex_table() -> TableSource {
     let json_source_path = test_data_path("spacex_launches.json");