Skip to content

Commit

Permalink
feat: convert table name to quoted style (apache#454)
Browse files Browse the repository at this point in the history
* convert table name to quoted style in sql statement.

* add unit test.

* revert pr444(apache#444).

* refactor related integration tests.

* remove useless commets.

* refactor the way of extracting table name.
  • Loading branch information
Rachelint authored Dec 7, 2022
1 parent fd0b0ea commit aa99551
Show file tree
Hide file tree
Showing 4 changed files with 143 additions and 14 deletions.
12 changes: 2 additions & 10 deletions sql/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! SQL statement

use sqlparser::ast::{
ColumnDef, Ident, ObjectName, SqlOption, Statement as SqlStatement, TableConstraint,
ColumnDef, ObjectName, SqlOption, Statement as SqlStatement, TableConstraint,
};

/// Statement representations
Expand Down Expand Up @@ -33,22 +33,14 @@ impl TableName {
pub fn is_empty(&self) -> bool {
self.0 .0.is_empty()
}

// Normalize an identifer to a lowercase string unless the identifier is quoted.
fn normalize_ident(id: &Ident) -> String {
match id.quote_style {
Some(_) => id.value.clone(),
None => id.value.to_ascii_lowercase(),
}
}
}

impl ToString for TableName {
fn to_string(&self) -> String {
self.0
.0
.iter()
.map(Self::normalize_ident)
.map(|ident| ident.value.as_str())
.collect::<Vec<_>>()
.join(".")
}
Expand Down
98 changes: 94 additions & 4 deletions sql/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@
use log::debug;
use paste::paste;
use sqlparser::{
ast::{ColumnDef, ColumnOption, ColumnOptionDef, Expr, Ident, TableConstraint},
ast::{
ColumnDef, ColumnOption, ColumnOptionDef, Expr, Ident, ObjectName, SetExpr,
Statement as SqlStatement, TableConstraint, TableFactor, TableWithJoins,
},
dialect::{keywords::Keyword, Dialect, MySqlDialect},
parser::{IsOptional::Mandatory, Parser as SqlParser, ParserError},
tokenizer::{Token, Tokenizer},
Expand Down Expand Up @@ -179,9 +182,9 @@ impl<'a> Parser<'a> {
}
_ => {
// use the native parser
Ok(Statement::Standard(Box::new(
self.parser.parse_statement()?,
)))
let mut statement = self.parser.parse_statement()?;
maybe_normalize_table_name(&mut statement);
Ok(Statement::Standard(Box::new(statement)))
}
}
}
Expand Down Expand Up @@ -540,6 +543,43 @@ fn build_timestamp_key_constraint(col_defs: &[ColumnDef], constraints: &mut Vec<
}
}

/// Add quotes in table name (for example: convert table to `table`).
///
/// It is used to process table name in `SELECT`, for preventing `datafusion`
/// converting the table name to lowercase, because `CeresDB` only support
/// case-sensitive in sql.
// TODO: maybe other items(such as: alias, column name) need to be normalized,
// too.
pub fn maybe_normalize_table_name(statement: &mut SqlStatement) {
if let SqlStatement::Query(query) = statement {
if let SetExpr::Select(select) = query.body.as_mut() {
select.from.iter_mut().for_each(maybe_convert_one_from);
}
}
}

fn maybe_convert_one_from(one_from: &mut TableWithJoins) {
let TableWithJoins { relation, joins } = one_from;
maybe_convert_relation(relation);
joins.iter_mut().for_each(|join| {
maybe_convert_relation(&mut join.relation);
});
}

fn maybe_convert_relation(relation: &mut TableFactor) {
if let TableFactor::Table { name, .. } = relation {
maybe_convert_table_name(name);
}
}

fn maybe_convert_table_name(object_name: &mut ObjectName) {
object_name.0.iter_mut().for_each(|id| {
if id.quote_style.is_none() {
let _ = std::mem::replace(id, Ident::with_quote('`', id.value.clone()));
}
})
}

#[cfg(test)]
mod tests {
use sqlparser::ast::{ColumnOptionDef, DataType, Ident, ObjectName, Value};
Expand Down Expand Up @@ -937,4 +977,54 @@ mod tests {
assert!(Parser::parse_sql(sql).is_err());
}
}

#[test]
fn test_normalizing_table_name_in_select() {
{
let sql = "select * from testa;";
let statements = Parser::parse_sql(sql).unwrap();
assert!(
if let Statement::Standard(standard_statement) = &statements[0] {
let standard_statement_str = format!("{}", standard_statement);
assert!(standard_statement_str.contains("`testa`"));

true
} else {
false
}
)
}

{
let sql = "select * from `testa`";
let statements = Parser::parse_sql(sql).unwrap();
assert!(
if let Statement::Standard(standard_statement) = &statements[0] {
let standard_statement_str = format!("{}", standard_statement);
assert!(standard_statement_str.contains("`testa`"));

true
} else {
false
}
)
}

{
let sql = "select * from `testa` join TEstB join TESTC";
let statements = Parser::parse_sql(sql).unwrap();
assert!(
if let Statement::Standard(standard_statement) = &statements[0] {
let standard_statement_str = format!("{}", standard_statement);
assert!(standard_statement_str.contains("`testa`"));
assert!(standard_statement_str.contains("`TEstB`"));
assert!(standard_statement_str.contains("`TESTC`"));

true
} else {
false
}
)
}
}
}
30 changes: 30 additions & 0 deletions tests/cases/local/03_dml/case_insensitive.result
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,26 @@ SELECT
FROM
CASE_INSENSITIVE_TABLE1;

