fix: don't error on delete to non-existent table (#3259) (#3263)

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>

influxdb_iox/src/influxdb_ioxd/http/dml.rs

@@ -83,9 +83,6 @@ pub enum HttpDmlError {
     #[snafu(display("Database {} not found", db_name))]
     NotFoundDatabase { db_name: String },
 
-    #[snafu(display("Table {}:{} not found", db_name, table_name))]
-    NotFoundTable { db_name: String, table_name: String },
-
     #[snafu(display("Cannot parse body: {}", source))]
     ParseBody {
         source: crate::influxdb_ioxd::http::utils::ParseBodyError,
@@ -117,7 +114,6 @@ impl HttpApiErrorSource for HttpDmlError {
             e @ Self::ReadingBodyAsUtf8 { .. } => e.invalid(),
             e @ Self::ParsingLineProtocol { .. } => e.invalid(),
             e @ Self::NotFoundDatabase { .. } => e.not_found(),
-            e @ Self::NotFoundTable { .. } => e.not_found(),
             Self::ParseBody { source } => source.to_http_api_error(),
             e @ Self::ParsingDelete { .. } => e.invalid(),
             e @ Self::BuildingDeletePredicate { .. } => e.invalid(),
@@ -131,9 +127,6 @@ pub enum InnerDmlError {
     #[snafu(display("Database {} not found", db_name))]
     DatabaseNotFound { db_name: String },
 
-    #[snafu(display("Table {}:{} not found", db_name, table_name))]
-    TableNotFound { db_name: String, table_name: String },
-
     #[snafu(display("User-provoked error while processing DML request: {}", source))]
     UserError {
         db_name: String,
@@ -154,16 +147,6 @@ impl From<InnerDmlError> for HttpDmlError {
                 debug!(%db_name, "database not found");
                 Self::NotFoundDatabase { db_name }
             }
-            InnerDmlError::TableNotFound {
-                db_name,
-                table_name,
-            } => {
-                debug!(%db_name, %table_name, "table not found");
-                Self::NotFoundTable {
-                    db_name,
-                    table_name,
-                }
-            }
             InnerDmlError::UserError { db_name, source } => {
                 debug!(e=%source, %db_name, "error writing lines");
                 Self::WritingPointsUser { db_name, source }
@@ -266,9 +249,7 @@ pub trait HttpDrivenDml: ServerType {
                         .unwrap(),
                 ))
             }
-            Err(
-                e @ (InnerDmlError::DatabaseNotFound { .. } | InnerDmlError::TableNotFound { .. }),
-            ) => {
+            Err(e @ InnerDmlError::DatabaseNotFound { .. }) => {
                 // Purposefully do not record ingest metrics
                 Err(e.into())
             }
@@ -690,6 +671,7 @@ pub mod test_utils {
             .body(delete_line)
             .send()
             .await;
+
         check_response(
             "delete",
             response,
@@ -719,13 +701,8 @@ pub mod test_utils {
             .body(delete_line)
             .send()
             .await;
-        check_response(
-            "delete",
-            response,
-            StatusCode::NOT_FOUND,
-            Some("Table MyOrg_MyBucket:not_a_table not found"),
-        )
-        .await;
+
+        check_response("delete", response, StatusCode::NO_CONTENT, None).await;
     }
 
     /// Assert that deleting with a malformed body returns the expected message and error code.

influxdb_iox/src/influxdb_ioxd/server_type/database/http.rs

@@ -183,19 +183,11 @@ impl HttpDrivenDml for DatabaseServerType {
                 db_name: db_name.to_string(),
             })?;
 
-        db.store_operation(&op).map_err(|e| match e {
-            // TODO: missing tables should not be special-cased (#3259)
-            server::db::DmlError::TableNotFound { table_name, .. } => {
-                InnerDmlError::TableNotFound {
-                    table_name,
-                    db_name: db_name.to_string(),
-                }
-            }
-            e => InnerDmlError::UserError {
+        db.store_operation(&op)
+            .map_err(|e| InnerDmlError::UserError {
                 db_name: db_name.to_string(),
                 source: Box::new(e),
-            },
-        })
+            })
     }
 }
 

influxdb_iox/tests/end_to_end_cases/delete_api.rs

@@ -7,7 +7,6 @@ use data_types::{
 use dml::{test_util::assert_delete_op_eq, DmlDelete};
 use futures::StreamExt;
 use influxdb_iox_client::management::generated_types::DatabaseRules;
-use test_helpers::assert_contains;
 
 use super::scenario::{create_router_to_write_buffer, rand_name};
 use crate::common::server_fixture::{ServerFixture, ServerType};
@@ -122,14 +121,12 @@ async fn test_delete_on_database() {
     // ------------------------------------------
     // Negative Delete test to get error messages
 
-    // Delete from non-existing table
+    // Delete from non-existing table should be a no-op
     let table = "notable";
-    let del = delete_client
+    delete_client
         .delete(db_name.clone(), table, pred.into())
         .await
-        .unwrap_err()
-        .to_string();
-    assert_contains!(del, "Cannot delete data from non-existent table");
+        .unwrap();
 
     // Verify both existing tables still have the same data
     // query to verify data deleted

server/src/db.rs

@@ -62,7 +62,7 @@ use crate::{
             chunk::{CatalogChunk, ChunkStage},
             partition::Partition,
             table::TableSchemaUpsertHandle,
-            Catalog, Error as CatalogError, TableNameFilter,
+            Catalog, TableNameFilter,
         },
         lifecycle::{LockableCatalogChunk, LockableCatalogPartition},
     },
@@ -138,16 +138,6 @@ pub enum DmlError {
     #[snafu(display("writing only allowed through write buffer"))]
     WritingOnlyAllowedThroughWriteBuffer,
 
-    #[snafu(display(
-        "Cannot delete data from non-existent table, {}: {}",
-        table_name,
-        source
-    ))]
-    TableNotFound {
-        table_name: String,
-        source: CatalogError,
-    },
-
     #[snafu(display(
         "Storing database write failed with the following error(s), and possibly more: {}",
         errors.iter().map(ToString::to_string).collect::<Vec<_>>().join(", ")
@@ -531,7 +521,7 @@ impl Db {
 
     /// Delete data from a table on a specified predicate
     ///
-    /// Returns an error if the table cannot be found in the catalog
+    /// Delete is silently ignored if table cannot be found
     ///
     /// **WARNING: Only use that when no write buffer is used.**
     pub fn delete(
@@ -553,11 +543,7 @@ impl Db {
 
         // get all partitions of this table
         // Note: we need an additional scope here to convince rustc that the future produced by this function is sendable.
-        {
-            let table = self
-                .catalog
-                .table(table_name)
-                .context(TableNotFound { table_name })?;
+        if let Ok(table) = self.catalog.table(table_name) {
             let partitions = table.partitions();
             for partition in partitions {
                 let partition = partition.write();