Skip to content

Commit 3f98455

Browse files
jjbayerjjbayer
authored
chore(spans): Upgrade sqlparser (#3057)
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.
1 parent 314e4ed commit 3f98455

File tree

5 files changed

+127
-95
lines changed

5 files changed

+127
-95
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
- Drop spans ending outside the valid timestamp range. ([#3013](https://github.com/getsentry/relay/pull/3013))
1717
- Extract INP metrics from spans. ([#2969](https://github.com/getsentry/relay/pull/2969), [#3041](https://github.com/getsentry/relay/pull/3041))
1818
- Add ability to rate limit metric buckets by namespace. ([#2941](https://github.com/getsentry/relay/pull/2941))
19+
- Upgrade sqlparser to 0.43.1.([#3057](https://github.com/getsentry/relay/pull/3057))
1920

2021
## 24.1.1
2122

‎Cargo.lock

Lines changed: 7 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎relay-event-normalization/Cargo.toml

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,7 @@ serde = { workspace = true }
3030
serde_json = { workspace = true }
3131
serde_urlencoded = "0.7.1"
3232
smallvec = { workspace = true }
33-
# Use a fork of sqlparser until https://github.com/sqlparser-rs/sqlparser-rs/pull/1065 gets merged:
34-
sqlparser = { git = "https://github.com/getsentry/sqlparser-rs.git", rev = "0bb7ec6ce661ecfc468f647fd1003b7839d37fa6", features = [
35-
"visitor",
36-
] }
33+
sqlparser = { version = "0.43.1", features = ["visitor"] }
3734
thiserror = { workspace = true }
3835
url = { workspace = true }
3936
uuid = { workspace = true }

‎relay-event-normalization/src/normalize/span/description/sql/mod.rs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -383,7 +383,7 @@ mod tests {
383383
scrub_sql_test!(
384384
strip_prefixes_mysql_generic,
385385
r#"SELECT `table`.`foo`, count(*) from `table` WHERE sku = %s"#,
386-
r#"SELECT foo, count(*) from table WHERE sku = %s"#
386+
r#"SELECT foo, count(*) FROM table WHERE sku = %s"#
387387
);
388388

389389
scrub_sql_test_with_dialect!(
@@ -581,7 +581,7 @@ mod tests {
581581
scrub_sql_test!(
582582
values_multi,
583583
"INSERT INTO a (b, c, d, e) VALuES (%s, %s, %s, %s), (%s, %s, %s, %s), (%s, %s, %s, %s) ON CONFLICT DO NOTHING",
584-
"INSERT INTO a (..) VALUES (%s) ON CONFLICT DO NOTHING"
584+
"INSERT INTO a (..) VALUES (%s) ON CONFLICT DO NOTHING"
585585
);
586586

587587
scrub_sql_test!(
@@ -876,13 +876,10 @@ mod tests {
876876
);
877877

878878
scrub_sql_test_with_dialect!(
879-
dont_fallback_to_regex,
879+
replace_into,
880880
"mysql",
881-
// sqlparser cannot parse REPLACE INTO. If we know that
882-
// a query is MySQL, we should give up rather than try to scrub
883-
// with regex
884881
r#"REPLACE INTO `foo` (`a`) VALUES ("abcd1234")"#,
885-
"REPLACE INTO foo (a) VALUES (%s)"
882+
"REPLACE INTO foo (..) VALUES (%s)"
886883
);
887884

888885
scrub_sql_test!(

‎relay-event-normalization/src/normalize/span/description/sql/parser.rs

Lines changed: 114 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ use itertools::Itertools;
66
use once_cell::sync::Lazy;
77
use regex::Regex;
88
use sqlparser::ast::{
9-
AlterTableOperation, Assignment, BinaryOperator, CloseCursor, ColumnDef, Expr, Ident,
10-
ObjectName, Query, Select, SelectItem, SetExpr, Statement, TableAlias, TableConstraint,
9+
AlterTableOperation, Assignment, BinaryOperator, CloseCursor, ColumnDef, Expr, FunctionArg,
10+
Ident, ObjectName, Query, Select, SelectItem, SetExpr, Statement, TableAlias, TableConstraint,
1111
TableFactor, UnaryOperator, Value, VisitMut, VisitorMut,
1212
};
1313
use sqlparser::dialect::{Dialect, GenericDialect};
@@ -198,12 +198,41 @@ impl VisitorMut for NormalizeVisitor {
198198
Self::simplify_table_alias(alias);
199199
}
200200
TableFactor::Pivot {
201-
table_alias,
202-
pivot_alias,
201+
value_column,
202+
alias,
203+
..
204+
} => {
205+
Self::simplify_compound_identifier(value_column);
206+
Self::simplify_table_alias(alias);
207+
}
208+
TableFactor::Function {
209+
name, args, alias, ..
210+
} => {
211+
Self::simplify_compound_identifier(&mut name.0);
212+
for arg in args {
213+
if let FunctionArg::Named { name, .. } = arg {
214+
Self::scrub_name(name);
215+
}
216+
}
217+
Self::simplify_table_alias(alias);
218+
}
219+
TableFactor::JsonTable { columns, alias, .. } => {
220+
for column in columns {
221+
Self::scrub_name(&mut column.name);
222+
}
223+
Self::simplify_table_alias(alias);
224+
}
225+
TableFactor::Unpivot {
226+
value,
227+
name,
228+
columns,
229+
alias,
203230
..
204231
} => {
205-
Self::simplify_table_alias(table_alias);
206-
Self::simplify_table_alias(pivot_alias);
232+
Self::scrub_name(value);
233+
Self::scrub_name(name);
234+
Self::simplify_compound_identifier(columns);
235+
Self::simplify_table_alias(alias);
207236
}
208237
}
209238
ControlFlow::Continue(())
@@ -311,8 +340,10 @@ impl VisitorMut for NormalizeVisitor {
311340
columns, source, ..
312341
} => {
313342
*columns = vec![Self::ellipsis()];
314-
if let SetExpr::Values(v) = &mut *source.body {
315-
v.rows = vec![vec![Expr::Value(Self::placeholder())]]
343+
if let Some(source) = source.as_mut() {
344+
if let SetExpr::Values(v) = &mut *source.body {
345+
v.rows = vec![vec![Expr::Value(Self::placeholder())]]
346+
}
316347
}
317348
}
318349
// Simple lists of col = value assignments are collapsed to `..`.
@@ -350,89 +381,93 @@ impl VisitorMut for NormalizeVisitor {
350381
Statement::Close {
351382
cursor: CloseCursor::Specific { name },
352383
} => Self::erase_name(name),
353-
Statement::AlterTable { name, operation } => {
384+
Statement::AlterTable {
385+
name, operations, ..
386+
} => {
354387
Self::simplify_compound_identifier(&mut name.0);
355-
match operation {
356-
AlterTableOperation::AddConstraint(c) => match c {
357-
TableConstraint::Unique { name, columns, .. } => {
358-
if let Some(name) = name {
359-
Self::scrub_name(name);
388+
for operation in operations {
389+
match operation {
390+
AlterTableOperation::AddConstraint(c) => match c {
391+
TableConstraint::Unique { name, columns, .. } => {
392+
if let Some(name) = name {
393+
Self::scrub_name(name);
394+
}
395+
for column in columns {
396+
Self::scrub_name(column);
397+
}
360398
}
361-
for column in columns {
362-
Self::scrub_name(column);
399+
TableConstraint::ForeignKey {
400+
name,
401+
columns,
402+
referred_columns,
403+
..
404+
} => {
405+
if let Some(name) = name {
406+
Self::scrub_name(name);
407+
}
408+
for column in columns {
409+
Self::scrub_name(column);
410+
}
411+
for column in referred_columns {
412+
Self::scrub_name(column);
413+
}
363414
}
364-
}
365-
TableConstraint::ForeignKey {
366-
name,
367-
columns,
368-
referred_columns,
369-
..
370-
} => {
371-
if let Some(name) = name {
372-
Self::scrub_name(name);
415+
TableConstraint::Check { name, .. } => {
416+
if let Some(name) = name {
417+
Self::scrub_name(name);
418+
}
373419
}
374-
for column in columns {
375-
Self::scrub_name(column);
420+
TableConstraint::Index { name, columns, .. } => {
421+
if let Some(name) = name {
422+
Self::scrub_name(name);
423+
}
424+
for column in columns {
425+
Self::scrub_name(column);
426+
}
376427
}
377-
for column in referred_columns {
378-
Self::scrub_name(column);
428+
TableConstraint::FulltextOrSpatial {
429+
opt_index_name,
430+
columns,
431+
..
432+
} => {
433+
if let Some(name) = opt_index_name {
434+
Self::scrub_name(name);
435+
}
436+
for column in columns {
437+
Self::scrub_name(column);
438+
}
379439
}
440+
},
441+
AlterTableOperation::AddColumn { column_def, .. } => {
442+
let ColumnDef { name, .. } = column_def;
443+
Self::scrub_name(name);
380444
}
381-
TableConstraint::Check { name, .. } => {
382-
if let Some(name) = name {
383-
Self::scrub_name(name);
384-
}
445+
AlterTableOperation::DropConstraint { name, .. } => Self::scrub_name(name),
446+
AlterTableOperation::DropColumn { column_name, .. } => {
447+
Self::scrub_name(column_name)
385448
}
386-
TableConstraint::Index { name, columns, .. } => {
387-
if let Some(name) = name {
388-
Self::scrub_name(name);
389-
}
390-
for column in columns {
391-
Self::scrub_name(column);
392-
}
449+
AlterTableOperation::RenameColumn {
450+
old_column_name,
451+
new_column_name,
452+
} => {
453+
Self::scrub_name(old_column_name);
454+
Self::scrub_name(new_column_name);
393455
}
394-
TableConstraint::FulltextOrSpatial {
395-
opt_index_name,
396-
columns,
397-
..
456+
AlterTableOperation::ChangeColumn {
457+
old_name, new_name, ..
398458
} => {
399-
if let Some(name) = opt_index_name {
400-
Self::scrub_name(name);
401-
}
402-
for column in columns {
403-
Self::scrub_name(column);
404-
}
459+
Self::scrub_name(old_name);
460+
Self::scrub_name(new_name);
405461
}
406-
},
407-
AlterTableOperation::AddColumn { column_def, .. } => {
408-
let ColumnDef { name, .. } = column_def;
409-
Self::scrub_name(name);
410-
}
411-
AlterTableOperation::DropConstraint { name, .. } => Self::scrub_name(name),
412-
AlterTableOperation::DropColumn { column_name, .. } => {
413-
Self::scrub_name(column_name)
414-
}
415-
AlterTableOperation::RenameColumn {
416-
old_column_name,
417-
new_column_name,
418-
} => {
419-
Self::scrub_name(old_column_name);
420-
Self::scrub_name(new_column_name);
421-
}
422-
AlterTableOperation::ChangeColumn {
423-
old_name, new_name, ..
424-
} => {
425-
Self::scrub_name(old_name);
426-
Self::scrub_name(new_name);
427-
}
428-
AlterTableOperation::RenameConstraint { old_name, new_name } => {
429-
Self::scrub_name(old_name);
430-
Self::scrub_name(new_name);
431-
}
432-
AlterTableOperation::AlterColumn { column_name, .. } => {
433-
Self::scrub_name(column_name);
462+
AlterTableOperation::RenameConstraint { old_name, new_name } => {
463+
Self::scrub_name(old_name);
464+
Self::scrub_name(new_name);
465+
}
466+
AlterTableOperation::AlterColumn { column_name, .. } => {
467+
Self::scrub_name(column_name);
468+
}
469+
_ => {}
434470
}
435-
_ => {}
436471
}
437472
}
438473

0 commit comments

Comments
 (0)