diff --git a/generated_types/protos/influxdata/iox/management/v1/service.proto b/generated_types/protos/influxdata/iox/management/v1/service.proto index 5793bcd6e0..84fb5bed3e 100644 --- a/generated_types/protos/influxdata/iox/management/v1/service.proto +++ b/generated_types/protos/influxdata/iox/management/v1/service.proto @@ -494,5 +494,4 @@ message DeleteRequest { } message DeleteResponse { - // NGA todo: define an appropriate response } diff --git a/influxdb_iox_client/src/client/management.rs b/influxdb_iox_client/src/client/management.rs index b47e9e2c37..67cc361c5a 100644 --- a/influxdb_iox_client/src/client/management.rs +++ b/influxdb_iox_client/src/client/management.rs @@ -1057,7 +1057,6 @@ impl Client { _ => DeleteError::ServerError(status), })?; - // NGA todo: return a handle to the delete? Ok(()) } diff --git a/predicate/src/delete_predicate.rs b/predicate/src/delete_predicate.rs index ffba6d264c..625704ef4c 100644 --- a/predicate/src/delete_predicate.rs +++ b/predicate/src/delete_predicate.rs @@ -649,7 +649,6 @@ mod tests { assert_eq!(expected, result); } - // NGA todo: check content of error messages #[test] fn test_parse_delete_negative() { // invalid key diff --git a/src/influxdb_ioxd/http.rs b/src/influxdb_ioxd/http.rs index 18cc3c118f..2013ee8502 100644 --- a/src/influxdb_ioxd/http.rs +++ b/src/influxdb_ioxd/http.rs @@ -282,7 +282,7 @@ impl ApplicationError { Self::ParsingLineProtocol { .. } => self.bad_request(), Self::ParsingDelete { .. } => self.bad_request(), Self::BuildingDeletePredicate { .. } => self.bad_request(), - Self::ExecutingDelete { .. } => self.internal_error(), + Self::ExecutingDelete { .. } => self.bad_request(), Self::ReadingBodyAsGzip { .. } => self.bad_request(), Self::ClientHangup { .. } => self.bad_request(), Self::RouteNotFound { .. } => self.not_found(), @@ -1150,6 +1150,44 @@ mod tests { "+----------------+--------------+-------+-----------------+----------------------+", ]; assert_batches_eq!(expected, &batches); + + // ------------------- + // negative tests + // Not able to parse _measurement="not_a_table" (it must be _measurement=\"not_a_table\" to work) + let delete_line = r#"{"start":"2021-04-01T14:00:00Z","stop":"2021-04-02T14:00:00Z", "predicate":"_measurement="not_a_table" and location=Boston"}"#; + let response = client + .post(&format!( + "{}/api/v2/delete?bucket={}&org={}", + server_url, bucket_name, org_name + )) + .body(delete_line) + .send() + .await; + check_response( + "delete", + response, + StatusCode::BAD_REQUEST, + Some("Unable to parse delete string"), + ) + .await; + + // delete from non-existing table + let delete_line = r#"{"start":"2021-04-01T14:00:00Z","stop":"2021-04-02T14:00:00Z", "predicate":"_measurement=not_a_table and location=Boston"}"#; + let response = client + .post(&format!( + "{}/api/v2/delete?bucket={}&org={}", + server_url, bucket_name, org_name + )) + .body(delete_line) + .send() + .await; + check_response( + "delete", + response, + StatusCode::BAD_REQUEST, + Some("Cannot delete data from non-existing table"), + ) + .await; } #[tokio::test] @@ -1607,7 +1645,7 @@ mod tests { assert_eq!(status, expected_status); if let Some(expected_body) = expected_body { - assert_eq!(body, expected_body); + assert!(body.contains(expected_body)); } } else { panic!("Unexpected error response: {:?}", response); diff --git a/src/influxdb_ioxd/rpc/error.rs b/src/influxdb_ioxd/rpc/error.rs index 442c2885fb..4fb78f0e6b 100644 --- a/src/influxdb_ioxd/rpc/error.rs +++ b/src/influxdb_ioxd/rpc/error.rs @@ -177,6 +177,12 @@ pub fn default_db_error_handler(error: server::db::Error) -> tonic::Status { ), } .into(), + Error::DeleteFromTable { source, table_name } => PreconditionViolation { + category: "database".to_string(), + subject: "influxdata.com/iox".to_string(), + description: format!("Cannot delete data from table: {} : {}", table_name, source), + } + .into(), Error::CatalogError { source } => default_catalog_error_handler(source), error => { error!(?error, "Unexpected error"); diff --git a/src/influxdb_ioxd/rpc/management.rs b/src/influxdb_ioxd/rpc/management.rs index a3f3a48e01..4d8bb67914 100644 --- a/src/influxdb_ioxd/rpc/management.rs +++ b/src/influxdb_ioxd/rpc/management.rs @@ -633,14 +633,13 @@ where })) } Ok(del_predicate) => { - //execute delete + // execute delete db.delete(&table_name, Arc::new(del_predicate)) .await .map_err(default_db_error_handler)?; } } - // NGA todo: return a delete handle with the response? Ok(Response::new(DeleteResponse {})) } } diff --git a/tests/end_to_end_cases/management_api.rs b/tests/end_to_end_cases/management_api.rs index 2f31a2f62d..eaeb6ff923 100644 --- a/tests/end_to_end_cases/management_api.rs +++ b/tests/end_to_end_cases/management_api.rs @@ -1587,8 +1587,10 @@ async fn test_delete() { let pred = "region = west"; let del = management_client .delete(db_name.clone(), table, start, stop, pred) - .await; - assert!(del.is_err()); + .await + .unwrap_err() + .to_string(); + assert!(del.contains("Cannot delete data from table")); // Verify both existing tables still have the same data // query to verify data deleted