Skip to content

Commit 87bfbce

Browse files
git-hulkgit-hulk
committed
Fix review comments
1 parent e05eb87 commit 87bfbce

File tree

8 files changed

+108
-121
lines changed

8 files changed

+108
-121
lines changed

‎src/ast/data_type.rs

+10-8
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,19 @@ use serde::{Deserialize, Serialize};
2525
#[cfg(feature = "visitor")]
2626
use sqlparser_derive::{Visit, VisitMut};
2727

28-
use crate::ast::{display_comma_separated, ObjectName, StructField, UnionField, Value};
28+
use crate::ast::{display_comma_separated, Expr, ObjectName, StructField, UnionField};
2929

3030
use super::{value::escape_single_quote_string, ColumnDef};
3131

3232
#[derive(Debug, Clone, PartialEq, PartialOrd, Eq, Ord, Hash)]
3333
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
3434
#[cfg_attr(feature = "visitor", derive(Visit, VisitMut))]
35-
pub enum EnumValue {
36-
String(String),
35+
pub enum EnumMember {
36+
Name(String),
3737
/// ClickHouse allows to specify an integer value for each enum value.
3838
///
3939
/// [clickhouse](https://clickhouse.com/docs/en/sql-reference/data-types/enum)
40-
Pair(String, Value),
40+
NamedValue(String, Expr),
4141
}
4242

4343
/// SQL data types
@@ -345,7 +345,7 @@ pub enum DataType {
345345
/// [clickhouse]: https://clickhouse.com/docs/en/sql-reference/data-types/nested-data-structures/nested
346346
Nested(Vec<ColumnDef>),
347347
/// Enums
348-
Enum(Vec<EnumValue>, Option<i64>),
348+
Enum(Vec<EnumMember>, Option<u8>),
349349
/// Set
350350
Set(Vec<String>),
351351
/// Struct
@@ -568,9 +568,11 @@ impl fmt::Display for DataType {
568568
write!(f, ", ")?;
569569
}
570570
match v {
571-
EnumValue::String(v) => write!(f, "'{}'", escape_single_quote_string(v))?,
572-
EnumValue::Pair(v, i) => {
573-
write!(f, "'{}' = {}", escape_single_quote_string(v), i)?
571+
EnumMember::Name(name) => {
572+
write!(f, "'{}'", escape_single_quote_string(name))?
573+
}
574+
EnumMember::NamedValue(name, value) => {
575+
write!(f, "'{}' = {}", escape_single_quote_string(name), value)?
574576
}
575577
}
576578
}

‎src/ast/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ use sqlparser_derive::{Visit, VisitMut};
4040
use crate::tokenizer::Span;
4141

4242
pub use self::data_type::{
43-
ArrayElemTypeDef, CharLengthUnits, CharacterLength, DataType, EnumValue, ExactNumberInfo,
43+
ArrayElemTypeDef, CharLengthUnits, CharacterLength, DataType, EnumMember, ExactNumberInfo,
4444
StructBracketKind, TimezoneInfo,
4545
};
4646
pub use self::dcl::{AlterRoleOperation, ResetConfig, RoleOption, SetConfigValue, Use};

‎src/dialect/clickhouse.rs

-6
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,4 @@ impl Dialect for ClickHouseDialect {
5050
fn supports_limit_comma(&self) -> bool {
5151
true
5252
}
53-
54-
/// ClickHouse supports `Enum8` and `Enum16` types.
55-
/// See [ClickHouse](https://clickhouse.com/docs/en/sql-reference/data-types/enum)
56-
fn supports_enum_type_with_bits(&self) -> bool {
57-
true
58-
}
5953
}

‎src/dialect/mod.rs

-5
Original file line numberDiff line numberDiff line change
@@ -707,11 +707,6 @@ pub trait Dialect: Debug + Any {
707707
fn is_reserved_for_identifier(&self, kw: Keyword) -> bool {
708708
keywords::RESERVED_FOR_IDENTIFIER.contains(&kw)
709709
}
710-
711-
/// Return true if the dialect supports the Enum type with bits like Enum8, Enum16
712-
fn supports_enum_type_with_bits(&self) -> bool {
713-
false
714-
}
715710
}
716711

717712
/// This represents the operators for which precedence must be defined

‎src/parser/mod.rs

+12-16
Original file line numberDiff line numberDiff line change
@@ -7997,25 +7997,21 @@ impl<'a> Parser<'a> {
79977997
}
79987998
}
79997999

8000-
pub fn parse_enum_values(&mut self) -> Result<Vec<EnumValue>, ParserError> {
8000+
pub fn parse_enum_values(&mut self) -> Result<Vec<EnumMember>, ParserError> {
80018001
self.expect_token(&Token::LParen)?;
8002-
let values = self.parse_comma_separated(Parser::parse_enum_value)?;
8002+
let values = self.parse_comma_separated(|parser| {
8003+
let name = parser.parse_literal_string()?;
8004+
let e = if parser.consume_token(&Token::Eq) {
8005+
let value = parser.parse_number()?;
8006+
EnumMember::NamedValue(name, value)
8007+
} else {
8008+
EnumMember::Name(name)
8009+
};
8010+
Ok(e)
8011+
})?;
80038012
self.expect_token(&Token::RParen)?;
8004-
Ok(values)
8005-
}
80068013

8007-
pub fn parse_enum_value(&mut self) -> Result<EnumValue, ParserError> {
8008-
let str = self.parse_literal_string()?;
8009-
let value = match self.peek_token().token {
8010-
Token::Eq => {
8011-
// Consume the `=` token
8012-
self.next_token();
8013-
let value = self.parse_number_value()?;
8014-
EnumValue::Pair(str, value)
8015-
}
8016-
_ => EnumValue::String(str),
8017-
};
8018-
Ok(value)
8014+
Ok(values)
80198015
}
80208016

80218017
/// Parse a SQL datatype (in the context of a CREATE TABLE statement for example)

‎tests/sqlparser_clickhouse.rs

-80
Original file line numberDiff line numberDiff line change
@@ -1621,86 +1621,6 @@ fn parse_explain_table() {
16211621
}
16221622
}
16231623

1624-
#[test]
1625-
fn parse_create_table_with_enum_types() {
1626-
let sql = "CREATE TABLE t0 (foo ENUM8('a' = 1, 'b' = 2), bar ENUM16('a' = 1, 'b' = 2), baz ENUM('a', 'b'))";
1627-
match clickhouse().verified_stmt(sql) {
1628-
Statement::CreateTable(CreateTable { name, columns, .. }) => {
1629-
assert_eq!(name.to_string(), "t0");
1630-
assert_eq!(
1631-
vec![
1632-
ColumnDef {
1633-
name: Ident::new("foo"),
1634-
data_type: DataType::Enum(
1635-
vec![
1636-
EnumValue::Pair(
1637-
"a".to_string(),
1638-
Number("1".parse().unwrap(), false)
1639-
),
1640-
EnumValue::Pair(
1641-
"b".to_string(),
1642-
Number("2".parse().unwrap(), false)
1643-
)
1644-
],
1645-
Some(8)
1646-
),
1647-
collation: None,
1648-
options: vec![],
1649-
},
1650-
ColumnDef {
1651-
name: Ident::new("bar"),
1652-
data_type: DataType::Enum(
1653-
vec![
1654-
EnumValue::Pair(
1655-
"a".to_string(),
1656-
Number("1".parse().unwrap(), false)
1657-
),
1658-
EnumValue::Pair(
1659-
"b".to_string(),
1660-
Number("2".parse().unwrap(), false)
1661-
)
1662-
],
1663-
Some(16)
1664-
),
1665-
collation: None,
1666-
options: vec![],
1667-
},
1668-
ColumnDef {
1669-
name: Ident::new("baz"),
1670-
data_type: DataType::Enum(
1671-
vec![
1672-
EnumValue::String("a".to_string()),
1673-
EnumValue::String("b".to_string())
1674-
],
1675-
None
1676-
),
1677-
collation: None,
1678-
options: vec![],
1679-
}
1680-
],
1681-
columns
1682-
);
1683-
}
1684-
_ => unreachable!(),
1685-
}
1686-
1687-
// invalid case missing value for enum pair
1688-
assert_eq!(
1689-
clickhouse_and_generic()
1690-
.parse_sql_statements("CREATE TABLE t0 (foo ENUM8('a' = 1, 'b' = ))")
1691-
.unwrap_err(),
1692-
ParserError("Expected: a value, found: )".to_string())
1693-
);
1694-
1695-
// invalid case that name is not a string
1696-
assert_eq!(
1697-
clickhouse_and_generic()
1698-
.parse_sql_statements("CREATE TABLE t0 (foo ENUM8('a' = 1, 2))")
1699-
.unwrap_err(),
1700-
ParserError("Expected: literal string, found: 2".to_string())
1701-
);
1702-
}
1703-
17041624
fn clickhouse() -> TestedDialects {
17051625
TestedDialects::new(vec![Box::new(ClickHouseDialect {})])
17061626
}

‎tests/sqlparser_common.rs

+83-3
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ mod test_utils;
5151
use pretty_assertions::assert_eq;
5252
use sqlparser::ast::ColumnOption::Comment;
5353
use sqlparser::ast::Expr::{Identifier, UnaryOp};
54+
use sqlparser::ast::Value::Number;
5455
use sqlparser::test_utils::all_dialects_except;
5556

5657
#[test]
@@ -9250,7 +9251,7 @@ fn parse_cache_table() {
92509251
format!(
92519252
"CACHE {table_flag} TABLE '{cache_table_name}' OPTIONS('K1' = 'V1', 'K2' = 0.88) {sql}",
92529253
)
9253-
.as_str()
9254+
.as_str()
92549255
),
92559256
Statement::Cache {
92569257
table_flag: Some(ObjectName(vec![Ident::new(table_flag)])),
@@ -9275,7 +9276,7 @@ fn parse_cache_table() {
92759276
format!(
92769277
"CACHE {table_flag} TABLE '{cache_table_name}' OPTIONS('K1' = 'V1', 'K2' = 0.88) AS {sql}",
92779278
)
9278-
.as_str()
9279+
.as_str()
92799280
),
92809281
Statement::Cache {
92819282
table_flag: Some(ObjectName(vec![Ident::new(table_flag)])),
@@ -11459,7 +11460,7 @@ fn parse_explain_with_option_list() {
1145911460
}),
1146011461
},
1146111462
];
11462-
run_explain_analyze (
11463+
run_explain_analyze(
1146311464
all_dialects_where(|d| d.supports_explain_with_utility_options()),
1146411465
"EXPLAIN (ANALYZE, VERBOSE true, WAL OFF, FORMAT YAML, USER_DEF_NUM -100.1) SELECT sqrt(id) FROM foo",
1146511466
false,
@@ -12459,3 +12460,82 @@ fn parse_create_table_with_bit_types() {
1245912460
_ => unreachable!(),
1246012461
}
1246112462
}
12463+
12464+
fn parse_create_table_with_enum_types() {
12465+
let sql = "CREATE TABLE t0 (foo ENUM8('a' = 1, 'b' = 2), bar ENUM16('a' = 1, 'b' = 2), baz ENUM('a', 'b'))";
12466+
match all_dialects().verified_stmt(sql) {
12467+
Statement::CreateTable(CreateTable { name, columns, .. }) => {
12468+
std::assert_eq!(name.to_string(), "t0");
12469+
std::assert_eq!(
12470+
vec![
12471+
ColumnDef {
12472+
name: Ident::new("foo"),
12473+
data_type: DataType::Enum(
12474+
vec![
12475+
EnumMember::NamedValue(
12476+
"a".to_string(),
12477+
Expr::Value(Number("1".parse().unwrap(), false))
12478+
),
12479+
EnumMember::NamedValue(
12480+
"b".to_string(),
12481+
Expr::Value(Number("2".parse().unwrap(), false))
12482+
)
12483+
],
12484+
Some(8)
12485+
),
12486+
collation: None,
12487+
options: vec![],
12488+
},
12489+
ColumnDef {
12490+
name: Ident::new("bar"),
12491+
data_type: DataType::Enum(
12492+
vec![
12493+
EnumMember::NamedValue(
12494+
"a".to_string(),
12495+
Expr::Value(Number("1".parse().unwrap(), false))
12496+
),
12497+
EnumMember::NamedValue(
12498+
"b".to_string(),
12499+
Expr::Value(Number("2".parse().unwrap(), false))
12500+
)
12501+
],
12502+
Some(16)
12503+
),
12504+
collation: None,
12505+
options: vec![],
12506+
},
12507+
ColumnDef {
12508+
name: Ident::new("baz"),
12509+
data_type: DataType::Enum(
12510+
vec![
12511+
EnumMember::Name("a".to_string()),
12512+
EnumMember::Name("b".to_string())
12513+
],
12514+
None
12515+
),
12516+
collation: None,
12517+
options: vec![],
12518+
}
12519+
],
12520+
columns
12521+
);
12522+
}
12523+
_ => unreachable!(),
12524+
}
12525+
12526+
// invalid case missing value for enum pair
12527+
std::assert_eq!(
12528+
all_dialects()
12529+
.parse_sql_statements("CREATE TABLE t0 (foo ENUM8('a' = 1, 'b' = ))")
12530+
.unwrap_err(),
12531+
ParserError::ParserError("Expected: a value, found: )".to_string())
12532+
);
12533+
12534+
// invalid case that name is not a string
12535+
std::assert_eq!(
12536+
all_dialects()
12537+
.parse_sql_statements("CREATE TABLE t0 (foo ENUM8('a' = 1, 2))")
12538+
.unwrap_err(),
12539+
ParserError::ParserError("Expected: literal string, found: 2".to_string())
12540+
);
12541+
}

‎tests/sqlparser_mysql.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -892,8 +892,8 @@ fn parse_create_table_set_enum() {
892892
name: Ident::new("baz"),
893893
data_type: DataType::Enum(
894894
vec![
895-
EnumValue::String("a".to_string()),
896-
EnumValue::String("b".to_string())
895+
EnumMember::Name("a".to_string()),
896+
EnumMember::Name("b".to_string())
897897
],
898898
None
899899
),

0 commit comments

Comments
 (0)