diff --git a/Cargo.lock b/Cargo.lock index 5fb6c83fa1..2bfb7741eb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8189,6 +8189,7 @@ dependencies = [ "catalog", "common-base", "common-catalog", + "common-datasource", "common-error", "common-time", "datafusion-sql", diff --git a/src/common/datasource/src/file_format.rs b/src/common/datasource/src/file_format.rs index e5bff711eb..59870a67ed 100644 --- a/src/common/datasource/src/file_format.rs +++ b/src/common/datasource/src/file_format.rs @@ -45,11 +45,12 @@ use crate::compression::CompressionType; use crate::error::{self, Result}; use crate::share_buffer::SharedBuffer; -pub const FORMAT_COMPRESSION_TYPE: &str = "COMPRESSION_TYPE"; -pub const FORMAT_DELIMTERL: &str = "DELIMTERL"; -pub const FORMAT_SCHEMA_INFER_MAX_RECORD: &str = "SCHEMA_INFER_MAX_RECORD"; -pub const FORMAT_HAS_HEADER: &str = "FORMAT_HAS_HEADER"; -pub const FORMAT_TYPE: &str = "FORMAT"; +pub const FORMAT_COMPRESSION_TYPE: &str = "compression_type"; +pub const FORMAT_DELIMITER: &str = "delimiter"; +pub const FORMAT_SCHEMA_INFER_MAX_RECORD: &str = "schema_infer_max_record"; +pub const FORMAT_HAS_HEADER: &str = "has_header"; +pub const FORMAT_TYPE: &str = "format"; +pub const FILE_PATTERN: &str = "pattern"; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Format { diff --git a/src/common/datasource/src/file_format/csv.rs b/src/common/datasource/src/file_format/csv.rs index d6b388fc22..70f5262046 100644 --- a/src/common/datasource/src/file_format/csv.rs +++ b/src/common/datasource/src/file_format/csv.rs @@ -50,11 +50,11 @@ impl TryFrom<&HashMap> for CsvFormat { fn try_from(value: &HashMap) -> Result { let mut format = CsvFormat::default(); - if let Some(delimiter) = value.get(file_format::FORMAT_DELIMTERL) { + if let Some(delimiter) = value.get(file_format::FORMAT_DELIMITER) { // TODO(weny): considers to support parse like "\t" (not only b'\t') format.delimiter = u8::from_str(delimiter).map_err(|_| { error::ParseFormatSnafu { - key: file_format::FORMAT_DELIMTERL, + key: file_format::FORMAT_DELIMITER, value: delimiter, } .build() @@ -210,7 +210,7 @@ mod tests { use super::*; use crate::file_format::{ - FileFormat, FORMAT_COMPRESSION_TYPE, FORMAT_DELIMTERL, FORMAT_HAS_HEADER, + FileFormat, FORMAT_COMPRESSION_TYPE, FORMAT_DELIMITER, FORMAT_HAS_HEADER, FORMAT_SCHEMA_INFER_MAX_RECORD, }; use crate::test_util::{self, format_schema, test_store}; @@ -301,7 +301,7 @@ mod tests { ); map.insert(FORMAT_COMPRESSION_TYPE.to_string(), "zstd".to_string()); - map.insert(FORMAT_DELIMTERL.to_string(), b'\t'.to_string()); + map.insert(FORMAT_DELIMITER.to_string(), b'\t'.to_string()); map.insert(FORMAT_HAS_HEADER.to_string(), "false".to_string()); let format = CsvFormat::try_from(&map).unwrap(); diff --git a/src/common/datasource/src/object_store/s3.rs b/src/common/datasource/src/object_store/s3.rs index 7688211021..0ebd80411b 100644 --- a/src/common/datasource/src/object_store/s3.rs +++ b/src/common/datasource/src/object_store/s3.rs @@ -20,12 +20,12 @@ use snafu::ResultExt; use crate::error::{self, Result}; -const ENDPOINT_URL: &str = "ENDPOINT_URL"; -const ACCESS_KEY_ID: &str = "ACCESS_KEY_ID"; -const SECRET_ACCESS_KEY: &str = "SECRET_ACCESS_KEY"; -const SESSION_TOKEN: &str = "SESSION_TOKEN"; -const REGION: &str = "REGION"; -const ENABLE_VIRTUAL_HOST_STYLE: &str = "ENABLE_VIRTUAL_HOST_STYLE"; +const ENDPOINT_URL: &str = "endpoint_url"; +const ACCESS_KEY_ID: &str = "access_key_id"; +const SECRET_ACCESS_KEY: &str = "secret_access_key"; +const SESSION_TOKEN: &str = "session_token"; +const REGION: &str = "region"; +const ENABLE_VIRTUAL_HOST_STYLE: &str = "enable_virtual_host_style"; pub fn build_s3_backend( host: &str, diff --git a/src/frontend/src/statement.rs b/src/frontend/src/statement.rs index 4e1fbeebe4..f0612756ab 100644 --- a/src/frontend/src/statement.rs +++ b/src/frontend/src/statement.rs @@ -178,7 +178,9 @@ fn to_copy_table_request(stmt: CopyTable, query_ctx: QueryContextRef) -> Result< .map_err(BoxedError::new) .context(ExternalSnafu)?; - let pattern = with.get("PATTERN").cloned(); + let pattern = with + .get(common_datasource::file_format::FILE_PATTERN) + .cloned(); Ok(CopyTableRequest { catalog_name, diff --git a/src/query/src/sql/show.rs b/src/query/src/sql/show.rs index 051874a48b..54e8c20cd4 100644 --- a/src/query/src/sql/show.rs +++ b/src/query/src/sql/show.rs @@ -328,8 +328,8 @@ CREATE EXTERNAL TABLE IF NOT EXISTS system_metrics ( ) ENGINE=file WITH( - FORMAT = 'csv', - LOCATION = 'foo.csv' + format = 'csv', + location = 'foo.csv' )"#, sql ); diff --git a/src/sql/Cargo.toml b/src/sql/Cargo.toml index f77dc21fff..9ccadcbc37 100644 --- a/src/sql/Cargo.toml +++ b/src/sql/Cargo.toml @@ -9,6 +9,7 @@ api = { path = "../api" } catalog = { path = "../catalog" } common-base = { path = "../common/base" } common-catalog = { path = "../common/catalog" } +common-datasource = { path = "../common/datasource" } common-error = { path = "../common/error" } common-time = { path = "../common/time" } datafusion-sql.workspace = true diff --git a/src/sql/src/parsers/copy_parser.rs b/src/sql/src/parsers/copy_parser.rs index 0405ddac03..2b43df6eb0 100644 --- a/src/sql/src/parsers/copy_parser.rs +++ b/src/sql/src/parsers/copy_parser.rs @@ -68,7 +68,7 @@ impl<'a> ParserContext<'a> { let with = options .into_iter() .filter_map(|option| { - parse_option_string(option.value).map(|v| (option.name.value.to_uppercase(), v)) + parse_option_string(option.value).map(|v| (option.name.value.to_lowercase(), v)) }) .collect(); @@ -80,7 +80,7 @@ impl<'a> ParserContext<'a> { let connection = connection_options .into_iter() .filter_map(|option| { - parse_option_string(option.value).map(|v| (option.name.value.to_uppercase(), v)) + parse_option_string(option.value).map(|v| (option.name.value.to_lowercase(), v)) }) .collect(); Ok(CopyTableArgument { @@ -109,7 +109,7 @@ impl<'a> ParserContext<'a> { let with = options .into_iter() .filter_map(|option| { - parse_option_string(option.value).map(|v| (option.name.value.to_uppercase(), v)) + parse_option_string(option.value).map(|v| (option.name.value.to_lowercase(), v)) }) .collect(); @@ -121,7 +121,7 @@ impl<'a> ParserContext<'a> { let connection = connection_options .into_iter() .filter_map(|option| { - parse_option_string(option.value).map(|v| (option.name.value.to_uppercase(), v)) + parse_option_string(option.value).map(|v| (option.name.value.to_lowercase(), v)) }) .collect(); @@ -243,7 +243,7 @@ mod tests { Test { sql: "COPY catalog0.schema0.tbl FROM 'tbl_file.parquet' WITH (PATTERN = 'demo.*') CONNECTION (FOO='Bar', ONE='two')", expected_pattern: Some("demo.*".into()), - expected_connection: [("FOO","Bar"),("ONE","two")].into_iter().map(|(k,v)|{(k.to_string(),v.to_string())}).collect() + expected_connection: [("foo","Bar"),("one","two")].into_iter().map(|(k,v)|{(k.to_string(),v.to_string())}).collect() }, ]; @@ -280,11 +280,11 @@ mod tests { }, Test { sql: "COPY catalog0.schema0.tbl TO 'tbl_file.parquet' CONNECTION (FOO='Bar', ONE='two')", - expected_connection: [("FOO","Bar"),("ONE","two")].into_iter().map(|(k,v)|{(k.to_string(),v.to_string())}).collect() + expected_connection: [("foo","Bar"),("one","two")].into_iter().map(|(k,v)|{(k.to_string(),v.to_string())}).collect() }, Test { sql:"COPY catalog0.schema0.tbl TO 'tbl_file.parquet' WITH (FORMAT = 'parquet') CONNECTION (FOO='Bar', ONE='two')", - expected_connection: [("FOO","Bar"),("ONE","two")].into_iter().map(|(k,v)|{(k.to_string(),v.to_string())}).collect() + expected_connection: [("foo","Bar"),("one","two")].into_iter().map(|(k,v)|{(k.to_string(),v.to_string())}).collect() }, ]; diff --git a/src/sql/src/parsers/create_parser.rs b/src/sql/src/parsers/create_parser.rs index 7b761e09a6..9c4fc8a906 100644 --- a/src/sql/src/parsers/create_parser.rs +++ b/src/sql/src/parsers/create_parser.rs @@ -85,7 +85,7 @@ impl<'a> ParserContext<'a> { .into_iter() .filter_map(|option| { if let Some(v) = parse_option_string(option.value) { - Some((option.name.value.to_uppercase(), v)) + Some((option.name.value.to_lowercase(), v)) } else { None } @@ -803,8 +803,8 @@ mod tests { sql: "CREATE EXTERNAL TABLE city with(location='/var/data/city.csv',format='csv');", expected_table_name: "city", expected_options: HashMap::from([ - ("LOCATION".to_string(), "/var/data/city.csv".to_string()), - ("FORMAT".to_string(), "csv".to_string()), + ("location".to_string(), "/var/data/city.csv".to_string()), + ("format".to_string(), "csv".to_string()), ]), expected_engine: IMMUTABLE_FILE_ENGINE, expected_if_not_exist: false, @@ -813,8 +813,8 @@ mod tests { sql: "CREATE EXTERNAL TABLE IF NOT EXISTS city ENGINE=foo with(location='/var/data/city.csv',format='csv');", expected_table_name: "city", expected_options: HashMap::from([ - ("LOCATION".to_string(), "/var/data/city.csv".to_string()), - ("FORMAT".to_string(), "csv".to_string()), + ("location".to_string(), "/var/data/city.csv".to_string()), + ("format".to_string(), "csv".to_string()), ]), expected_engine: "foo", expected_if_not_exist: true, @@ -848,8 +848,8 @@ mod tests { ) with(location='/var/data/city.csv',format='csv');"; let options = HashMap::from([ - ("LOCATION".to_string(), "/var/data/city.csv".to_string()), - ("FORMAT".to_string(), "csv".to_string()), + ("location".to_string(), "/var/data/city.csv".to_string()), + ("format".to_string(), "csv".to_string()), ]); let stmts = ParserContext::create_with_dialect(sql, &GenericDialect {}).unwrap(); diff --git a/src/sql/src/statements/copy.rs b/src/sql/src/statements/copy.rs index db7e69d5b9..35dfe1bfe1 100644 --- a/src/sql/src/statements/copy.rs +++ b/src/sql/src/statements/copy.rs @@ -15,7 +15,6 @@ use std::collections::HashMap; use sqlparser::ast::ObjectName; - #[derive(Debug, Clone, PartialEq, Eq)] pub enum CopyTable { To(CopyTableArgument), @@ -33,16 +32,16 @@ pub struct CopyTableArgument { #[cfg(test)] impl CopyTableArgument { - const FORMAT: &str = "FORMAT"; - pub fn format(&self) -> Option { self.with - .get(Self::FORMAT) + .get(common_datasource::file_format::FORMAT_TYPE) .cloned() .or_else(|| Some("PARQUET".to_string())) } pub fn pattern(&self) -> Option { - self.with.get("PATTERN").cloned() + self.with + .get(common_datasource::file_format::FILE_PATTERN) + .cloned() } } diff --git a/src/sql/src/statements/create.rs b/src/sql/src/statements/create.rs index 431742db6c..6e4c5b0254 100644 --- a/src/sql/src/statements/create.rs +++ b/src/sql/src/statements/create.rs @@ -198,8 +198,7 @@ pub struct CreateExternalTable { pub columns: Vec, pub constraints: Vec, /// Table options in `WITH`. - /// All keys are uppercase. - /// TODO(weny): unify the key's case styling. + /// All keys are lowercase. pub options: HashMap, pub if_not_exists: bool, pub engine: String, diff --git a/src/table/src/requests.rs b/src/table/src/requests.rs index 56ff0a513c..8845284bd6 100644 --- a/src/table/src/requests.rs +++ b/src/table/src/requests.rs @@ -29,10 +29,10 @@ use crate::error; use crate::error::ParseTableOptionSnafu; use crate::metadata::TableId; -pub const IMMUTABLE_TABLE_META_KEY: &str = "IMMUTABLE_TABLE_META"; -pub const IMMUTABLE_TABLE_LOCATION_KEY: &str = "LOCATION"; -pub const IMMUTABLE_TABLE_PATTERN_KEY: &str = "PATTERN"; -pub const IMMUTABLE_TABLE_FORMAT_KEY: &str = "FORMAT"; +pub const IMMUTABLE_TABLE_META_KEY: &str = "__private.immutable_table_meta"; +pub const IMMUTABLE_TABLE_LOCATION_KEY: &str = "location"; +pub const IMMUTABLE_TABLE_PATTERN_KEY: &str = "pattern"; +pub const IMMUTABLE_TABLE_FORMAT_KEY: &str = "format"; #[derive(Debug, Clone)] pub struct CreateDatabaseRequest {