Skip to content

Commit 070d38a

Browse files
git-hulkgit-hulk
committed
Fix review comments
1 parent 39d0109 commit 070d38a

File tree

8 files changed

+109
-121
lines changed

8 files changed

+109
-121
lines changed

‎src/ast/data_type.rs

Lines changed: 10 additions & 8 deletions
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
@@ -335,7 +335,7 @@ pub enum DataType {
335335
/// [clickhouse]: https://clickhouse.com/docs/en/sql-reference/data-types/nested-data-structures/nested
336336
Nested(Vec<ColumnDef>),
337337
/// Enums
338-
Enum(Vec<EnumValue>, Option<i64>),
338+
Enum(Vec<EnumMember>, Option<u8>),
339339
/// Set
340340
Set(Vec<String>),
341341
/// Struct
@@ -554,9 +554,11 @@ impl fmt::Display for DataType {
554554
write!(f, ", ")?;
555555
}
556556
match v {
557-
EnumValue::String(v) => write!(f, "'{}'", escape_single_quote_string(v))?,
558-
EnumValue::Pair(v, i) => {
559-
write!(f, "'{}' = {}", escape_single_quote_string(v), i)?
557+
EnumMember::Name(name) => {
558+
write!(f, "'{}'", escape_single_quote_string(name))?
559+
}
560+
EnumMember::NamedValue(name, value) => {
561+
write!(f, "'{}' = {}", escape_single_quote_string(name), value)?
560562
}
561563
}
562564
}

‎src/ast/mod.rs

Lines changed: 1 addition & 1 deletion
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

Lines changed: 0 additions & 6 deletions
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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -697,11 +697,6 @@ pub trait Dialect: Debug + Any {
697697
fn is_reserved_for_identifier(&self, kw: Keyword) -> bool {
698698
keywords::RESERVED_FOR_IDENTIFIER.contains(&kw)
699699
}
700-
701-
/// Return true if the dialect supports the Enum type with bits like Enum8, Enum16
702-
fn supports_enum_type_with_bits(&self) -> bool {
703-
false
704-
}
705700
}
706701

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

‎src/parser/mod.rs

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7979,25 +7979,21 @@ impl<'a> Parser<'a> {
79797979
}
79807980
}
79817981

7982-
pub fn parse_enum_values(&mut self) -> Result<Vec<EnumValue>, ParserError> {
7982+
pub fn parse_enum_values(&mut self) -> Result<Vec<EnumMember>, ParserError> {
79837983
self.expect_token(&Token::LParen)?;
7984-
let values = self.parse_comma_separated(Parser::parse_enum_value)?;
7984+
let values = self.parse_comma_separated(|parser| {
7985+
let name = parser.parse_literal_string()?;
7986+
let e = if parser.consume_token(&Token::Eq) {
7987+
let value = parser.parse_number()?;
7988+
EnumMember::NamedValue(name, value)
7989+
} else {
7990+
EnumMember::Name(name)
7991+
};
7992+
Ok(e)
7993+
})?;
79857994
self.expect_token(&Token::RParen)?;
7986-
Ok(values)
7987-
}
79887995

7989-
pub fn parse_enum_value(&mut self) -> Result<EnumValue, ParserError> {
7990-
let str = self.parse_literal_string()?;
7991-
let value = match self.peek_token().token {
7992-
Token::Eq => {
7993-
// Consume the `=` token
7994-
self.next_token();
7995-
let value = self.parse_number_value()?;
7996-
EnumValue::Pair(str, value)
7997-
}
7998-
_ => EnumValue::String(str),
7999-
};
8000-
Ok(value)
7996+
Ok(values)
80017997
}
80027998

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

‎tests/sqlparser_clickhouse.rs

Lines changed: 0 additions & 80 deletions
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

Lines changed: 84 additions & 3 deletions
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,
@@ -12440,3 +12441,83 @@ fn test_reserved_keywords_for_identifiers() {
1244012441
let sql = "SELECT MAX(interval) FROM tbl";
1244112442
dialects.parse_sql_statements(sql).unwrap();
1244212443
}
12444+
12445+
#[test]
12446+
fn parse_create_table_with_enum_types() {
12447+
let sql = "CREATE TABLE t0 (foo ENUM8('a' = 1, 'b' = 2), bar ENUM16('a' = 1, 'b' = 2), baz ENUM('a', 'b'))";
12448+
match all_dialects().verified_stmt(sql) {
12449+
Statement::CreateTable(CreateTable { name, columns, .. }) => {
12450+
std::assert_eq!(name.to_string(), "t0");
12451+
std::assert_eq!(
12452+
vec![
12453+
ColumnDef {
12454+
name: Ident::new("foo"),
12455+
data_type: DataType::Enum(
12456+
vec![
12457+
EnumMember::NamedValue(
12458+
"a".to_string(),
12459+
Expr::Value(Number("1".parse().unwrap(), false))
12460+
),
12461+
EnumMember::NamedValue(
12462+
"b".to_string(),
12463+
Expr::Value(Number("2".parse().unwrap(), false))
12464+
)
12465+
],
12466+
Some(8)
12467+
),
12468+
collation: None,
12469+
options: vec![],
12470+
},
12471+
ColumnDef {
12472+
name: Ident::new("bar"),
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(16)
12485+
),
12486+
collation: None,
12487+
options: vec![],
12488+
},
12489+
ColumnDef {
12490+
name: Ident::new("baz"),
12491+
data_type: DataType::Enum(
12492+
vec![
12493+
EnumMember::Name("a".to_string()),
12494+
EnumMember::Name("b".to_string())
12495+
],
12496+
None
12497+
),
12498+
collation: None,
12499+
options: vec![],
12500+
}
12501+
],
12502+
columns
12503+
);
12504+
}
12505+
_ => unreachable!(),
12506+
}
12507+
12508+
// invalid case missing value for enum pair
12509+
std::assert_eq!(
12510+
all_dialects()
12511+
.parse_sql_statements("CREATE TABLE t0 (foo ENUM8('a' = 1, 'b' = ))")
12512+
.unwrap_err(),
12513+
ParserError::ParserError("Expected: a value, found: )".to_string())
12514+
);
12515+
12516+
// invalid case that name is not a string
12517+
std::assert_eq!(
12518+
all_dialects()
12519+
.parse_sql_statements("CREATE TABLE t0 (foo ENUM8('a' = 1, 2))")
12520+
.unwrap_err(),
12521+
ParserError::ParserError("Expected: literal string, found: 2".to_string())
12522+
);
12523+
}

‎tests/sqlparser_mysql.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -891,8 +891,8 @@ fn parse_create_table_set_enum() {
891891
name: Ident::new("baz"),
892892
data_type: DataType::Enum(
893893
vec![
894-
EnumValue::String("a".to_string()),
895-
EnumValue::String("b".to_string())
894+
EnumMember::Name("a".to_string()),
895+
EnumMember::Name("b".to_string())
896896
],
897897
None
898898
),

0 commit comments

Comments
 (0)