fix: check table existence in create table procedure (#1880)

* fix: check table existence in table procedures

* fix: use correct error variant

* chore: address view comments

* chore: address comments

* test: change error code
This commit is contained in:
Yingwen
2023-07-04 23:01:27 +09:00
committed by GitHub
parent ccee60f37d
commit f37b394f1a
5 changed files with 83 additions and 85 deletions

106
Cargo.lock generated
View File

@@ -199,7 +199,7 @@ checksum = "8f1f8f5a6f3d50d89e3797d7593a50f96bb2aaa20ca0cc7be1fb673232c91d72"
[[package]]
name = "api"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"arrow-flight",
"common-base",
@@ -841,7 +841,7 @@ dependencies = [
[[package]]
name = "benchmarks"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"arrow",
"clap 4.3.2",
@@ -1224,7 +1224,7 @@ checksum = "37b2a672a2cb129a2e41c10b1224bb368f9f37a2b16b612598138befd7b37eb5"
[[package]]
name = "catalog"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"api",
"arc-swap",
@@ -1509,7 +1509,7 @@ checksum = "2da6da31387c7e4ef160ffab6d5e7f00c42626fe39aea70a7b0f1773f7dd6c1b"
[[package]]
name = "client"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"api",
"arrow-flight",
@@ -1535,7 +1535,7 @@ dependencies = [
"prost",
"rand",
"snafu",
"substrait 0.4.0",
"substrait 0.3.2",
"substrait 0.7.5",
"tokio",
"tokio-stream",
@@ -1572,7 +1572,7 @@ dependencies = [
[[package]]
name = "cmd"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"anymap",
"build-data",
@@ -1602,7 +1602,7 @@ dependencies = [
"servers",
"session",
"snafu",
"substrait 0.4.0",
"substrait 0.3.2",
"temp-env",
"tikv-jemallocator",
"tokio",
@@ -1634,7 +1634,7 @@ checksum = "55b672471b4e9f9e95499ea597ff64941a309b2cdbffcc46f2cc5e2d971fd335"
[[package]]
name = "common-base"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"anymap",
"bitvec",
@@ -1648,7 +1648,7 @@ dependencies = [
[[package]]
name = "common-catalog"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"async-trait",
"chrono",
@@ -1665,7 +1665,7 @@ dependencies = [
[[package]]
name = "common-datasource"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"arrow",
"arrow-schema",
@@ -1691,7 +1691,7 @@ dependencies = [
[[package]]
name = "common-error"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"snafu",
"strum",
@@ -1699,7 +1699,7 @@ dependencies = [
[[package]]
name = "common-function"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"arc-swap",
"chrono-tz 0.6.3",
@@ -1722,7 +1722,7 @@ dependencies = [
[[package]]
name = "common-function-macro"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"arc-swap",
"backtrace",
@@ -1738,7 +1738,7 @@ dependencies = [
[[package]]
name = "common-grpc"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"api",
"arrow-flight",
@@ -1768,7 +1768,7 @@ dependencies = [
[[package]]
name = "common-grpc-expr"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"api",
"async-trait",
@@ -1787,7 +1787,7 @@ dependencies = [
[[package]]
name = "common-mem-prof"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"common-error",
"snafu",
@@ -1800,7 +1800,7 @@ dependencies = [
[[package]]
name = "common-meta"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"api",
"async-stream",
@@ -1823,7 +1823,7 @@ dependencies = [
[[package]]
name = "common-pprof"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"common-error",
"pprof",
@@ -1834,7 +1834,7 @@ dependencies = [
[[package]]
name = "common-procedure"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"async-stream",
"async-trait",
@@ -1856,7 +1856,7 @@ dependencies = [
[[package]]
name = "common-procedure-test"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"async-trait",
"common-procedure",
@@ -1864,7 +1864,7 @@ dependencies = [
[[package]]
name = "common-query"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"api",
"async-trait",
@@ -1884,7 +1884,7 @@ dependencies = [
[[package]]
name = "common-recordbatch"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"common-error",
"datafusion",
@@ -1900,7 +1900,7 @@ dependencies = [
[[package]]
name = "common-runtime"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"async-trait",
"common-error",
@@ -1916,7 +1916,7 @@ dependencies = [
[[package]]
name = "common-telemetry"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"backtrace",
"common-error",
@@ -1941,7 +1941,7 @@ dependencies = [
[[package]]
name = "common-test-util"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"once_cell",
"rand",
@@ -1950,7 +1950,7 @@ dependencies = [
[[package]]
name = "common-time"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"chrono",
"chrono-tz 0.8.2",
@@ -2590,7 +2590,7 @@ dependencies = [
[[package]]
name = "datanode"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"api",
"async-compat",
@@ -2646,7 +2646,7 @@ dependencies = [
"sql",
"storage",
"store-api",
"substrait 0.4.0",
"substrait 0.3.2",
"table",
"table-procedure",
"tokio",
@@ -2660,7 +2660,7 @@ dependencies = [
[[package]]
name = "datatypes"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"arrow",
"arrow-array",
@@ -3101,7 +3101,7 @@ dependencies = [
[[package]]
name = "file-table-engine"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"async-trait",
"common-catalog",
@@ -3210,7 +3210,7 @@ dependencies = [
[[package]]
name = "frontend"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"api",
"async-compat",
@@ -3265,7 +3265,7 @@ dependencies = [
"storage",
"store-api",
"strfmt",
"substrait 0.4.0",
"substrait 0.3.2",
"table",
"tokio",
"toml",
@@ -4871,7 +4871,7 @@ checksum = "518ef76f2f87365916b142844c16d8fefd85039bc5699050210a7778ee1cd1de"
[[package]]
name = "log-store"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"arc-swap",
"async-stream",
@@ -5133,7 +5133,7 @@ dependencies = [
[[package]]
name = "meta-client"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"api",
"async-trait",
@@ -5161,7 +5161,7 @@ dependencies = [
[[package]]
name = "meta-srv"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"anymap",
"api",
@@ -5354,7 +5354,7 @@ dependencies = [
[[package]]
name = "mito"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"anymap",
"arc-swap",
@@ -5391,7 +5391,7 @@ dependencies = [
[[package]]
name = "mito2"
version = "0.4.0"
version = "0.3.2"
[[package]]
name = "moka"
@@ -5829,7 +5829,7 @@ dependencies = [
[[package]]
name = "object-store"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"anyhow",
"async-trait",
@@ -6223,7 +6223,7 @@ dependencies = [
[[package]]
name = "partition"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"api",
"async-trait",
@@ -6810,7 +6810,7 @@ dependencies = [
[[package]]
name = "promql"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"async-recursion",
"async-trait",
@@ -7060,7 +7060,7 @@ dependencies = [
[[package]]
name = "query"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"ahash 0.8.3",
"approx_eq",
@@ -7114,7 +7114,7 @@ dependencies = [
"stats-cli",
"store-api",
"streaming-stats",
"substrait 0.4.0",
"substrait 0.3.2",
"table",
"tokio",
"tokio-stream",
@@ -8290,7 +8290,7 @@ checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd"
[[package]]
name = "script"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"arrow",
"async-trait",
@@ -8545,7 +8545,7 @@ dependencies = [
[[package]]
name = "servers"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"aide",
"api",
@@ -8633,7 +8633,7 @@ dependencies = [
[[package]]
name = "session"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"arc-swap",
"common-catalog",
@@ -8908,7 +8908,7 @@ dependencies = [
[[package]]
name = "sql"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"api",
"common-base",
@@ -8955,7 +8955,7 @@ dependencies = [
[[package]]
name = "sqlness-runner"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"async-trait",
"client",
@@ -9137,7 +9137,7 @@ dependencies = [
[[package]]
name = "storage"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"arc-swap",
"arrow",
@@ -9190,7 +9190,7 @@ dependencies = [
[[package]]
name = "store-api"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"async-stream",
"async-trait",
@@ -9305,7 +9305,7 @@ dependencies = [
[[package]]
name = "substrait"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"async-recursion",
"async-trait",
@@ -9460,7 +9460,7 @@ dependencies = [
[[package]]
name = "table"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"anymap",
"async-trait",
@@ -9496,7 +9496,7 @@ dependencies = [
[[package]]
name = "table-procedure"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"async-trait",
"catalog",
@@ -9589,7 +9589,7 @@ dependencies = [
[[package]]
name = "tests-integration"
version = "0.4.0"
version = "0.3.2"
dependencies = [
"api",
"async-trait",

View File

@@ -28,7 +28,7 @@ use table::engine::{EngineContext, TableEngineProcedureRef, TableEngineRef, Tabl
use table::requests::{CreateTableRequest, OpenTableRequest};
use crate::error::{
AccessCatalogSnafu, DeserializeProcedureSnafu, SchemaNotFoundSnafu, SerializeProcedureSnafu,
AccessCatalogSnafu, DeserializeProcedureSnafu, SerializeProcedureSnafu, TableExistsSnafu,
};
/// Procedure to create a table.
@@ -132,23 +132,24 @@ impl CreateTableProcedure {
}
async fn on_prepare(&mut self) -> Result<Status> {
if !self
let table_exists = self
.catalog_manager
.schema_exist(
.table_exist(
&self.data.request.catalog_name,
&self.data.request.schema_name,
&self.data.request.table_name,
)
.await
.context(AccessCatalogSnafu)?
{
logging::error!(
"Failed to create table {}, schema not found",
self.data.table_ref(),
);
return SchemaNotFoundSnafu {
name: &self.data.request.schema_name,
}
.fail()?;
.context(AccessCatalogSnafu)?;
if table_exists {
return if self.data.request.create_if_not_exists {
Ok(Status::Done)
} else {
TableExistsSnafu {
name: &self.data.request.table_name,
}
.fail()?
};
}
self.data.state = CreateTableState::EngineCreateTable;
@@ -168,8 +169,9 @@ impl CreateTableProcedure {
// do this check as we might not submitted the subprocedure yet when the manager
// recover this procedure from procedure store.
logging::info!(
"On engine create table {}, subprocedure not found, sub_id: {}",
"On engine create table {}, table_id: {}, subprocedure not found, sub_id: {}",
self.data.request.table_name,
self.data.request.id,
sub_id
);
@@ -195,8 +197,9 @@ impl CreateTableProcedure {
}),
ProcedureState::Done => {
logging::info!(
"On engine create table {}, done, sub_id: {}",
"On engine create table {}, table_id: {}, done, sub_id: {}",
self.data.request.table_name,
self.data.request.id,
sub_id
);
// The sub procedure is done, we can execute next step.

View File

@@ -23,7 +23,7 @@ use common_procedure::{
};
use common_telemetry::logging;
use serde::{Deserialize, Serialize};
use snafu::{OptionExt, ResultExt};
use snafu::{ensure, ResultExt};
use table::engine::{EngineContext, TableEngineProcedureRef, TableReference};
use table::requests::DropTableRequest;
@@ -122,18 +122,21 @@ impl DropTableProcedure {
async fn on_prepare(&mut self) -> Result<Status> {
let request = &self.data.request;
// Ensure the table exists.
let _ = self
let table_exists = self
.catalog_manager
.table(
.table_exist(
&request.catalog_name,
&request.schema_name,
&request.table_name,
)
.await
.context(AccessCatalogSnafu)?
.context(TableNotFoundSnafu {
.context(AccessCatalogSnafu)?;
ensure!(
table_exists,
TableNotFoundSnafu {
name: &request.table_name,
})?;
}
);
self.data.state = DropTableState::RemoveFromCatalog;

View File

@@ -44,12 +44,6 @@ pub enum Error {
source: catalog::error::Error,
},
#[snafu(display("Catalog {} not found", name))]
CatalogNotFound { name: String },
#[snafu(display("Schema {} not found", name))]
SchemaNotFound { name: String },
#[snafu(display("Table {} not found", name))]
TableNotFound { name: String },
@@ -72,10 +66,8 @@ impl ErrorExt for Error {
}
InvalidRawSchema { source, .. } => source.status_code(),
AccessCatalog { source, .. } => source.status_code(),
CatalogNotFound { .. } | SchemaNotFound { .. } | TableExists { .. } => {
StatusCode::InvalidArguments
}
TableNotFound { .. } => StatusCode::TableNotFound,
TableExists { .. } => StatusCode::TableAlreadyExists,
}
}

View File

@@ -63,11 +63,11 @@ SELECT * FROM new_table;
ALTER TABLE new_table RENAME new_table;
Error: 1004(InvalidArguments), Table already exists: greptime.public.new_table
Error: 4000(TableAlreadyExists), Table already exists: greptime.public.new_table
ALTER TABLE new_table RENAME t;
Error: 1004(InvalidArguments), Table already exists: greptime.public.t
Error: 4000(TableAlreadyExists), Table already exists: greptime.public.t
DROP TABLE t;