Skip to content

Commit f8b6a6b

Browse files
MichaelScofieldsunng87
MichaelScofield
and
sunng87
authored
fix!: not allowed to create column name same with keyword without quoted (#1333)
* fix: not allowed to create column name same with keyword without quoted * fix: tests * Update src/sql/src/parsers/create_parser.rs Co-authored-by: Ning Sun <[email protected]> * fix: tests --------- Co-authored-by: Ning Sun <[email protected]>
1 parent dce0adf commit f8b6a6b

File tree

7 files changed

+58
-20
lines changed

7 files changed

+58
-20
lines changed

‎src/datanode/src/sql/create.rs

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,12 @@ mod tests {
349349

350350
#[tokio::test]
351351
async fn test_create_table_with_options() {
352-
let sql = r#"CREATE TABLE demo_table (timestamp BIGINT TIME INDEX, value DOUBLE, host STRING PRIMARY KEY) engine=mito with(regions=1, ttl='7days',write_buffer_size='32MB',some='other');"#;
352+
let sql = r#"
353+
CREATE TABLE demo_table (
354+
"timestamp" BIGINT TIME INDEX,
355+
"value" DOUBLE,
356+
host STRING PRIMARY KEY
357+
) engine=mito with(regions=1, ttl='7days',write_buffer_size='32MB',some='other');"#;
353358
let parsed_stmt = sql_to_statement(sql);
354359
let handler = create_mock_sql_handler().await;
355360
let c = handler
@@ -368,7 +373,12 @@ mod tests {
368373
pub async fn test_create_with_inline_primary_key() {
369374
let handler = create_mock_sql_handler().await;
370375
let parsed_stmt = sql_to_statement(
371-
r#"CREATE TABLE demo_table (timestamp BIGINT TIME INDEX, value DOUBLE, host STRING PRIMARY KEY) engine=mito with(regions=1);"#,
376+
r#"
377+
CREATE TABLE demo_table(
378+
"timestamp" BIGINT TIME INDEX,
379+
"value" DOUBLE,
380+
host STRING PRIMARY KEY
381+
) engine=mito with(regions=1);"#,
372382
);
373383
let c = handler
374384
.create_to_request(42, parsed_stmt, &TableReference::bare("demo_table"))
@@ -407,8 +417,8 @@ mod tests {
407417
let handler = create_mock_sql_handler().await;
408418
let parsed_stmt = sql_to_statement(
409419
r#"create table demo_table (
410-
timestamp BIGINT TIME INDEX,
411-
value DOUBLE,
420+
"timestamp" BIGINT TIME INDEX,
421+
"value" DOUBLE,
412422
host STRING PRIMARY KEY,
413423
PRIMARY KEY(host)) engine=mito with(regions=1);"#,
414424
);
@@ -423,8 +433,8 @@ mod tests {
423433
let handler = create_mock_sql_handler().await;
424434
let parsed_stmt = sql_to_statement(
425435
r#"create table demo_table (
426-
timestamp BIGINT TIME INDEX,
427-
value DOUBLE PRIMARY KEY,
436+
"timestamp" BIGINT TIME INDEX,
437+
"value" DOUBLE PRIMARY KEY,
428438
host STRING PRIMARY KEY) engine=mito with(regions=1);"#,
429439
);
430440
let error = handler

‎src/frontend/src/tests/promql_test.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,8 @@ async fn sql_insert_promql_query_ceil(instance: Arc<dyn MockInstance>) {
176176
const AGGREGATORS_CREATE_TABLE: &str = r#"create table http_requests (
177177
job string,
178178
instance string,
179-
group string,
180-
value double,
179+
"group" string,
180+
"value" double,
181181
ts timestamp TIME INDEX,
182182
PRIMARY KEY (job, instance, group),
183183
);"#;

‎src/sql/src/parsers/create_parser.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ use once_cell::sync::Lazy;
2020
use snafu::{ensure, OptionExt, ResultExt};
2121
use sqlparser::ast::{ColumnOption, ColumnOptionDef, DataType, Value};
2222
use sqlparser::dialect::keywords::Keyword;
23+
use sqlparser::keywords::ALL_KEYWORDS;
2324
use sqlparser::parser::IsOptional::Mandatory;
2425
use sqlparser::parser::{Parser, ParserError};
2526
use sqlparser::tokenizer::{Token, TokenWithLocation, Word};
@@ -386,6 +387,16 @@ impl<'a> ParserContext<'a> {
386387
let parser = &mut self.parser;
387388

388389
let name = parser.parse_identifier()?;
390+
if name.quote_style.is_none() &&
391+
// "ALL_KEYWORDS" are sorted.
392+
ALL_KEYWORDS.binary_search(&name.value.to_uppercase().as_str()).is_ok()
393+
{
394+
return Err(ParserError::ParserError(format!(
395+
"Cannot use keyword '{}' as column name. Hint: add quotes to the name.",
396+
&name.value
397+
)));
398+
}
399+
389400
let data_type = parser.parse_data_type()?;
390401
let collation = if parser.parse_keyword(Keyword::COLLATE) {
391402
Some(parser.parse_object_name()?)
@@ -1441,4 +1452,21 @@ ENGINE=mito";
14411452
assert!(result.is_err());
14421453
assert_matches!(result, Err(crate::error::Error::InvalidTimeIndex { .. }));
14431454
}
1455+
1456+
#[test]
1457+
fn test_invalid_column_name() {
1458+
let sql = "create table foo(user string, i bigint time index)";
1459+
let result = ParserContext::create_with_dialect(sql, &GenericDialect {});
1460+
assert!(result
1461+
.unwrap_err()
1462+
.to_string()
1463+
.contains("Cannot use keyword 'user' as column name"));
1464+
1465+
// If column name is quoted, it's valid even same with keyword.
1466+
let sql = r#"
1467+
create table foo("user" string, i bigint time index)
1468+
"#;
1469+
let result = ParserContext::create_with_dialect(sql, &GenericDialect {});
1470+
assert!(result.is_ok());
1471+
}
14441472
}

‎tests/cases/standalone/common/create/create.result

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ DROP TABLE test2;
8888

8989
Affected Rows: 1
9090

91-
CREATE TABLE test_pk (timestamp BIGINT TIME INDEX, host STRING PRIMARY KEY, value DOUBLE);
91+
CREATE TABLE test_pk ("timestamp" BIGINT TIME INDEX, host STRING PRIMARY KEY, "value" DOUBLE);
9292

9393
Affected Rows: 0
9494

@@ -106,15 +106,15 @@ DROP TABLE test_pk;
106106

107107
Affected Rows: 1
108108

109-
CREATE TABLE test_multiple_pk_definitions (timestamp BIGINT TIME INDEX, host STRING PRIMARY KEY, value DOUBLE, PRIMARY KEY(host));
109+
CREATE TABLE test_multiple_pk_definitions ("timestamp" BIGINT TIME INDEX, host STRING PRIMARY KEY, "value" DOUBLE, PRIMARY KEY(host));
110110

111111
Error: 1004(InvalidArguments), Illegal primary keys definition: found definitions of primary keys in multiple places
112112

113-
CREATE TABLE test_multiple_pk_definitions (timestamp BIGINT TIME INDEX, host STRING PRIMARY KEY, value DOUBLE, PRIMARY KEY(host), PRIMARY KEY(host));
113+
CREATE TABLE test_multiple_pk_definitions ("timestamp" BIGINT TIME INDEX, host STRING PRIMARY KEY, "value" DOUBLE, PRIMARY KEY(host), PRIMARY KEY(host));
114114

115115
Error: 1004(InvalidArguments), Illegal primary keys definition: found definitions of primary keys in multiple places
116116

117-
CREATE TABLE test_multiple_inline_pk_definitions (timestamp BIGINT TIME INDEX, host STRING PRIMARY KEY, value DOUBLE PRIMARY KEY);
117+
CREATE TABLE test_multiple_inline_pk_definitions ("timestamp" BIGINT TIME INDEX, host STRING PRIMARY KEY, "value" DOUBLE PRIMARY KEY);
118118

119119
Error: 1004(InvalidArguments), Illegal primary keys definition: not allowed to inline multiple primary keys in columns options
120120

‎tests/cases/standalone/common/create/create.sql

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,15 @@ DROP TABLE test1;
3636

3737
DROP TABLE test2;
3838

39-
CREATE TABLE test_pk (timestamp BIGINT TIME INDEX, host STRING PRIMARY KEY, value DOUBLE);
39+
CREATE TABLE test_pk ("timestamp" BIGINT TIME INDEX, host STRING PRIMARY KEY, "value" DOUBLE);
4040

4141
DESC TABLE test_pk;
4242

4343
DROP TABLE test_pk;
4444

45-
CREATE TABLE test_multiple_pk_definitions (timestamp BIGINT TIME INDEX, host STRING PRIMARY KEY, value DOUBLE, PRIMARY KEY(host));
45+
CREATE TABLE test_multiple_pk_definitions ("timestamp" BIGINT TIME INDEX, host STRING PRIMARY KEY, "value" DOUBLE, PRIMARY KEY(host));
4646

47-
CREATE TABLE test_multiple_pk_definitions (timestamp BIGINT TIME INDEX, host STRING PRIMARY KEY, value DOUBLE, PRIMARY KEY(host), PRIMARY KEY(host));
47+
CREATE TABLE test_multiple_pk_definitions ("timestamp" BIGINT TIME INDEX, host STRING PRIMARY KEY, "value" DOUBLE, PRIMARY KEY(host), PRIMARY KEY(host));
4848

49-
CREATE TABLE test_multiple_inline_pk_definitions (timestamp BIGINT TIME INDEX, host STRING PRIMARY KEY, value DOUBLE PRIMARY KEY);
49+
CREATE TABLE test_multiple_inline_pk_definitions ("timestamp" BIGINT TIME INDEX, host STRING PRIMARY KEY, "value" DOUBLE PRIMARY KEY);
5050

‎tests/cases/standalone/order/order_variable_size_payload.result

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -352,8 +352,8 @@ CREATE TABLE DirectReports
352352
Name varchar NOT NULL,
353353
Title varchar NOT NULL,
354354
EmployeeLevel int NOT NULL,
355-
Sort varchar NOT NULL,
356-
Timestamp BIGINT TIME INDEX,
355+
"Sort" varchar NOT NULL,
356+
"Timestamp" BIGINT TIME INDEX,
357357
);
358358

359359
Affected Rows: 0

‎tests/cases/standalone/order/order_variable_size_payload.sql

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@ CREATE TABLE DirectReports
8787
Name varchar NOT NULL,
8888
Title varchar NOT NULL,
8989
EmployeeLevel int NOT NULL,
90-
Sort varchar NOT NULL,
91-
Timestamp BIGINT TIME INDEX,
90+
"Sort" varchar NOT NULL,
91+
"Timestamp" BIGINT TIME INDEX,
9292
);
9393

9494
INSERT INTO DirectReports VALUES

0 commit comments

Comments
 (0)