Skip to content

Commit

Permalink
feat: remove sql in error desc (GreptimeTeam#4589)
Browse files Browse the repository at this point in the history
Signed-off-by: Ruihang Xia <[email protected]>
  • Loading branch information
waynexia authored and CookiePieWw committed Sep 17, 2024
1 parent 3c287be commit 6e1d4a3
Show file tree
Hide file tree
Showing 19 changed files with 83 additions and 78 deletions.
86 changes: 68 additions & 18 deletions src/sql/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,29 +38,38 @@ pub type Result<T> = std::result::Result<T, Error>;
#[snafu(visibility(pub))]
#[stack_trace_debug]
pub enum Error {
#[snafu(display("SQL statement is not supported: {}, keyword: {}", sql, keyword))]
Unsupported { sql: String, keyword: String },
#[snafu(display("SQL statement is not supported, keyword: {}", keyword))]
Unsupported {
keyword: String,
#[snafu(implicit)]
location: Location,
},

#[snafu(display(
"Unexpected token while parsing SQL statement: {}, expected: '{}', found: {}",
sql,
"Unexpected token while parsing SQL statement, expected: '{}', found: {}",
expected,
actual,
))]
Unexpected {
sql: String,
expected: String,
actual: String,
#[snafu(source)]
error: ParserError,
#[snafu(implicit)]
location: Location,
},

#[snafu(display(
"Unsupported expr in default constraint: {:?} for column: {}",
expr,
column_name
))]
UnsupportedDefaultValue { column_name: String, expr: Expr },
UnsupportedDefaultValue {
column_name: String,
expr: Expr,
#[snafu(implicit)]
location: Location,
},

// Syntax error from sql parser.
#[snafu(display(""))]
Expand All @@ -84,31 +93,52 @@ pub enum Error {
MissingTimeIndex {},

#[snafu(display("Invalid time index: {}", msg))]
InvalidTimeIndex { msg: String },
InvalidTimeIndex {
msg: String,
#[snafu(implicit)]
location: Location,
},

#[snafu(display("Invalid SQL, error: {}", msg))]
InvalidSql { msg: String },
InvalidSql {
msg: String,
#[snafu(implicit)]
location: Location,
},

#[snafu(display(
"Unexpected token while parsing SQL statement: {}, expected: '{}', found: {}",
sql,
"Unexpected token while parsing SQL statement, expected: '{}', found: {}",
expected,
actual,
))]
UnexpectedToken {
sql: String,
expected: String,
actual: String,
#[snafu(implicit)]
location: Location,
},

#[snafu(display("Invalid column option, column name: {}, error: {}", name, msg))]
InvalidColumnOption { name: String, msg: String },
InvalidColumnOption {
name: String,
msg: String,
#[snafu(implicit)]
location: Location,
},

#[snafu(display("SQL data type not supported yet: {:?}", t))]
SqlTypeNotSupported { t: crate::ast::DataType },
SqlTypeNotSupported {
t: crate::ast::DataType,
#[snafu(implicit)]
location: Location,
},

#[snafu(display("Failed to parse value: {}", msg))]
ParseSqlValue { msg: String },
ParseSqlValue {
msg: String,
#[snafu(implicit)]
location: Location,
},

#[snafu(display(
"Column {} expect type: {:?}, actual: {:?}",
Expand All @@ -120,10 +150,16 @@ pub enum Error {
column_name: String,
expect: ConcreteDataType,
actual: ConcreteDataType,
#[snafu(implicit)]
location: Location,
},

#[snafu(display("Invalid database name: {}", name))]
InvalidDatabaseName { name: String },
InvalidDatabaseName {
name: String,
#[snafu(implicit)]
location: Location,
},

