Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: resolve more integration tests #2406

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/frontend/src/error.rs
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
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
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
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
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
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
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
Loading