fix: partition column with mixed quoted and unquoted idents (#4491)

* fix: partition column with mixed quoted and unquoted idents

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

* update error message

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

---------

Signed-off-by: Ruihang Xia <waynestxia@gmail.com>

Cargo.lock

@@ -214,7 +214,7 @@ checksum = "d301b3b94cb4b2f23d7917810addbbaff90738e0ca2be692bd027e70d7e0330c"
 
 [[package]]
 name = "api"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "common-base",
  "common-decimal",
@@ -762,7 +762,7 @@ dependencies = [
 
 [[package]]
 name = "auth"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "async-trait",
@@ -1277,7 +1277,7 @@ dependencies = [
 
 [[package]]
 name = "cache"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "catalog",
  "common-error",
@@ -1285,7 +1285,7 @@ dependencies = [
  "common-meta",
  "moka",
  "snafu 0.8.3",
- "substrait 0.9.0",
+ "substrait 0.9.1",
 ]
 
 [[package]]
@@ -1312,7 +1312,7 @@ checksum = "37b2a672a2cb129a2e41c10b1224bb368f9f37a2b16b612598138befd7b37eb5"
 
 [[package]]
 name = "catalog"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "arrow",
@@ -1637,7 +1637,7 @@ checksum = "4b82cf0babdbd58558212896d1a4272303a57bdb245c2bf1147185fb45640e70"
 
 [[package]]
 name = "client"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "arc-swap",
@@ -1667,7 +1667,7 @@ dependencies = [
  "serde_json",
  "snafu 0.8.3",
  "substrait 0.37.3",
- "substrait 0.9.0",
+ "substrait 0.9.1",
  "tokio",
  "tokio-stream",
  "tonic 0.11.0",
@@ -1697,7 +1697,7 @@ dependencies = [
 
 [[package]]
 name = "cmd"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "async-trait",
  "auth",
@@ -1753,7 +1753,7 @@ dependencies = [
  "session",
  "snafu 0.8.3",
  "store-api",
- "substrait 0.9.0",
+ "substrait 0.9.1",
  "table",
  "temp-env",
  "tempfile",
@@ -1799,7 +1799,7 @@ checksum = "55b672471b4e9f9e95499ea597ff64941a309b2cdbffcc46f2cc5e2d971fd335"
 
 [[package]]
 name = "common-base"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "anymap",
  "bitvec",
@@ -1815,7 +1815,7 @@ dependencies = [
 
 [[package]]
 name = "common-catalog"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "chrono",
  "common-error",
@@ -1826,7 +1826,7 @@ dependencies = [
 
 [[package]]
 name = "common-config"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "common-base",
  "common-error",
@@ -1849,7 +1849,7 @@ dependencies = [
 
 [[package]]
 name = "common-datasource"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "arrow",
  "arrow-schema",
@@ -1886,7 +1886,7 @@ dependencies = [
 
 [[package]]
 name = "common-decimal"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "bigdecimal",
  "common-error",
@@ -1899,7 +1899,7 @@ dependencies = [
 
 [[package]]
 name = "common-error"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "snafu 0.8.3",
  "strum 0.25.0",
@@ -1908,7 +1908,7 @@ dependencies = [
 
 [[package]]
 name = "common-frontend"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "async-trait",
@@ -1923,7 +1923,7 @@ dependencies = [
 
 [[package]]
 name = "common-function"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "arc-swap",
@@ -1957,7 +1957,7 @@ dependencies = [
 
 [[package]]
 name = "common-greptimedb-telemetry"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "async-trait",
  "common-runtime",
@@ -1974,7 +1974,7 @@ dependencies = [
 
 [[package]]
 name = "common-grpc"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "arrow-flight",
@@ -2000,7 +2000,7 @@ dependencies = [
 
 [[package]]
 name = "common-grpc-expr"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "common-base",
@@ -2018,7 +2018,7 @@ dependencies = [
 
 [[package]]
 name = "common-macro"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "arc-swap",
  "common-query",
@@ -2032,7 +2032,7 @@ dependencies = [
 
 [[package]]
 name = "common-mem-prof"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "common-error",
  "common-macro",
@@ -2045,7 +2045,7 @@ dependencies = [
 
 [[package]]
 name = "common-meta"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "anymap2",
  "api",
@@ -2100,11 +2100,11 @@ dependencies = [
 
 [[package]]
 name = "common-plugins"
-version = "0.9.0"
+version = "0.9.1"
 
 [[package]]
 name = "common-procedure"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "async-stream",
  "async-trait",
@@ -2130,7 +2130,7 @@ dependencies = [
 
 [[package]]
 name = "common-procedure-test"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "async-trait",
  "common-procedure",
@@ -2138,7 +2138,7 @@ dependencies = [
 
 [[package]]
 name = "common-query"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "async-trait",
@@ -2163,7 +2163,7 @@ dependencies = [
 
 [[package]]
 name = "common-recordbatch"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "arc-swap",
  "common-error",
@@ -2182,7 +2182,7 @@ dependencies = [
 
 [[package]]
 name = "common-runtime"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "async-trait",
  "common-error",
@@ -2204,7 +2204,7 @@ dependencies = [
 
 [[package]]
 name = "common-telemetry"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "atty",
  "backtrace",
@@ -2231,7 +2231,7 @@ dependencies = [
 
 [[package]]
 name = "common-test-util"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "client",
  "common-query",
@@ -2243,7 +2243,7 @@ dependencies = [
 
 [[package]]
 name = "common-time"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "arrow",
  "chrono",
@@ -2259,7 +2259,7 @@ dependencies = [
 
 [[package]]
 name = "common-version"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "build-data",
  "const_format",
@@ -2270,7 +2270,7 @@ dependencies = [
 
 [[package]]
 name = "common-wal"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "common-base",
  "common-error",
@@ -3066,7 +3066,7 @@ dependencies = [
 
 [[package]]
 name = "datanode"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "arrow-flight",
@@ -3115,7 +3115,7 @@ dependencies = [
  "session",
  "snafu 0.8.3",
  "store-api",
- "substrait 0.9.0",
+ "substrait 0.9.1",
  "table",
  "tokio",
  "toml 0.8.14",
@@ -3124,7 +3124,7 @@ dependencies = [
 
 [[package]]
 name = "datatypes"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "arrow",
  "arrow-array",
@@ -3685,7 +3685,7 @@ dependencies = [
 
 [[package]]
 name = "file-engine"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "async-trait",
@@ -3781,7 +3781,7 @@ dependencies = [
 
 [[package]]
 name = "flow"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "arrow-schema",
@@ -3835,7 +3835,7 @@ dependencies = [
  "snafu 0.8.3",
  "store-api",
  "strum 0.25.0",
- "substrait 0.9.0",
+ "substrait 0.9.1",
  "table",
  "tokio",
  "tonic 0.11.0",
@@ -3882,7 +3882,7 @@ checksum = "6c2141d6d6c8512188a7891b4b01590a45f6dac67afb4f255c4124dbb86d4eaa"
 
 [[package]]
 name = "frontend"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "arc-swap",
@@ -4993,7 +4993,7 @@ dependencies = [
 
 [[package]]
 name = "index"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "async-trait",
  "asynchronous-codec",
@@ -5763,7 +5763,7 @@ checksum = "90ed8c1e510134f979dbc4f070f87d4313098b704861a105fe34231c70a3901c"
 
 [[package]]
 name = "log-store"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "async-stream",
  "async-trait",
@@ -6070,7 +6070,7 @@ dependencies = [
 
 [[package]]
 name = "meta-client"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "async-trait",
@@ -6096,7 +6096,7 @@ dependencies = [
 
 [[package]]
 name = "meta-srv"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "async-trait",
@@ -6172,7 +6172,7 @@ dependencies = [
 
 [[package]]
 name = "metric-engine"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "aquamarine",
@@ -6263,7 +6263,7 @@ dependencies = [
 
 [[package]]
 name = "mito2"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "aquamarine",
@@ -6910,7 +6910,7 @@ dependencies = [
 
 [[package]]
 name = "object-store"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "anyhow",
  "bytes",
@@ -7157,7 +7157,7 @@ dependencies = [
 
 [[package]]
 name = "operator"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "async-trait",
@@ -7202,7 +7202,7 @@ dependencies = [
  "sql",
  "sqlparser 0.45.0 (git+https://github.com/GreptimeTeam/sqlparser-rs.git?rev=54a267ac89c09b11c0c88934690530807185d3e7)",
  "store-api",
- "substrait 0.9.0",
+ "substrait 0.9.1",
  "table",
  "tokio",
  "tokio-util",
@@ -7452,7 +7452,7 @@ dependencies = [
 
 [[package]]
 name = "partition"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "async-trait",
@@ -7741,7 +7741,7 @@ checksum = "8b870d8c151b6f2fb93e84a13146138f05d02ed11c7e7c54f8826aaaf7c9f184"
 
 [[package]]
 name = "pipeline"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "arrow",
@@ -7899,7 +7899,7 @@ dependencies = [
 
 [[package]]
 name = "plugins"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "auth",
  "common-base",
@@ -8168,7 +8168,7 @@ dependencies = [
 
 [[package]]
 name = "promql"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "ahash 0.8.11",
  "async-trait",
@@ -8403,7 +8403,7 @@ dependencies = [
 
 [[package]]
 name = "puffin"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "async-compression 0.4.11",
  "async-trait",
@@ -8525,7 +8525,7 @@ dependencies = [
 
 [[package]]
 name = "query"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "ahash 0.8.11",
  "api",
@@ -8588,7 +8588,7 @@ dependencies = [
  "stats-cli",
  "store-api",
  "streaming-stats",
- "substrait 0.9.0",
+ "substrait 0.9.1",
  "table",
  "tokio",
  "tokio-stream",
@@ -9916,7 +9916,7 @@ checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49"
 
 [[package]]
 name = "script"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "arc-swap",
@@ -10209,7 +10209,7 @@ dependencies = [
 
 [[package]]
 name = "servers"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "aide",
  "api",
@@ -10315,7 +10315,7 @@ dependencies = [
 
 [[package]]
 name = "session"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "arc-swap",
@@ -10616,7 +10616,7 @@ dependencies = [
 
 [[package]]
 name = "sql"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "chrono",
@@ -10676,7 +10676,7 @@ dependencies = [
 
 [[package]]
 name = "sqlness-runner"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "async-trait",
  "clap 4.5.7",
@@ -10893,7 +10893,7 @@ dependencies = [
 
 [[package]]
 name = "store-api"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "aquamarine",
@@ -11062,7 +11062,7 @@ dependencies = [
 
 [[package]]
 name = "substrait"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "async-trait",
  "bytes",
@@ -11263,7 +11263,7 @@ dependencies = [
 
 [[package]]
 name = "table"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "async-trait",
@@ -11528,7 +11528,7 @@ checksum = "3369f5ac52d5eb6ab48c6b4ffdc8efbcad6b89c765749064ba298f2c68a16a76"
 
 [[package]]
 name = "tests-fuzz"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "arbitrary",
  "async-trait",
@@ -11570,7 +11570,7 @@ dependencies = [
 
 [[package]]
 name = "tests-integration"
-version = "0.9.0"
+version = "0.9.1"
 dependencies = [
  "api",
  "arrow-flight",
@@ -11630,7 +11630,7 @@ dependencies = [
  "sql",
  "sqlx",
  "store-api",
- "substrait 0.9.0",
+ "substrait 0.9.1",
  "table",
  "tempfile",
  "time",

Cargo.toml

@@ -64,7 +64,7 @@ members = [
 resolver = "2"
 
 [workspace.package]
-version = "0.9.0"
+version = "0.9.1"
 edition = "2021"
 license = "Apache-2.0"
 

docker/docker-compose/cluster-with-etcd.yaml

@@ -1,9 +1,9 @@
 x-custom:
   etcd_initial_cluster_token: &etcd_initial_cluster_token "--initial-cluster-token=etcd-cluster"
   etcd_common_settings: &etcd_common_settings
-    image: quay.io/coreos/etcd:v3.5.10
+    image: "${ETCD_REGISTRY:-quay.io}/${ETCD_NAMESPACE:-coreos}/etcd:${ETCD_VERSION:-v3.5.10}"
     entrypoint: /usr/local/bin/etcd
-  greptimedb_image: &greptimedb_image docker.io/greptimedb/greptimedb:latest
+  greptimedb_image: &greptimedb_image "${GREPTIMEDB_REGISTRY:-docker.io}/${GREPTIMEDB_NAMESPACE:-greptime}/greptimedb:${GREPTIMEDB_VERSION:-latest}"
 
 services:
   etcd0:

src/mito2/src/cache/file_cache.rs

@@ -199,7 +199,9 @@ impl FileCache {
             .metakey(Metakey::ContentLength)
             .await
             .context(OpenDalSnafu)?;
-        let (mut total_size, mut total_keys) = (0, 0);
+        // Use i64 for total_size to reduce the risk of overflow.
+        // It is possible that the total size of the cache is larger than i32::MAX.
+        let (mut total_size, mut total_keys) = (0i64, 0);
         while let Some(entry) = lister.try_next().await.context(OpenDalSnafu)? {
             let meta = entry.metadata();
             if !meta.is_file() {
@@ -212,13 +214,11 @@ impl FileCache {
             self.memory_index
                 .insert(key, IndexValue { file_size })
                 .await;
-            total_size += file_size;
+            total_size += i64::from(file_size);
             total_keys += 1;
         }
         // The metrics is a signed int gauge so we can updates it finally.
-        CACHE_BYTES
-            .with_label_values(&[FILE_TYPE])
-            .add(total_size.into());
+        CACHE_BYTES.with_label_values(&[FILE_TYPE]).add(total_size);
 
         info!(
             "Recovered file cache, num_keys: {}, num_bytes: {}, cost: {:?}",

src/mito2/src/config.rs

@@ -43,6 +43,8 @@ const SST_META_CACHE_SIZE_FACTOR: u64 = 32;
 const INDEX_CONTENT_CACHE_SIZE_FACTOR: u64 = 32;
 /// Use `1/MEM_CACHE_SIZE_FACTOR` of OS memory size as mem cache size in default mode
 const MEM_CACHE_SIZE_FACTOR: u64 = 16;
+/// Use `1/PAGE_CACHE_SIZE_FACTOR` of OS memory size as page cache size in default mode
+const PAGE_CACHE_SIZE_FACTOR: u64 = 8;
 /// Use `1/INDEX_CREATE_MEM_THRESHOLD_FACTOR` of OS memory size as mem threshold for creating index
 const INDEX_CREATE_MEM_THRESHOLD_FACTOR: u64 = 16;
 
@@ -236,12 +238,13 @@ impl MitoConfig {
         );
         // shouldn't be greater than 512MB in default mode.
         let mem_cache_size = cmp::min(sys_memory / MEM_CACHE_SIZE_FACTOR, ReadableSize::mb(512));
+        let page_cache_size = sys_memory / PAGE_CACHE_SIZE_FACTOR;
 
         self.global_write_buffer_size = global_write_buffer_size;
         self.global_write_buffer_reject_size = global_write_buffer_reject_size;
         self.sst_meta_cache_size = sst_meta_cache_size;
         self.vector_cache_size = mem_cache_size;
-        self.page_cache_size = mem_cache_size;
+        self.page_cache_size = page_cache_size;
         self.selector_result_cache_size = mem_cache_size;
     }
 

src/sql/src/parsers/create_parser.rs

@@ -950,7 +950,7 @@ fn ensure_exprs_are_binary(exprs: &[Expr], columns: &[&Column]) -> Result<()> {
             ensure_one_expr(right, columns)?;
         } else {
             return error::InvalidSqlSnafu {
-                msg: format!("Partition rule expr {:?} is not a binary expr!", expr),
+                msg: format!("Partition rule expr {:?} is not a binary expr", expr),
             }
             .fail();
         }
@@ -974,7 +974,7 @@ fn ensure_one_expr(expr: &Expr, columns: &[&Column]) -> Result<()> {
                 columns.iter().any(|c| &c.name().value == column_name),
                 error::InvalidSqlSnafu {
                     msg: format!(
-                        "Column {:?} in rule expr is not referenced in PARTITION ON!",
+                        "Column {:?} in rule expr is not referenced in PARTITION ON",
                         column_name
                     ),
                 }
@@ -987,7 +987,7 @@ fn ensure_one_expr(expr: &Expr, columns: &[&Column]) -> Result<()> {
             Ok(())
         }
         _ => error::InvalidSqlSnafu {
-            msg: format!("Partition rule expr {:?} is not a binary expr!", expr),
+            msg: format!("Partition rule expr {:?} is not a binary expr", expr),
         }
         .fail(),
     }
@@ -1002,13 +1002,14 @@ fn ensure_partition_columns_defined<'a>(
         .column_list
         .iter()
         .map(|x| {
+            let x = ParserContext::canonicalize_identifier(x.clone());
             // Normally the columns in "create table" won't be too many,
             // a linear search to find the target every time is fine.
             columns
                 .iter()
-                .find(|c| c.name() == x)
+                .find(|c| *c.name().value == x.value)
                 .context(error::InvalidSqlSnafu {
-                    msg: format!("Partition column {:?} not defined!", x.value),
+                    msg: format!("Partition column {:?} not defined", x.value),
                 })
         })
         .collect::<Result<Vec<&Column>>>()
@@ -1320,7 +1321,7 @@ ENGINE=mito";
         assert!(result
             .unwrap_err()
             .to_string()
-            .contains("Partition column \"x\" not defined!"));
+            .contains("Partition column \"x\" not defined"));
     }
 
     #[test]
@@ -1447,6 +1448,30 @@ ENGINE=mito";
         }
     }
 
+    #[test]
+    fn test_parse_create_table_with_quoted_partitions() {
+        let sql = r"
+CREATE TABLE monitor (
+  `host_id`    INT,
+  idc        STRING,
+  ts         TIMESTAMP,
+  cpu        DOUBLE DEFAULT 0,
+  memory     DOUBLE,
+  TIME INDEX (ts),
+  PRIMARY KEY (host),
+)
+PARTITION ON COLUMNS(IdC, host_id) (
+  idc <= 'hz' AND host_id < 1000,
+  idc > 'hz' AND idc <= 'sh' AND host_id < 2000,
+  idc > 'sh' AND host_id < 3000,
+  idc > 'sh' AND host_id >= 3000,
+)";
+        let result =
+            ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default())
+                .unwrap();
+        assert_eq!(result.len(), 1);
+    }
+
     #[test]
     fn test_parse_create_table_with_timestamp_index() {
         let sql1 = r"
@@ -1728,7 +1753,7 @@ ENGINE=mito";
             ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default());
         assert_eq!(
             result.unwrap_err().output_msg(),
-            "Invalid SQL, error: Column \"b\" in rule expr is not referenced in PARTITION ON!"
+            "Invalid SQL, error: Column \"b\" in rule expr is not referenced in PARTITION ON"
         );
     }
 
@@ -1744,7 +1769,7 @@ ENGINE=mito";
             ParserContext::create_with_dialect(sql, &GreptimeDbDialect {}, ParseOptions::default());
         assert_eq!(
             result.unwrap_err().output_msg(),
-            "Invalid SQL, error: Partition rule expr Identifier(Ident { value: \"b\", quote_style: None }) is not a binary expr!"
+            "Invalid SQL, error: Partition rule expr Identifier(Ident { value: \"b\", quote_style: None }) is not a binary expr"
         );
     }