#[snafu(display("Invalid interval provided: {}", reason))]
InvalidInterval {
Expand All @@ -140,10 +176,18 @@ pub enum Error {
},

#[snafu(display("Invalid table name: {}", name))]
InvalidTableName { name: String },
InvalidTableName {
name: String,
#[snafu(implicit)]
location: Location,
},

#[snafu(display("Invalid flow name: {}", name))]
InvalidFlowName { name: String },
InvalidFlowName {
name: String,
#[snafu(implicit)]
location: Location,
},

#[snafu(display("Invalid default constraint, column: {}", column))]
InvalidDefault {
Expand Down Expand Up @@ -207,7 +251,11 @@ pub enum Error {
},

#[snafu(display("Invalid sql value: {}", value))]
InvalidSqlValue { value: String },
InvalidSqlValue {
value: String,
#[snafu(implicit)]
location: Location,
},

#[snafu(display(
"Converting timestamp {:?} to unit {:?} overflow",
Expand All @@ -217,6 +265,8 @@ pub enum Error {
TimestampOverflow {
timestamp: Timestamp,
target_unit: TimeUnit,
#[snafu(implicit)]
location: Location,
},

#[snafu(display("Unable to convert statement {} to DataFusion statement", statement))]
Expand Down
8 changes: 1 addition & 7 deletions src/sql/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,6 @@ impl<'a> ParserContext<'a> {
self.parser
.parse_object_name(false)
.context(error::UnexpectedSnafu {
sql: self.sql,
expected: "a table name",
actual: self.parser.peek_token().to_string(),
})?;
Expand Down Expand Up @@ -171,7 +170,6 @@ impl<'a> ParserContext<'a> {

let database_name = self.parser.parse_identifier(false).context(
error::UnexpectedSnafu {
sql: self.sql,
expected: "a database name",
actual: self.peek_token_as_string(),
},
Expand Down Expand Up @@ -213,11 +211,7 @@ impl<'a> ParserContext<'a> {

/// Raises an "unsupported statement" error.
pub fn unsupported<T>(&self, keyword: String) -> Result<T> {
error::UnsupportedSnafu {
sql: self.sql,
keyword,
}
.fail()
error::UnsupportedSnafu { keyword }.fail()
}

// Report unexpected token
Expand Down
4 changes: 0 additions & 4 deletions src/sql/src/parsers/copy_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ impl<'a> ParserContext<'a> {
let database_name = self
.parse_object_name()
.with_context(|_| error::UnexpectedSnafu {
sql: self.sql,
expected: "a database name",
actual: self.peek_token_as_string(),
})?;
Expand Down Expand Up @@ -98,7 +97,6 @@ impl<'a> ParserContext<'a> {
let raw_table_name = self
.parse_object_name()
.with_context(|_| error::UnexpectedSnafu {
sql: self.sql,
expected: "a table name",
actual: self.peek_token_as_string(),
})?;
Expand Down Expand Up @@ -133,7 +131,6 @@ impl<'a> ParserContext<'a> {
self.parser
.parse_literal_string()
.with_context(|_| error::UnexpectedSnafu {
sql: self.sql,
expected: "a file name",
actual: self.peek_token_as_string(),
})?;
Expand Down Expand Up @@ -163,7 +160,6 @@ impl<'a> ParserContext<'a> {
self.parser
.parse_literal_uint()
.with_context(|_| error::UnexpectedSnafu {
sql: self.sql,
expected: "the number of maximum rows",
actual: self.peek_token_as_string(),
})?,
Expand Down
14 changes: 1 addition & 13 deletions src/sql/src/parsers/create_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,6 @@ impl<'a> ParserContext<'a> {
let _ = self.parser.next_token();
let if_not_exists = self.parse_if_not_exist()?;
let database_name = self.parse_object_name().context(error::UnexpectedSnafu {
sql: self.sql,
expected: "a database name",
actual: self.peek_token_as_string(),
})?;
Expand Down Expand Up @@ -350,18 +349,13 @@ impl<'a> ParserContext<'a> {
.expect_keyword(Keyword::EXISTS)
.map(|_| true)
.context(UnexpectedSnafu {
sql: self.sql,
expected: "EXISTS",
actual: self.peek_token_as_string(),
});
}

if self.parser.parse_keywords(&[Keyword::IF, Keyword::EXISTS]) {
return UnsupportedSnafu {
sql: self.sql,
keyword: "EXISTS",
}
.fail();
return UnsupportedSnafu { keyword: "EXISTS" }.fail();
}

Ok(false)
Expand Down Expand Up @@ -394,7 +388,6 @@ impl<'a> ParserContext<'a> {
self.parser
.expect_keywords(&[Keyword::ON, Keyword::COLUMNS])
.context(error::UnexpectedSnafu {
sql: self.sql,
expected: "ON, COLUMNS",
actual: self.peek_token_as_string(),
})?;
Expand Down Expand Up @@ -425,7 +418,6 @@ impl<'a> ParserContext<'a> {
self.parser
.expect_token(&Token::LParen)
.context(error::UnexpectedSnafu {
sql: self.sql,
expected: "(",
actual: self.peek_token_as_string(),
})?;
Expand All @@ -441,7 +433,6 @@ impl<'a> ParserContext<'a> {
self.parser
.expect_token(&Token::RParen)
.context(error::UnexpectedSnafu {
sql: self.sql,
expected: ")",
actual: self.peek_token_as_string(),
})?;
Expand Down Expand Up @@ -758,7 +749,6 @@ impl<'a> ParserContext<'a> {
self.parser
.expect_keyword(Keyword::KEY)
.context(error::UnexpectedSnafu {
sql: self.sql,
expected: "KEY",
actual: self.peek_token_as_string(),
})?;
Expand Down Expand Up @@ -786,7 +776,6 @@ impl<'a> ParserContext<'a> {
self.parser
.expect_keyword(Keyword::INDEX)
.context(error::UnexpectedSnafu {
sql: self.sql,
expected: "INDEX",
actual: self.peek_token_as_string(),
})?;
Expand Down Expand Up @@ -842,7 +831,6 @@ impl<'a> ParserContext<'a> {
self.parser
.expect_token(&Token::Eq)
.context(error::UnexpectedSnafu {
sql: self.sql,
expected: "=",
actual: self.peek_token_as_string(),
})?;
Expand Down
1 change: 0 additions & 1 deletion src/sql/src/parsers/delete_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ impl<'a> ParserContext<'a> {
Ok(Statement::Delete(Box::new(Delete { inner: spstatement })))
}
unexp => error::UnsupportedSnafu {
sql: self.sql.to_string(),
keyword: unexp.to_string(),
}
.fail(),
Expand Down
1 change: 0 additions & 1 deletion src/sql/src/parsers/describe_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ impl<'a> ParserContext<'a> {
let raw_table_idents =
self.parse_object_name()
.with_context(|_| error::UnexpectedSnafu {
sql: self.sql,
expected: "a table name",
actual: self.peek_token_as_string(),
})?;
Expand Down
4 changes: 0 additions & 4 deletions src/sql/src/parsers/drop_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ impl<'a> ParserContext<'a> {
let raw_view_ident = self
.parse_object_name()
.with_context(|_| error::UnexpectedSnafu {
sql: self.sql,
expected: "a view name",
actual: self.peek_token_as_string(),
})?;
Expand All @@ -75,7 +74,6 @@ impl<'a> ParserContext<'a> {
let raw_flow_ident = self
.parse_object_name()
.with_context(|_| error::UnexpectedSnafu {
sql: self.sql,
expected: "a flow name",
actual: self.peek_token_as_string(),
})?;
Expand All @@ -99,7 +97,6 @@ impl<'a> ParserContext<'a> {
let raw_table_ident =
self.parse_object_name()
.with_context(|_| error::UnexpectedSnafu {
sql: self.sql,
expected: "a table name",
actual: self.peek_token_as_string(),
})?;
Expand All @@ -126,7 +123,6 @@ impl<'a> ParserContext<'a> {
let database_name = self
.parse_object_name()
.with_context(|_| error::UnexpectedSnafu {
sql: self.sql,
expected: "a database name",
actual: self.peek_token_as_string(),
})?;
Expand Down
1 change: 0 additions & 1 deletion src/sql/src/parsers/explain_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ impl<'a> ParserContext<'a> {
.parser
.parse_explain(DescribeAlias::Explain)
.with_context(|_| error::UnexpectedSnafu {
sql: self.sql,
expected: "a query statement",
actual: self.peek_token_as_string(),
})?;
Expand Down
1 change: 0 additions & 1 deletion src/sql/src/parsers/insert_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ impl<'a> ParserContext<'a> {
Ok(Statement::Insert(Box::new(Insert { inner: spstatement })))
}
unexp => error::UnsupportedSnafu {
sql: self.sql.to_string(),
keyword: unexp.to_string(),
}
.fail(),
Expand Down
1 change: 0 additions & 1 deletion src/sql/src/parsers/set_var_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ impl<'a> ParserContext<'a> {
})),

unexp => error::UnsupportedSnafu {
sql: self.sql.to_string(),
keyword: unexp.to_string(),
}
.fail(),
Expand Down
Loading

0 comments on commit 6e1d4a3

Please sign in to comment.