Skip to content

Commit

Permalink
chore(spans): Upgrade sqlparser (#3057)
Browse files Browse the repository at this point in the history 
The `sqlparser` lib has been on a fork since
#2846. We can now go back to the
official version because
apache/datafusion-sqlparser-rs#1065 has been merged &
released.
  • Loading branch information
@jjbayer
jjbayer authored Feb 6, 2024
1 parent 314e4ed commit 3f98455
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 95 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
- Drop spans ending outside the valid timestamp range. ([#3013](https://github.com/getsentry/relay/pull/3013))
- Extract INP metrics from spans. ([#2969](https://github.com/getsentry/relay/pull/2969), [#3041](https://github.com/getsentry/relay/pull/3041))
- Add ability to rate limit metric buckets by namespace. ([#2941](https://github.com/getsentry/relay/pull/2941))
- Upgrade sqlparser to 0.43.1.([#3057](https://github.com/getsentry/relay/pull/3057))

## 24.1.1

Expand Down
12 changes: 7 additions & 5 deletions Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 1 addition & 4 deletions relay-event-normalization/Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,7 @@ serde = { workspace = true }
serde_json = { workspace = true }
serde_urlencoded = "0.7.1"
smallvec = { workspace = true }
# Use a fork of sqlparser until https://github.com/sqlparser-rs/sqlparser-rs/pull/1065 gets merged:
sqlparser = { git = "https://github.com/getsentry/sqlparser-rs.git", rev = "0bb7ec6ce661ecfc468f647fd1003b7839d37fa6", features = [
"visitor",
] }
sqlparser = { version = "0.43.1", features = ["visitor"] }
thiserror = { workspace = true }
url = { workspace = true }
uuid = { workspace = true }
Expand Down
11 changes: 4 additions & 7 deletions relay-event-normalization/src/normalize/span/description/sql/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,7 @@ mod tests {
scrub_sql_test!(
strip_prefixes_mysql_generic,
r#"SELECT `table`.`foo`, count(*) from `table` WHERE sku = %s"#,
r#"SELECT foo, count(*) from table WHERE sku = %s"#
r#"SELECT foo, count(*) FROM table WHERE sku = %s"#
);

scrub_sql_test_with_dialect!(
Expand Down Expand Up @@ -581,7 +581,7 @@ mod tests {
scrub_sql_test!(
values_multi,
"INSERT INTO a (b, c, d, e) VALuES (%s, %s, %s, %s), (%s, %s, %s, %s), (%s, %s, %s, %s) ON CONFLICT DO NOTHING",
"INSERT INTO a (..) VALUES (%s) ON CONFLICT DO NOTHING"
"INSERT INTO a (..) VALUES (%s) ON CONFLICT DO NOTHING"
);

scrub_sql_test!(
Expand Down Expand Up @@ -876,13 +876,10 @@ mod tests {
);

scrub_sql_test_with_dialect!(
dont_fallback_to_regex,
replace_into,
"mysql",
// sqlparser cannot parse REPLACE INTO. If we know that
// a query is MySQL, we should give up rather than try to scrub
// with regex
r#"REPLACE INTO `foo` (`a`) VALUES ("abcd1234")"#,
"REPLACE INTO foo (a) VALUES (%s)"
"REPLACE INTO foo (..) VALUES (%s)"
);

scrub_sql_test!(
Expand Down
193 changes: 114 additions & 79 deletions relay-event-normalization/src/normalize/span/description/sql/parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ use itertools::Itertools;
use once_cell::sync::Lazy;
use regex::Regex;
use sqlparser::ast::{
AlterTableOperation, Assignment, BinaryOperator, CloseCursor, ColumnDef, Expr, Ident,
ObjectName, Query, Select, SelectItem, SetExpr, Statement, TableAlias, TableConstraint,
AlterTableOperation, Assignment, BinaryOperator, CloseCursor, ColumnDef, Expr, FunctionArg,
Ident, ObjectName, Query, Select, SelectItem, SetExpr, Statement, TableAlias, TableConstraint,
TableFactor, UnaryOperator, Value, VisitMut, VisitorMut,
};
use sqlparser::dialect::{Dialect, GenericDialect};
Expand Down Expand Up @@ -198,12 +198,41 @@ impl VisitorMut for NormalizeVisitor {
Self::simplify_table_alias(alias);
}
TableFactor::Pivot {
table_alias,
pivot_alias,
value_column,
alias,
..
} => {
Self::simplify_compound_identifier(value_column);
Self::simplify_table_alias(alias);
}
TableFactor::Function {
name, args, alias, ..
} => {
Self::simplify_compound_identifier(&mut name.0);
for arg in args {
if let FunctionArg::Named { name, .. } = arg {
Self::scrub_name(name);
}
}
Self::simplify_table_alias(alias);
}
TableFactor::JsonTable { columns, alias, .. } => {
for column in columns {
Self::scrub_name(&mut column.name);
}
Self::simplify_table_alias(alias);
}
TableFactor::Unpivot {
value,
name,
columns,
alias,
..
} => {
Self::simplify_table_alias(table_alias);
Self::simplify_table_alias(pivot_alias);
Self::scrub_name(value);
Self::scrub_name(name);
Self::simplify_compound_identifier(columns);
Self::simplify_table_alias(alias);
}
}
ControlFlow::Continue(())
Expand Down Expand Up @@ -311,8 +340,10 @@ impl VisitorMut for NormalizeVisitor {
columns, source, ..
} => {
*columns = vec![Self::ellipsis()];
if let SetExpr::Values(v) = &mut *source.body {
v.rows = vec![vec![Expr::Value(Self::placeholder())]]
if let Some(source) = source.as_mut() {
if let SetExpr::Values(v) = &mut *source.body {
v.rows = vec![vec![Expr::Value(Self::placeholder())]]
}
}
}
// Simple lists of col = value assignments are collapsed to `..`.
Expand Down Expand Up @@ -350,89 +381,93 @@ impl VisitorMut for NormalizeVisitor {
Statement::Close {
cursor: CloseCursor::Specific { name },
} => Self::erase_name(name),
Statement::AlterTable { name, operation } => {
Statement::AlterTable {
name, operations, ..
} => {
Self::simplify_compound_identifier(&mut name.0);
match operation {
AlterTableOperation::AddConstraint(c) => match c {
TableConstraint::Unique { name, columns, .. } => {
if let Some(name) = name {
Self::scrub_name(name);
for operation in operations {
match operation {
AlterTableOperation::AddConstraint(c) => match c {
TableConstraint::Unique { name, columns, .. } => {
if let Some(name) = name {
Self::scrub_name(name);
}
for column in columns {
Self::scrub_name(column);
}
}
for column in columns {
Self::scrub_name(column);
TableConstraint::ForeignKey {
name,
columns,
referred_columns,
..
} => {
if let Some(name) = name {
Self::scrub_name(name);
}
for column in columns {
Self::scrub_name(column);
}
for column in referred_columns {
Self::scrub_name(column);
}
}
}
TableConstraint::ForeignKey {
name,
columns,
referred_columns,
..
} => {
if let Some(name) = name {
Self::scrub_name(name);
TableConstraint::Check { name, .. } => {
if let Some(name) = name {
Self::scrub_name(name);
}
}
for column in columns {
Self::scrub_name(column);
TableConstraint::Index { name, columns, .. } => {
if let Some(name) = name {
Self::scrub_name(name);
}
for column in columns {
Self::scrub_name(column);
}
}
for column in referred_columns {
Self::scrub_name(column);
TableConstraint::FulltextOrSpatial {
opt_index_name,
columns,
..
} => {
if let Some(name) = opt_index_name {
Self::scrub_name(name);
}
for column in columns {
Self::scrub_name(column);
}
}
},
AlterTableOperation::AddColumn { column_def, .. } => {
let ColumnDef { name, .. } = column_def;
Self::scrub_name(name);
}
TableConstraint::Check { name, .. } => {
if let Some(name) = name {
Self::scrub_name(name);
}
AlterTableOperation::DropConstraint { name, .. } => Self::scrub_name(name),
AlterTableOperation::DropColumn { column_name, .. } => {
Self::scrub_name(column_name)
}
TableConstraint::Index { name, columns, .. } => {
if let Some(name) = name {
Self::scrub_name(name);
}
for column in columns {
Self::scrub_name(column);
}
AlterTableOperation::RenameColumn {
old_column_name,
new_column_name,
} => {
Self::scrub_name(old_column_name);
Self::scrub_name(new_column_name);
}
TableConstraint::FulltextOrSpatial {
opt_index_name,
columns,
..
AlterTableOperation::ChangeColumn {
old_name, new_name, ..
} => {
if let Some(name) = opt_index_name {
Self::scrub_name(name);
}
for column in columns {
Self::scrub_name(column);
}
Self::scrub_name(old_name);
Self::scrub_name(new_name);
}
},
AlterTableOperation::AddColumn { column_def, .. } => {
let ColumnDef { name, .. } = column_def;
Self::scrub_name(name);
}
AlterTableOperation::DropConstraint { name, .. } => Self::scrub_name(name),
AlterTableOperation::DropColumn { column_name, .. } => {
Self::scrub_name(column_name)
}
AlterTableOperation::RenameColumn {
old_column_name,
new_column_name,
} => {
Self::scrub_name(old_column_name);
Self::scrub_name(new_column_name);
}
AlterTableOperation::ChangeColumn {
old_name, new_name, ..
} => {
Self::scrub_name(old_name);
Self::scrub_name(new_name);
}
AlterTableOperation::RenameConstraint { old_name, new_name } => {
Self::scrub_name(old_name);
Self::scrub_name(new_name);
}
AlterTableOperation::AlterColumn { column_name, .. } => {
Self::scrub_name(column_name);
AlterTableOperation::RenameConstraint { old_name, new_name } => {
Self::scrub_name(old_name);
Self::scrub_name(new_name);
}
AlterTableOperation::AlterColumn { column_name, .. } => {
Self::scrub_name(column_name);
}
_ => {}
}
_ => {}
}
}

Expand Down

0 comments on commit 3f98455

Please sign in to comment.