feat: validate partition rule on create table (#4213)

Signed-off-by: Ruihang Xia <[email protected]>

src/operator/src/error.rs

@@ -156,6 +156,13 @@ pub enum Error {
         source: common_meta::error::Error,
     },
 
+    #[snafu(display("Invalid partition"))]
+    InvalidPartition {
+        #[snafu(implicit)]
+        location: Location,
+        source: partition::error::Error,
+    },
+
     #[snafu(display("Invalid SQL, error: {}", err_msg))]
     InvalidSql {
         err_msg: String,
@@ -727,7 +734,8 @@ impl ErrorExt for Error {
             | Error::InvalidViewName { .. }
             | Error::InvalidExpr { .. }
             | Error::InvalidViewStmt { .. }
-            | Error::ConvertIdentifier { .. } => StatusCode::InvalidArguments,
+            | Error::ConvertIdentifier { .. }
+            | Error::InvalidPartition { .. } => StatusCode::InvalidArguments,
 
             Error::TableAlreadyExists { .. } | Error::ViewAlreadyExists { .. } => {
                 StatusCode::TableAlreadyExists

src/operator/src/statement/ddl.rs

@@ -40,6 +40,7 @@ use datatypes::schema::RawSchema;
 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;
@@ -69,9 +70,9 @@ use crate::error::{
     self, AlterExprToRequestSnafu, CatalogSnafu, ColumnDataTypeSnafu, ColumnNotFoundSnafu,
     CreateLogicalTablesSnafu, CreateTableInfoSnafu, DeserializePartitionSnafu, EmptyDdlExprSnafu,
     ExtractTableNamesSnafu, FlowNotFoundSnafu, InvalidPartitionColumnsSnafu,
-    InvalidPartitionRuleSnafu, InvalidTableNameSnafu, InvalidViewNameSnafu, InvalidViewStmtSnafu,
-    ParseSqlValueSnafu, Result, SchemaInUseSnafu, SchemaNotFoundSnafu, SubstraitCodecSnafu,
-    TableAlreadyExistsSnafu, TableMetadataManagerSnafu, TableNotFoundSnafu,
+    InvalidPartitionRuleSnafu, InvalidPartitionSnafu, InvalidTableNameSnafu, InvalidViewNameSnafu,
+    InvalidViewStmtSnafu, ParseSqlValueSnafu, Result, SchemaInUseSnafu, SchemaNotFoundSnafu,
+    SubstraitCodecSnafu, TableAlreadyExistsSnafu, TableMetadataManagerSnafu, TableNotFoundSnafu,
     UnrecognizedTableOptionSnafu, ViewAlreadyExistsSnafu,
 };
 use crate::expr_factory;
@@ -230,9 +231,7 @@ impl StatementExecutor {
         );
 
         let (partitions, partition_cols) = parse_partitions(create_table, partitions, &query_ctx)?;
-
         validate_partition_columns(create_table, &partition_cols)?;
-
         let mut table_info = create_table_info(create_table, partition_cols, schema_opts)?;
 
         let resp = self
@@ -1103,6 +1102,18 @@ fn parse_partitions(
     let partition_entries =
         find_partition_entries(create_table, &partitions, &partition_columns, query_ctx)?;
 
+    // Validates partition
+    let mut exprs = vec![];
+    for partition in &partition_entries {
+        for bound in partition {
+            if let PartitionBound::Expr(expr) = bound {
+                exprs.push(expr.clone());
+            }
+        }
+    }
+    MultiDimPartitionRule::try_new(partition_columns.clone(), vec![], exprs)
+        .context(InvalidPartitionSnafu)?;
+
     Ok((
         partition_entries
             .into_iter()

src/partition/src/multi_dim.rs

@@ -66,7 +66,7 @@ impl MultiDimPartitionRule {
         };
 
         let mut checker = RuleChecker::new(&rule);
-        checker.check(&rule)?;
+        checker.check()?;
 
         Ok(rule)
     }
@@ -189,8 +189,8 @@ impl<'a> RuleChecker<'a> {
         }
     }
 
-    pub fn check(&mut self, rule: &MultiDimPartitionRule) -> Result<()> {
-        for expr in &rule.exprs {
+    pub fn check(&mut self) -> Result<()> {
+        for expr in &self.rule.exprs {
             self.walk_expr(expr)?
         }
 

tests/cases/standalone/common/partition.result

@@ -167,3 +167,17 @@ DROP TABLE my_table;
 
 Affected Rows: 0
 
+-- incorrect partition rule
+CREATE TABLE invalid_rule (
+  a INT PRIMARY KEY,
+  b STRING,
+  ts TIMESTAMP TIME INDEX,
+)
+PARTITION ON COLUMNS (a) (
+  a < 10,
+  a > 10 AND a < 20,
+  a >= 20
+);
+
+Error: 1004(InvalidArguments), Unclosed value Int32(10) on column a
+

tests/cases/standalone/common/partition.sql

@@ -69,3 +69,15 @@ INSERT INTO my_table VALUES
 SELECT * FROM my_table;
 
 DROP TABLE my_table;
+
+-- incorrect partition rule
+CREATE TABLE invalid_rule (
+  a INT PRIMARY KEY,
+  b STRING,
+  ts TIMESTAMP TIME INDEX,
+)
+PARTITION ON COLUMNS (a) (
+  a < 10,
+  a > 10 AND a < 20,
+  a >= 20
+);