From 5908febd6c8b35c3a2e076c05d6a0ec5b0f2754a Mon Sep 17 00:00:00 2001 From: Zhenchi Date: Thu, 24 Jul 2025 15:55:51 +0800 Subject: [PATCH] refactor: remove unused PartitionDef (#6573) * refactor: remove unused PartitionDef Signed-off-by: Zhenchi * fix snafu Signed-off-by: Zhenchi --------- Signed-off-by: Zhenchi --- src/common/meta/src/ddl/table_meta.rs | 1 + src/common/meta/src/rpc/ddl.rs | 1 - src/common/meta/src/rpc/router.rs | 15 -- src/operator/src/error.rs | 18 +- src/operator/src/statement/ddl.rs | 39 ++--- src/operator/src/tests/partition_manager.rs | 19 +-- src/partition/src/expr.rs | 9 + src/partition/src/partition.rs | 161 +----------------- tests-fuzz/src/context.rs | 3 +- tests-fuzz/src/generator/create_expr.rs | 49 ++---- tests-fuzz/src/ir/create_expr.rs | 8 +- .../src/translator/mysql/create_expr.rs | 42 ++--- 12 files changed, 79 insertions(+), 286 deletions(-) diff --git a/src/common/meta/src/ddl/table_meta.rs b/src/common/meta/src/ddl/table_meta.rs index 1103dc2bda..96bcff4a23 100644 --- a/src/common/meta/src/ddl/table_meta.rs +++ b/src/common/meta/src/ddl/table_meta.rs @@ -138,6 +138,7 @@ impl TableMetadataAllocator { }) .collect::>(); + // If the table has no partitions, we need to create a default region. if region_routes.is_empty() { region_routes.push(RegionRoute { region: Region { diff --git a/src/common/meta/src/rpc/ddl.rs b/src/common/meta/src/rpc/ddl.rs index 74ca629a80..13d6736390 100644 --- a/src/common/meta/src/rpc/ddl.rs +++ b/src/common/meta/src/rpc/ddl.rs @@ -606,7 +606,6 @@ impl From for PbDropTableTask { #[derive(Debug, PartialEq, Clone)] pub struct CreateTableTask { pub create_table: CreateTableExpr, - // TODO(zhongzc): change to `Vec` pub partitions: Vec, pub table_info: RawTableInfo, } diff --git a/src/common/meta/src/rpc/router.rs b/src/common/meta/src/rpc/router.rs index efe2af2c7a..599d2d1aad 100644 --- a/src/common/meta/src/rpc/router.rs +++ b/src/common/meta/src/rpc/router.rs @@ -482,21 +482,6 @@ where Ok(values) } -impl From for PbPartition { - fn from(p: LegacyPartition) -> Self { - let expression = if !p.value_list.is_empty() { - String::from_utf8_lossy(&p.value_list[0]).to_string() - } else { - "".to_string() - }; - - Self { - expression, - ..Default::default() - } - } -} - #[cfg(test)] mod tests { use super::*; diff --git a/src/operator/src/error.rs b/src/operator/src/error.rs index b023530b7b..0c96a419de 100644 --- a/src/operator/src/error.rs +++ b/src/operator/src/error.rs @@ -419,13 +419,6 @@ pub enum Error { source: datatypes::error::Error, }, - #[snafu(display("Failed to deserialize partition in meta to partition def"))] - DeserializePartition { - #[snafu(implicit)] - location: Location, - source: partition::error::Error, - }, - #[snafu(display("Failed to describe schema for given statement"))] DescribeStatement { #[snafu(implicit)] @@ -851,6 +844,13 @@ pub enum Error { #[snafu(implicit)] location: Location, }, + + #[snafu(display("Failed to convert partition expression to protobuf"))] + PartitionExprToPb { + source: partition::error::Error, + #[snafu(implicit)] + location: Location, + }, } pub type Result = std::result::Result; @@ -891,6 +891,7 @@ impl ErrorExt for Error { | Error::InvalidPartition { .. } | Error::PhysicalExpr { .. } | Error::InvalidJsonFormat { .. } + | Error::PartitionExprToPb { .. } | Error::CursorNotFound { .. } | Error::CursorExists { .. } | Error::CreatePartitionRules { .. } => StatusCode::InvalidArguments, @@ -947,8 +948,7 @@ impl ErrorExt for Error { | Error::DescribeStatement { source, .. } => source.status_code(), Error::AlterExprToRequest { source, .. } => source.status_code(), Error::External { source, .. } => source.status_code(), - Error::DeserializePartition { source, .. } - | Error::FindTablePartitionRule { source, .. } + Error::FindTablePartitionRule { source, .. } | Error::SplitInsert { source, .. } | Error::SplitDelete { source, .. } | Error::FindRegionLeader { source, .. } => source.status_code(), diff --git a/src/operator/src/statement/ddl.rs b/src/operator/src/statement/ddl.rs index 472a57eb25..8832d6bd3d 100644 --- a/src/operator/src/statement/ddl.rs +++ b/src/operator/src/statement/ddl.rs @@ -43,7 +43,6 @@ use common_meta::rpc::ddl::{ CreateFlowTask, DdlTask, DropFlowTask, DropViewTask, SubmitDdlTaskRequest, SubmitDdlTaskResponse, }; -use common_meta::rpc::router::{LegacyPartition, LegacyPartition as MetaPartition}; use common_query::Output; use common_sql::convert::sql_value_to_value; use common_telemetry::{debug, info, tracing, warn}; @@ -56,7 +55,6 @@ use datatypes::value::Value; use lazy_static::lazy_static; use partition::expr::{Operand, PartitionExpr, RestrictedOp}; use partition::multi_dim::MultiDimPartitionRule; -use partition::partition::{PartitionBound, PartitionDef}; use query::parser::QueryStatement; use query::plan::extract_and_rewrite_full_table_names; use query::query_engine::DefaultSerializer; @@ -85,9 +83,9 @@ use table::TableRef; use crate::error::{ self, AlterExprToRequestSnafu, BuildDfLogicalPlanSnafu, CatalogSnafu, ColumnDataTypeSnafu, ColumnNotFoundSnafu, ConvertSchemaSnafu, CreateLogicalTablesSnafu, CreateTableInfoSnafu, - DeserializePartitionSnafu, EmptyDdlExprSnafu, ExternalSnafu, ExtractTableNamesSnafu, - FlowNotFoundSnafu, InvalidPartitionRuleSnafu, InvalidPartitionSnafu, InvalidSqlSnafu, - InvalidTableNameSnafu, InvalidViewNameSnafu, InvalidViewStmtSnafu, Result, SchemaInUseSnafu, + EmptyDdlExprSnafu, ExternalSnafu, ExtractTableNamesSnafu, FlowNotFoundSnafu, + InvalidPartitionRuleSnafu, InvalidPartitionSnafu, InvalidSqlSnafu, InvalidTableNameSnafu, + InvalidViewNameSnafu, InvalidViewStmtSnafu, PartitionExprToPbSnafu, Result, SchemaInUseSnafu, SchemaNotFoundSnafu, SchemaReadOnlySnafu, SubstraitCodecSnafu, TableAlreadyExistsSnafu, TableMetadataManagerSnafu, TableNotFoundSnafu, UnrecognizedTableOptionSnafu, ViewAlreadyExistsSnafu, @@ -1337,11 +1335,14 @@ impl StatementExecutor { async fn create_table_procedure( &self, create_table: CreateTableExpr, - partitions: Vec, + partitions: Vec, table_info: RawTableInfo, query_context: QueryContextRef, ) -> Result { - let partitions = partitions.into_iter().map(Into::into).collect(); + let partitions = partitions + .into_iter() + .map(|expr| expr.as_pb_partition().context(PartitionExprToPbSnafu)) + .collect::>>()?; let request = SubmitDdlTaskRequest { query_context, @@ -1533,36 +1534,24 @@ impl StatementExecutor { } } -/// Parse partition statement [Partitions] into [MetaPartition] and partition columns. +/// Parse partition statement [Partitions] into [PartitionExpr] and partition columns. pub fn parse_partitions( create_table: &CreateTableExpr, partitions: Option, query_ctx: &QueryContextRef, -) -> Result<(Vec, Vec)> { +) -> Result<(Vec, Vec)> { // If partitions are not defined by user, use the timestamp column (which has to be existed) as // the partition column, and create only one partition. let partition_columns = find_partition_columns(&partitions)?; - let partition_entries = + let partition_exprs = find_partition_entries(create_table, &partitions, &partition_columns, query_ctx)?; // Validates partition - let exprs = partition_entries.clone(); + let exprs = partition_exprs.clone(); MultiDimPartitionRule::try_new(partition_columns.clone(), vec![], exprs, true) .context(InvalidPartitionSnafu)?; - // TODO(zhongzc): change to PartitionExpr - let meta_partitions: Vec = partition_entries - .into_iter() - .map(|x| { - MetaPartition::try_from(PartitionDef::new( - partition_columns.clone(), - vec![PartitionBound::Expr(x)], - )) - }) - .collect::>() - .context(DeserializePartitionSnafu)?; - - Ok((meta_partitions, partition_columns)) + Ok((partition_exprs, partition_columns)) } /// Verifies an alter and returns whether it is necessary to perform the alter. @@ -1720,7 +1709,7 @@ fn find_partition_columns(partitions: &Option) -> Result /// Parse [Partitions] into a group of partition entries. /// -/// Returns a list of [PartitionBound], each of which defines a partition. +/// Returns a list of [PartitionExpr], each of which defines a partition. fn find_partition_entries( create_table: &CreateTableExpr, partitions: &Option, diff --git a/src/operator/src/tests/partition_manager.rs b/src/operator/src/tests/partition_manager.rs index 297596dd00..c9f6a5cbff 100644 --- a/src/operator/src/tests/partition_manager.rs +++ b/src/operator/src/tests/partition_manager.rs @@ -20,13 +20,12 @@ use common_meta::key::table_route::TableRouteValue; use common_meta::key::TableMetadataManager; use common_meta::kv_backend::KvBackendRef; use common_meta::peer::Peer; -use common_meta::rpc::router::{Region, RegionRoute}; +use common_meta::rpc::router::{LegacyPartition, Region, RegionRoute}; use datatypes::prelude::ConcreteDataType; use datatypes::schema::{ColumnSchema, SchemaBuilder}; use moka::future::CacheBuilder; use partition::expr::{Operand, PartitionExpr, RestrictedOp}; use partition::manager::{PartitionRuleManager, PartitionRuleManagerRef}; -use partition::partition::{PartitionBound, PartitionDef}; use store_api::storage::RegionNumber; use table::metadata::{TableInfo, TableInfoBuilder, TableMetaBuilder}; @@ -163,18 +162,10 @@ pub(crate) async fn create_partition_rule_manager( id: 1.into(), name: "r3".to_string(), // Keep the old partition definition to test compatibility. - partition: Some( - PartitionDef::new( - vec!["a".to_string()], - vec![PartitionBound::Expr(PartitionExpr::new( - Operand::Column("a".to_string()), - RestrictedOp::GtEq, - Operand::Value(datatypes::value::Value::Int32(50)), - ))], - ) - .try_into() - .unwrap(), - ), + partition: Some(LegacyPartition { + column_list: vec![b"a".to_vec()], + value_list: vec![b"{\"Expr\":{\"lhs\":{\"Column\":\"a\"},\"op\":\"GtEq\",\"rhs\":{\"Value\":{\"Int32\":50}}}}".to_vec()], + }), attrs: BTreeMap::new(), partition_expr: Default::default(), }, diff --git a/src/partition/src/expr.rs b/src/partition/src/expr.rs index 67b8e36234..382a134e09 100644 --- a/src/partition/src/expr.rs +++ b/src/partition/src/expr.rs @@ -15,6 +15,7 @@ use std::fmt::{Debug, Display, Formatter}; use std::sync::Arc; +use api::v1::meta::Partition; use datafusion_common::{ScalarValue, ToDFSchema}; use datafusion_expr::execution_props::ExecutionProps; use datafusion_expr::Expr; @@ -291,6 +292,14 @@ impl PartitionExpr { _ => Ok(None), } } + + /// Converts [Self] to [Partition]. + pub fn as_pb_partition(&self) -> error::Result { + Ok(Partition { + expression: self.as_json_str()?, + ..Default::default() + }) + } } impl Display for PartitionExpr { diff --git a/src/partition/src/partition.rs b/src/partition/src/partition.rs index 67da449e62..110f61a39e 100644 --- a/src/partition/src/partition.rs +++ b/src/partition/src/partition.rs @@ -14,18 +14,15 @@ use std::any::Any; use std::collections::HashMap; -use std::fmt::{Debug, Display, Formatter}; +use std::fmt::Debug; use std::sync::Arc; -use common_meta::rpc::router::LegacyPartition as MetaPartition; use datatypes::arrow::array::{BooleanArray, RecordBatch}; use datatypes::prelude::Value; -use itertools::Itertools; use serde::{Deserialize, Serialize}; -use snafu::ResultExt; use store_api::storage::RegionNumber; -use crate::error::{self, Error, Result}; +use crate::error::Result; pub type PartitionRuleRef = Arc; @@ -58,106 +55,6 @@ pub enum PartitionBound { Expr(crate::expr::PartitionExpr), } -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct PartitionDef { - partition_columns: Vec, - partition_bounds: Vec, -} - -impl Display for PartitionBound { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - match self { - Self::Value(v) => write!(f, "{}", v), - Self::MaxValue => write!(f, "MAXVALUE"), - Self::Expr(e) => write!(f, "{}", e), - } - } -} - -impl Display for PartitionDef { - fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!( - f, - "{}", - self.partition_bounds - .iter() - .map(|b| format!("{b}")) - .join(", ") - ) - } -} - -impl PartitionDef { - pub fn new(partition_columns: Vec, partition_bounds: Vec) -> Self { - Self { - partition_columns, - partition_bounds, - } - } - - pub fn partition_columns(&self) -> &Vec { - &self.partition_columns - } - - pub fn partition_bounds(&self) -> &Vec { - &self.partition_bounds - } -} - -impl TryFrom<&MetaPartition> for PartitionDef { - type Error = Error; - - fn try_from(partition: &MetaPartition) -> Result { - let MetaPartition { - column_list, - value_list, - } = partition; - - let partition_columns = column_list - .iter() - .map(|x| String::from_utf8_lossy(x).to_string()) - .collect::>(); - - let partition_bounds = value_list - .iter() - .map(|x| serde_json::from_str(&String::from_utf8_lossy(x))) - .collect::, serde_json::Error>>() - .context(error::DeserializeJsonSnafu)?; - - Ok(PartitionDef { - partition_columns, - partition_bounds, - }) - } -} - -impl TryFrom for MetaPartition { - type Error = Error; - - fn try_from(partition: PartitionDef) -> Result { - let PartitionDef { - partition_columns: columns, - partition_bounds: bounds, - } = partition; - - let column_list = columns - .into_iter() - .map(|x| x.into_bytes()) - .collect::>>(); - - let value_list = bounds - .into_iter() - .map(|x| serde_json::to_string(&x).map(|s| s.into_bytes())) - .collect::>, serde_json::Error>>() - .context(error::SerializeJsonSnafu)?; - - Ok(MetaPartition { - column_list, - value_list, - }) - } -} - pub struct RegionMask { array: BooleanArray, selected_rows: usize, @@ -199,57 +96,3 @@ impl RegionMask { self.selected_rows } } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_partition_def() { - // PartitionDef -> MetaPartition - let def = PartitionDef { - partition_columns: vec!["a".to_string(), "b".to_string()], - partition_bounds: vec![ - PartitionBound::MaxValue, - PartitionBound::Value(1_i32.into()), - ], - }; - assert_eq!("MAXVALUE, 1", def.to_string()); - - let partition: MetaPartition = def.try_into().unwrap(); - assert_eq!( - r#"{"column_list":["a","b"],"value_list":["\"MaxValue\"","{\"Value\":{\"Int32\":1}}"]}"#, - serde_json::to_string(&partition).unwrap(), - ); - - // MetaPartition -> PartitionDef - let partition = &MetaPartition { - column_list: vec![b"a".to_vec(), b"b".to_vec()], - value_list: vec![ - b"\"MaxValue\"".to_vec(), - b"{\"Value\":{\"Int32\":1}}".to_vec(), - ], - }; - let def: PartitionDef = partition.try_into().unwrap(); - assert_eq!( - def.partition_columns, - vec!["a".to_string(), "b".to_string()] - ); - assert_eq!( - def.partition_bounds, - vec![ - PartitionBound::MaxValue, - PartitionBound::Value(1_i32.into()) - ] - ); - } - - #[test] - fn test_partition_bound() { - let b1 = PartitionBound::Value(1_i32.into()); - let b2 = PartitionBound::Value(100_i32.into()); - let b3 = PartitionBound::MaxValue; - assert!(b1 < b2); - assert!(b2 < b3); - } -} diff --git a/tests-fuzz/src/context.rs b/tests-fuzz/src/context.rs index 5678399f8b..9311d5c19e 100644 --- a/tests-fuzz/src/context.rs +++ b/tests-fuzz/src/context.rs @@ -16,14 +16,13 @@ use std::sync::Arc; use common_query::AddColumnLocation; use datatypes::types::cast; -use partition::partition::PartitionDef; use rand::Rng; use snafu::{ensure, OptionExt}; use crate::error::{self, Result}; use crate::generator::Random; use crate::ir::alter_expr::{AlterTableOperation, AlterTableOption}; -use crate::ir::create_expr::ColumnOption; +use crate::ir::create_expr::{ColumnOption, PartitionDef}; use crate::ir::{AlterTableExpr, Column, CreateTableExpr, Ident}; pub type TableContextRef = Arc; diff --git a/tests-fuzz/src/generator/create_expr.rs b/tests-fuzz/src/generator/create_expr.rs index 44a60257aa..1291e2749a 100644 --- a/tests-fuzz/src/generator/create_expr.rs +++ b/tests-fuzz/src/generator/create_expr.rs @@ -18,7 +18,6 @@ use datatypes::data_type::ConcreteDataType; use datatypes::value::Value; use derive_builder::Builder; use partition::expr::{Operand, PartitionExpr, RestrictedOp}; -use partition::partition::{PartitionBound, PartitionDef}; use rand::seq::SliceRandom; use rand::Rng; use snafu::{ensure, ResultExt}; @@ -28,9 +27,11 @@ use crate::context::TableContextRef; use crate::error::{self, Error, Result}; use crate::fake::{random_capitalize_map, MappedGenerator, WordGenerator}; use crate::generator::{ColumnOptionGenerator, ConcreteDataTypeGenerator, Random}; -use crate::ir::create_expr::{ColumnOption, CreateDatabaseExprBuilder, CreateTableExprBuilder}; +use crate::ir::create_expr::{ + ColumnOption, CreateDatabaseExprBuilder, CreateTableExprBuilder, PartitionDef, +}; use crate::ir::{ - column_options_generator, generate_columns, generate_partition_bounds, generate_random_value, + column_options_generator, generate_columns, generate_partition_bounds, partible_column_options_generator, primary_key_options_generator, ts_column_options_generator, Column, ColumnTypeGenerator, CreateDatabaseExpr, CreateTableExpr, Ident, PartibleColumnTypeGenerator, StringColumnTypeGenerator, TsColumnTypeGenerator, @@ -108,25 +109,6 @@ impl Generator for CreateTableExprGenerato self.ts_column_options_generator.as_ref(), ) .remove(0); - - if need_partible_column { - // Generates partition bounds. - let mut partition_bounds = Vec::with_capacity(self.partition); - for _ in 0..self.partition - 1 { - partition_bounds.push(PartitionBound::Value(generate_random_value( - rng, - &column.column_type, - None, - ))); - partition_bounds.sort(); - } - partition_bounds.push(PartitionBound::MaxValue); - builder.partition(PartitionDef::new( - vec![name.value.to_string()], - partition_bounds, - )); - } - columns.push(column); } else { // Generates the partible column. @@ -202,16 +184,16 @@ fn generate_partition_def( column_name: Ident, ) -> PartitionDef { let bounds = generate_partition_bounds(&column_type, partitions - 1); - let mut partition_bounds = Vec::with_capacity(partitions); + let mut partition_exprs = Vec::with_capacity(partitions); let first_bound = bounds[0].clone(); - partition_bounds.push(PartitionBound::Expr(PartitionExpr::new( + partition_exprs.push(PartitionExpr::new( Operand::Column(column_name.to_string()), RestrictedOp::Lt, Operand::Value(first_bound), - ))); + )); for bound_idx in 1..bounds.len() { - partition_bounds.push(PartitionBound::Expr(PartitionExpr::new( + partition_exprs.push(PartitionExpr::new( Operand::Expr(PartitionExpr::new( Operand::Column(column_name.to_string()), RestrictedOp::GtEq, @@ -223,16 +205,19 @@ fn generate_partition_def( RestrictedOp::Lt, Operand::Value(bounds[bound_idx].clone()), )), - ))); + )); } let last_bound = bounds.last().unwrap().clone(); - partition_bounds.push(PartitionBound::Expr(PartitionExpr::new( + partition_exprs.push(PartitionExpr::new( Operand::Column(column_name.to_string()), RestrictedOp::GtEq, Operand::Value(last_bound), - ))); + )); - PartitionDef::new(vec![column_name.to_string()], partition_bounds) + PartitionDef { + columns: vec![column_name.to_string()], + exprs: partition_exprs, + } } /// Generate a physical table with 2 columns: ts of TimestampType::Millisecond as time index and val of Float64Type. @@ -413,7 +398,7 @@ mod tests { assert_eq!(expr.engine, "mito2"); assert!(expr.if_not_exists); assert_eq!(expr.columns.len(), 10); - assert_eq!(expr.partition.unwrap().partition_bounds().len(), 3); + assert_eq!(expr.partition.unwrap().exprs.len(), 3); let expr = CreateTableExprGeneratorBuilder::default() .columns(10) @@ -440,7 +425,7 @@ mod tests { .unwrap(); let serialized = serde_json::to_string(&expr).unwrap(); - let expected = r#"{"table_name":{"value":"quasi","quote_style":null},"columns":[{"name":{"value":"mOLEsTIAs","quote_style":null},"column_type":{"Float64":{}},"options":["PrimaryKey","Null"]},{"name":{"value":"CUMQUe","quote_style":null},"column_type":{"Timestamp":{"Second":null}},"options":["TimeIndex"]},{"name":{"value":"NaTus","quote_style":null},"column_type":{"Int64":{}},"options":[]},{"name":{"value":"EXPeDITA","quote_style":null},"column_type":{"Float64":{}},"options":[]},{"name":{"value":"ImPEDiT","quote_style":null},"column_type":{"Float32":{}},"options":[{"DefaultValue":{"Float32":0.56425774}}]},{"name":{"value":"ADIpisci","quote_style":null},"column_type":{"Float32":{}},"options":["PrimaryKey"]},{"name":{"value":"deBITIs","quote_style":null},"column_type":{"Float32":{}},"options":[{"DefaultValue":{"Float32":0.31315368}}]},{"name":{"value":"toTaM","quote_style":null},"column_type":{"Int32":{}},"options":["NotNull"]},{"name":{"value":"QuI","quote_style":null},"column_type":{"Float32":{}},"options":[{"DefaultValue":{"Float32":0.39941502}}]},{"name":{"value":"INVeNtOre","quote_style":null},"column_type":{"Boolean":null},"options":["PrimaryKey"]}],"if_not_exists":true,"partition":{"partition_columns":["mOLEsTIAs"],"partition_bounds":[{"Expr":{"lhs":{"Column":"mOLEsTIAs"},"op":"Lt","rhs":{"Value":{"Float64":5.992310449541053e307}}}},{"Expr":{"lhs":{"Expr":{"lhs":{"Column":"mOLEsTIAs"},"op":"GtEq","rhs":{"Value":{"Float64":5.992310449541053e307}}}},"op":"And","rhs":{"Expr":{"lhs":{"Column":"mOLEsTIAs"},"op":"Lt","rhs":{"Value":{"Float64":1.1984620899082105e308}}}}}},{"Expr":{"lhs":{"Column":"mOLEsTIAs"},"op":"GtEq","rhs":{"Value":{"Float64":1.1984620899082105e308}}}}]},"engine":"mito2","options":{},"primary_keys":[0,5,9]}"#; + let expected = r#"{"table_name":{"value":"quasi","quote_style":null},"columns":[{"name":{"value":"mOLEsTIAs","quote_style":null},"column_type":{"Float64":{}},"options":["PrimaryKey","Null"]},{"name":{"value":"CUMQUe","quote_style":null},"column_type":{"Timestamp":{"Second":null}},"options":["TimeIndex"]},{"name":{"value":"NaTus","quote_style":null},"column_type":{"Int64":{}},"options":[]},{"name":{"value":"EXPeDITA","quote_style":null},"column_type":{"Float64":{}},"options":[]},{"name":{"value":"ImPEDiT","quote_style":null},"column_type":{"Float32":{}},"options":[{"DefaultValue":{"Float32":0.56425774}}]},{"name":{"value":"ADIpisci","quote_style":null},"column_type":{"Float32":{}},"options":["PrimaryKey"]},{"name":{"value":"deBITIs","quote_style":null},"column_type":{"Float32":{}},"options":[{"DefaultValue":{"Float32":0.31315368}}]},{"name":{"value":"toTaM","quote_style":null},"column_type":{"Int32":{}},"options":["NotNull"]},{"name":{"value":"QuI","quote_style":null},"column_type":{"Float32":{}},"options":[{"DefaultValue":{"Float32":0.39941502}}]},{"name":{"value":"INVeNtOre","quote_style":null},"column_type":{"Boolean":null},"options":["PrimaryKey"]}],"if_not_exists":true,"partition":{"columns":["mOLEsTIAs"],"exprs":[{"lhs":{"Column":"mOLEsTIAs"},"op":"Lt","rhs":{"Value":{"Float64":5.992310449541053e307}}},{"lhs":{"Expr":{"lhs":{"Column":"mOLEsTIAs"},"op":"GtEq","rhs":{"Value":{"Float64":5.992310449541053e307}}}},"op":"And","rhs":{"Expr":{"lhs":{"Column":"mOLEsTIAs"},"op":"Lt","rhs":{"Value":{"Float64":1.1984620899082105e308}}}}},{"lhs":{"Column":"mOLEsTIAs"},"op":"GtEq","rhs":{"Value":{"Float64":1.1984620899082105e308}}}]},"engine":"mito2","options":{},"primary_keys":[0,5,9]}"#; assert_eq!(expected, serialized); } diff --git a/tests-fuzz/src/ir/create_expr.rs b/tests-fuzz/src/ir/create_expr.rs index 1153886d96..d938604f21 100644 --- a/tests-fuzz/src/ir/create_expr.rs +++ b/tests-fuzz/src/ir/create_expr.rs @@ -17,7 +17,7 @@ use std::fmt::Display; use datatypes::value::Value; use derive_builder::Builder; -use partition::partition::PartitionDef; +use partition::expr::PartitionExpr; use serde::{Deserialize, Serialize}; use crate::ir::{Column, Ident}; @@ -70,6 +70,12 @@ pub struct CreateTableExpr { pub primary_keys: Vec, } +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct PartitionDef { + pub columns: Vec, + pub exprs: Vec, +} + #[derive(Debug, Builder, Clone, Serialize, Deserialize)] pub struct CreateDatabaseExpr { #[builder(setter(into))] diff --git a/tests-fuzz/src/translator/mysql/create_expr.rs b/tests-fuzz/src/translator/mysql/create_expr.rs index 073643b059..0b6f9fa85d 100644 --- a/tests-fuzz/src/translator/mysql/create_expr.rs +++ b/tests-fuzz/src/translator/mysql/create_expr.rs @@ -13,8 +13,6 @@ // limitations under the License. use datatypes::data_type::ConcreteDataType; -use datatypes::value::Value; -use partition::partition::PartitionBound; use sql::statements::concrete_data_type_to_sql_data_type; use crate::error::{Error, Result}; @@ -76,28 +74,17 @@ impl CreateTableExprTranslator { input.partition.as_ref().map(|partition| { format!( "PARTITION ON COLUMNS({}) (\n{}\n)", - partition.partition_columns().join(", "), + partition.columns.join(", "), partition - .partition_bounds() + .exprs .iter() - .map(Self::format_partition_bound) + .map(|expr| expr.to_parser_expr().to_string()) .collect::>() .join(",\n") ) }) } - fn format_partition_bound(bound: &PartitionBound) -> String { - match bound { - PartitionBound::Value(v) => match v { - Value::String(v) => format!("'{}'", v.as_utf8()), - _ => format!("{v}"), - }, - PartitionBound::MaxValue => "MAXVALUE".to_string(), - PartitionBound::Expr(expr) => expr.to_parser_expr().to_string(), - } - } - fn format_column_type(column_type: &ConcreteDataType) -> String { // Safety: We don't use the `Dictionary` type concrete_data_type_to_sql_data_type(column_type) @@ -183,10 +170,9 @@ impl CreateDatabaseExprTranslator { #[cfg(test)] mod tests { use partition::expr::{Operand, PartitionExpr, RestrictedOp}; - use partition::partition::{PartitionBound, PartitionDef}; use super::CreateTableExprTranslator; - use crate::ir::create_expr::{CreateDatabaseExprBuilder, CreateTableExprBuilder}; + use crate::ir::create_expr::{CreateDatabaseExprBuilder, CreateTableExprBuilder, PartitionDef}; use crate::test_utils; use crate::translator::DslTranslator; @@ -198,15 +184,15 @@ mod tests { .table_name("system_metrics") .engine("mito") .primary_keys(vec![0, 1]) - .partition(PartitionDef::new( - vec!["idc".to_string()], - vec![ - PartitionBound::Expr(PartitionExpr::new( + .partition(PartitionDef { + columns: vec!["idc".to_string()], + exprs: vec![ + PartitionExpr::new( Operand::Column("idc".to_string()), RestrictedOp::Lt, Operand::Value(datatypes::value::Value::Int32(10)), - )), - PartitionBound::Expr(PartitionExpr::new( + ), + PartitionExpr::new( Operand::Expr(PartitionExpr::new( Operand::Column("idc".to_string()), RestrictedOp::GtEq, @@ -218,14 +204,14 @@ mod tests { RestrictedOp::Lt, Operand::Value(datatypes::value::Value::Int32(50)), )), - )), - PartitionBound::Expr(PartitionExpr::new( + ), + PartitionExpr::new( Operand::Column("idc".to_string()), RestrictedOp::GtEq, Operand::Value(datatypes::value::Value::Int32(50)), - )), + ), ], - )) + }) .build() .unwrap();