Failed to execute query, err: Server(ServerError { code: 500, msg: "Failed to create plan, query: SELECT \n * \n FROM \n CASE_INSENSITIVE_TABLE1;. Caused by: Failed to create plan, err:Failed to generate datafusion plan, err:Execution error: Table is not found, \"table:CASE_INSENSITIVE_TABLE1\"" })

SELECT
*
FROM
`case_insensitive_table1`;

tsid,ts,value1,
Int64(0),Timestamp(Timestamp(1)),Double(10.0),
Int64(0),Timestamp(Timestamp(2)),Double(20.0),
Int64(0),Timestamp(Timestamp(3)),Double(30.0),


SELECT
*
FROM
`CASE_INSENSITIVE_TABLE1`;

Failed to execute query, err: Server(ServerError { code: 500, msg: "Failed to create plan, query: SELECT \n * \n FROM \n `CASE_INSENSITIVE_TABLE1`;. Caused by: Failed to create plan, err:Failed to generate datafusion plan, err:Execution error: Table is not found, \"table:CASE_INSENSITIVE_TABLE1\"" })

SHOW CREATE TABLE case_insensitive_table1;

Table,Create Table,
Expand All @@ -47,10 +61,18 @@ String(StringBytes(b"case_insensitive_table1")),String(StringBytes(b"CREATE TABL

SHOW CREATE TABLE CASE_INSENSITIVE_TABLE1;

Failed to execute query, err: Server(ServerError { code: 500, msg: "Failed to create plan, query: SHOW CREATE TABLE CASE_INSENSITIVE_TABLE1;. Caused by: Failed to create plan, err:Table not found, table:CASE_INSENSITIVE_TABLE1" })

SHOW CREATE TABLE `case_insensitive_table1`;

Table,Create Table,
String(StringBytes(b"case_insensitive_table1")),String(StringBytes(b"CREATE TABLE `case_insensitive_table1` (`tsid` uint64 NOT NULL, `ts` timestamp NOT NULL, `value1` double, PRIMARY KEY(tsid,ts), TIMESTAMP KEY(ts)) ENGINE=Analytic WITH(arena_block_size='2097152', compaction_strategy='default', compression='ZSTD', enable_ttl='false', num_rows_per_row_group='8192', segment_duration='', storage_format='COLUMNAR', ttl='7d', update_mode='OVERWRITE', write_buffer_size='33554432')")),


SHOW CREATE TABLE `CASE_INSENSITIVE_TABLE1`;

Failed to execute query, err: Server(ServerError { code: 500, msg: "Failed to create plan, query: SHOW CREATE TABLE `CASE_INSENSITIVE_TABLE1`;. Caused by: Failed to create plan, err:Table not found, table:CASE_INSENSITIVE_TABLE1" })

DESC case_insensitive_table1;

name,type,is_primary,is_nullable,is_tag,
Expand All @@ -61,9 +83,17 @@ String(StringBytes(b"value1")),String(StringBytes(b"double")),Boolean(false),Boo

DESC CASE_INSENSITIVE_TABLE1;

Failed to execute query, err: Server(ServerError { code: 500, msg: "Failed to create plan, query: DESC CASE_INSENSITIVE_TABLE1;. Caused by: Failed to create plan, err:Table not found, table:CASE_INSENSITIVE_TABLE1" })

DESC `case_insensitive_table1`;

name,type,is_primary,is_nullable,is_tag,
String(StringBytes(b"tsid")),String(StringBytes(b"uint64")),Boolean(true),Boolean(false),Boolean(false),
String(StringBytes(b"ts")),String(StringBytes(b"timestamp")),Boolean(true),Boolean(false),Boolean(false),
String(StringBytes(b"value1")),String(StringBytes(b"double")),Boolean(false),Boolean(true),Boolean(false),


DESC `CASE_INSENSITIVE_TABLE1`;

Failed to execute query, err: Server(ServerError { code: 500, msg: "Failed to create plan, query: DESC `CASE_INSENSITIVE_TABLE1`;. Caused by: Failed to create plan, err:Table not found, table:CASE_INSENSITIVE_TABLE1" })

17 changes: 17 additions & 0 deletions tests/cases/local/03_dml/case_insensitive.sql
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,28 @@ SELECT
FROM
CASE_INSENSITIVE_TABLE1;

SELECT
*
FROM
`case_insensitive_table1`;

SELECT
*
FROM
`CASE_INSENSITIVE_TABLE1`;

SHOW CREATE TABLE case_insensitive_table1;

SHOW CREATE TABLE CASE_INSENSITIVE_TABLE1;

SHOW CREATE TABLE `case_insensitive_table1`;

SHOW CREATE TABLE `CASE_INSENSITIVE_TABLE1`;

DESC case_insensitive_table1;

DESC CASE_INSENSITIVE_TABLE1;

DESC `case_insensitive_table1`;

DESC `CASE_INSENSITIVE_TABLE1`;

0 comments on commit aa99551

Please sign in to comment.