Skip to content

Commit

Permalink
fix: resolve more integration tests (#2406)
Browse files Browse the repository at this point in the history 
* fix: resolve more integration tests

* Update tests-integration/tests/http.rs

Co-authored-by: Weny Xu <[email protected]>

---------

Co-authored-by: Weny Xu <[email protected]>
  • Loading branch information
@MichaelScofield @WenyXu
MichaelScofield and WenyXu authored Sep 15, 2023
1 parent 9572b1e commit 4b13c88
Show file tree
Hide file tree
Showing 13 changed files with 220 additions and 263 deletions.
10 changes: 5 additions & 5 deletions src/frontend/src/error.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,8 @@ pub enum Error {
#[snafu(display("Invalid SQL, error: {}", err_msg))]
InvalidSql { err_msg: String, location: Location },

#[snafu(display("Incomplete GRPC result: {}", err_msg))]
IncompleteGrpcResult { err_msg: String, location: Location },
#[snafu(display("Incomplete GRPC request: {}", err_msg))]
IncompleteGrpcRequest { err_msg: String, location: Location },

#[snafu(display("Failed to find Datanode by region: {:?}", region))]
FindDatanode {
Expand Down Expand Up @@ -713,7 +713,8 @@ impl ErrorExt for Error {
| Error::ProjectSchema { .. }
| Error::UnsupportedFormat { .. }
| Error::EmptyData { .. }
| Error::ColumnNoneDefaultValue { .. } => StatusCode::InvalidArguments,
| Error::ColumnNoneDefaultValue { .. }
| Error::IncompleteGrpcRequest { .. } => StatusCode::InvalidArguments,

Error::NotSupported { .. } => StatusCode::Unsupported,

Expand Down Expand Up @@ -770,8 +771,7 @@ impl ErrorExt for Error {
| Error::MissingInsertBody { .. }
| Error::InvalidRegionRequest { .. } => StatusCode::Internal,

Error::IncompleteGrpcResult { .. }
| Error::ContextValueNotFound { .. }
Error::ContextValueNotFound { .. }
| Error::InvalidSystemTableDef { .. }
| Error::EncodeJson { .. } => StatusCode::Unexpected,

Expand Down
46 changes: 39 additions & 7 deletions src/frontend/src/instance/grpc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use api::v1::ddl_request::Expr as DdlExpr;
use api::v1::ddl_request::{Expr as DdlExpr, Expr};
use api::v1::greptime_request::Request;
use api::v1::query_request::Query;
use api::v1::{DeleteRequests, InsertRequests, RowDeleteRequests, RowInsertRequests};
Expand All @@ -27,9 +27,7 @@ use servers::query_handler::sql::SqlQueryHandler;
use session::context::QueryContextRef;
use snafu::{ensure, OptionExt, ResultExt};

use crate::error::{
self, Error, IncompleteGrpcResultSnafu, NotSupportedSnafu, PermissionSnafu, Result,
};
use crate::error::{Error, IncompleteGrpcRequestSnafu, NotSupportedSnafu, PermissionSnafu, Result};
use crate::instance::Instance;

#[async_trait]
Expand All @@ -53,7 +51,7 @@ impl GrpcQueryHandler for Instance {
Request::Deletes(requests) => self.handle_deletes(requests, ctx.clone()).await?,
Request::RowDeletes(requests) => self.handle_row_deletes(requests, ctx.clone()).await?,
Request::Query(query_request) => {
let query = query_request.query.context(IncompleteGrpcResultSnafu {
let query = query_request.query.context(IncompleteGrpcRequestSnafu {
err_msg: "Missing field 'QueryRequest.query'",
})?;
match query {
Expand Down Expand Up @@ -93,10 +91,12 @@ impl GrpcQueryHandler for Instance {
}
}
Request::Ddl(request) => {
let expr = request.expr.context(error::UnexpectedSnafu {
violated: "expected expr",
let mut expr = request.expr.context(IncompleteGrpcRequestSnafu {
err_msg: "'expr' is absent in DDL request",
})?;

fill_catalog_and_schema_from_context(&mut expr, &ctx);

match expr {
DdlExpr::CreateTable(mut expr) => {
// TODO(weny): supports to create multiple region table.
Expand Down Expand Up @@ -135,6 +135,38 @@ impl GrpcQueryHandler for Instance {
}
}

fn fill_catalog_and_schema_from_context(ddl_expr: &mut DdlExpr, ctx: &QueryContextRef) {
let catalog = ctx.current_catalog();
let schema = ctx.current_schema();

macro_rules! check_and_fill {
($expr:ident) => {
if $expr.catalog_name.is_empty() {
$expr.catalog_name = catalog.to_string();
}
if $expr.schema_name.is_empty() {
$expr.schema_name = schema.to_string();
}
};
}

match ddl_expr {
Expr::CreateDatabase(_) => { /* do nothing*/ }
Expr::CreateTable(expr) => {
check_and_fill!(expr);
}
Expr::Alter(expr) => {
check_and_fill!(expr);
}
Expr::DropTable(expr) => {
check_and_fill!(expr);
}
Expr::TruncateTable(expr) => {
check_and_fill!(expr);
}
}
}

impl Instance {
pub async fn handle_inserts(
&self,
Expand Down
8 changes: 3 additions & 5 deletions src/sql/src/statements/create.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,7 @@ impl Display for Partitions {
if !self.column_list.is_empty() {
write!(
f,
r#"PARTITION BY RANGE COLUMNS ({}) (
{}
)"#,
"PARTITION BY RANGE COLUMNS ({}) (\n{}\n)",
format_list_comma!(self.column_list),
format_list_indent!(self.entries),
)
Expand Down Expand Up @@ -261,10 +259,10 @@ CREATE TABLE IF NOT EXISTS demo (
PRIMARY KEY (ts, host)
)
PARTITION BY RANGE COLUMNS (ts) (
PARTITION r0 VALUES LESS THAN (5),
PARTITION r0 VALUES LESS THAN (5),
PARTITION r1 VALUES LESS THAN (9),
PARTITION r2 VALUES LESS THAN (MAXVALUE)
)
)
ENGINE=mito
WITH(
regions = 1,
Expand Down
5 changes: 4 additions & 1 deletion tests-integration/src/cluster.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@ use tonic::transport::Server;
use tower::service_fn;

use crate::test_util::{
create_datanode_opts, create_tmp_dir_and_datanode_opts, FileDirGuard, StorageGuard, StorageType,
self, create_datanode_opts, create_tmp_dir_and_datanode_opts, FileDirGuard, StorageGuard,
StorageType,
};

pub struct GreptimeDbCluster {
Expand Down Expand Up @@ -101,6 +102,8 @@ impl GreptimeDbClusterBuilder {
.build_frontend(meta_srv.clone(), datanode_clients)
.await;

test_util::prepare_another_catalog_and_schema(frontend.as_ref()).await;

frontend.start().await.unwrap();

GreptimeDbCluster {
Expand Down
31 changes: 17 additions & 14 deletions tests-integration/src/grpc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ CREATE TABLE {table_name} (
}

async fn test_insert_delete_and_query_on_existing_table(instance: &Instance, table_name: &str) {
let ts_millisecond_values = vec![
let timestamp_millisecond_values = vec![
1672557972000,
1672557973000,
1672557974000,
Expand Down Expand Up @@ -341,7 +341,7 @@ CREATE TABLE {table_name} (
Column {
column_name: "b".to_string(),
values: Some(Values {
string_values: ts_millisecond_values
string_values: timestamp_millisecond_values
.iter()
.map(|x| format!("ts: {x}"))
.collect(),
Expand All @@ -354,7 +354,7 @@ CREATE TABLE {table_name} (
Column {
column_name: "ts".to_string(),
values: Some(Values {
ts_millisecond_values,
timestamp_millisecond_values,
..Default::default()
}),
semantic_type: SemanticType::Timestamp as i32,
Expand All @@ -363,7 +363,6 @@ CREATE TABLE {table_name} (
},
],
row_count: 16,
..Default::default()
};
let output = query(
instance,
Expand Down Expand Up @@ -409,7 +408,6 @@ CREATE TABLE {table_name} (

let new_grpc_delete_request = |a, b, ts, row_count| DeleteRequest {
table_name: table_name.to_string(),
region_number: 0,
key_columns: vec![
Column {
column_name: "a".to_string(),
Expand All @@ -435,7 +433,7 @@ CREATE TABLE {table_name} (
column_name: "ts".to_string(),
semantic_type: SemanticType::Timestamp as i32,
values: Some(Values {
ts_millisecond_values: ts,
timestamp_millisecond_values: ts,
..Default::default()
}),
datatype: ColumnDataType::TimestampMillisecond as i32,
Expand Down Expand Up @@ -545,6 +543,7 @@ CREATE TABLE {table_name} (
.handle_read(RegionQueryRequest {
region_id: region_id.as_u64(),
plan: plan.to_vec(),
..Default::default()
})
.await
.unwrap();
Expand Down Expand Up @@ -574,7 +573,11 @@ CREATE TABLE {table_name} (
Column {
column_name: "ts".to_string(),
values: Some(Values {
ts_millisecond_values: vec![1672557975000, 1672557976000, 1672557977000],
timestamp_millisecond_values: vec![
1672557975000,
1672557976000,
1672557977000,
],
..Default::default()
}),
semantic_type: SemanticType::Timestamp as i32,
Expand All @@ -583,7 +586,6 @@ CREATE TABLE {table_name} (
},
],
row_count: 3,
..Default::default()
};

// Test auto create not existed table upon insertion.
Expand All @@ -609,7 +611,11 @@ CREATE TABLE {table_name} (
Column {
column_name: "ts".to_string(),
values: Some(Values {
ts_millisecond_values: vec![1672557978000, 1672557979000, 1672557980000],
timestamp_millisecond_values: vec![
1672557978000,
1672557979000,
1672557980000,
],
..Default::default()
}),
semantic_type: SemanticType::Timestamp as i32,
Expand All @@ -618,7 +624,6 @@ CREATE TABLE {table_name} (
},
],
row_count: 3,
..Default::default()
};

// Test auto add not existed column upon insertion.
Expand Down Expand Up @@ -653,11 +658,10 @@ CREATE TABLE {table_name} (

let delete = DeleteRequest {
table_name: "auto_created_table".to_string(),
region_number: 0,
key_columns: vec![Column {
column_name: "ts".to_string(),
values: Some(Values {
ts_millisecond_values: vec![1672557975000, 1672557979000],
timestamp_millisecond_values: vec![1672557975000, 1672557979000],
..Default::default()
}),
semantic_type: SemanticType::Timestamp as i32,
Expand Down Expand Up @@ -739,7 +743,7 @@ CREATE TABLE {table_name} (
Column {
column_name: "ts".to_string(),
values: Some(Values {
ts_millisecond_values: vec![
timestamp_millisecond_values: vec![
1672557972000,
1672557973000,
1672557974000,
Expand All @@ -757,7 +761,6 @@ CREATE TABLE {table_name} (
},
],
row_count: 8,
..Default::default()
};

let request = Request::Inserts(InsertRequests {
Expand Down
1 change: 1 addition & 0 deletions tests-integration/src/instance.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ mod tests {
.handle_read(QueryRequest {
region_id: region_id.as_u64(),
plan: plan.to_vec(),
..Default::default()
})
.await
.unwrap();
Expand Down
13 changes: 8 additions & 5 deletions tests-integration/src/standalone.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,15 @@

use std::sync::Arc;

use catalog::kvbackend::DummyKvCacheInvalidator;
use catalog::kvbackend::{DummyKvCacheInvalidator, KvBackendCatalogManager};
use common_base::Plugins;
use common_config::KvStoreConfig;
use common_procedure::options::ProcedureConfig;
use datanode::datanode::builder::DatanodeBuilder;
use datanode::datanode::DatanodeOptions;
use frontend::catalog::FrontendCatalogManager;
use frontend::instance::{Instance, StandaloneDatanodeManager};
use frontend::instance::{FrontendInstance, Instance, StandaloneDatanodeManager};

use crate::test_util::{create_tmp_dir_and_datanode_opts, StorageType, TestGuard};
use crate::test_util::{self, create_tmp_dir_and_datanode_opts, StorageType, TestGuard};

pub struct GreptimeDbStandalone {
pub instance: Arc<Instance>,
Expand Down Expand Up @@ -81,7 +80,7 @@ impl GreptimeDbStandaloneBuilder {
.await
.unwrap();

let catalog_manager = FrontendCatalogManager::new(
let catalog_manager = KvBackendCatalogManager::new(
kv_store.clone(),
Arc::new(DummyKvCacheInvalidator),
Arc::new(StandaloneDatanodeManager(datanode.region_server())),
Expand All @@ -103,6 +102,10 @@ impl GreptimeDbStandaloneBuilder {
.await
.unwrap();

test_util::prepare_another_catalog_and_schema(&instance).await;

instance.start().await.unwrap();

GreptimeDbStandalone {
instance: Arc::new(instance),
datanode_opts: opts,
Expand Down
Loading

0 comments on commit 4b13c88

Please sign in to comment.