Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "chore(spans): Upgrade sqlparser" #3059

Merged
merged 1 commit into from
Feb 6, 2024
Merged

Revert "chore(spans): Upgrade sqlparser" #3059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Changelog

## Unreleased

Check failure on line 3 in CHANGELOG.md

View workflow job for this annotation

GitHub Actions / Changelogs

Missing changelog entry. 

Please consider adding a changelog entry for the next release.

**Features**:

Expand All @@ -16,7 +16,6 @@
- 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: 5 additions & 7 deletions Cargo.lock
View file
Open in desktop

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

5 changes: 4 additions & 1 deletion relay-event-normalization/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ serde = { workspace = true }
serde_json = { workspace = true }
serde_urlencoded = "0.7.1"
smallvec = { workspace = true }
sqlparser = { version = "0.43.1", features = ["visitor"] }
# 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",
] }
thiserror = { workspace = true }
url = { workspace = true }
uuid = { workspace = true }
Expand Down
11 changes: 7 additions & 4 deletions relay-event-normalization/src/normalize/span/description/sql/mod.rs
View file
Open in desktop
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,10 +876,13 @@ mod tests {
);

scrub_sql_test_with_dialect!(
replace_into,
dont_fallback_to_regex,
"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 (..) VALUES (%s)"
"REPLACE INTO foo (a) VALUES (%s)"
);

scrub_sql_test!(
Expand Down
193 changes: 79 additions & 114 deletions relay-event-normalization/src/normalize/span/description/sql/parser.rs
View file
Open in desktop
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, FunctionArg,
Ident, ObjectName, Query, Select, SelectItem, SetExpr, Statement, TableAlias, TableConstraint,
AlterTableOperation, Assignment, BinaryOperator, CloseCursor, ColumnDef, Expr, Ident,
ObjectName, Query, Select, SelectItem, SetExpr, Statement, TableAlias, TableConstraint,
TableFactor, UnaryOperator, Value, VisitMut, VisitorMut,
};
use sqlparser::dialect::{Dialect, GenericDialect};
Expand Down Expand Up @@ -198,41 +198,12 @@ impl VisitorMut for NormalizeVisitor {
Self::simplify_table_alias(alias);
}
TableFactor::Pivot {
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,
table_alias,
pivot_alias,
..
} => {
Self::scrub_name(value);
Self::scrub_name(name);
Self::simplify_compound_identifier(columns);
Self::simplify_table_alias(alias);
Self::simplify_table_alias(table_alias);
Self::simplify_table_alias(pivot_alias);
}
}
ControlFlow::Continue(())
Expand Down Expand Up @@ -340,10 +311,8 @@ impl VisitorMut for NormalizeVisitor {
columns, source, ..
} => {
*columns = vec![Self::ellipsis()];
if let Some(source) = source.as_mut() {
if let SetExpr::Values(v) = &mut *source.body {
v.rows = vec![vec![Expr::Value(Self::placeholder())]]
}
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 @@ -381,93 +350,89 @@ impl VisitorMut for NormalizeVisitor {
Statement::Close {
cursor: CloseCursor::Specific { name },
} => Self::erase_name(name),
Statement::AlterTable {
name, operations, ..
} => {
Statement::AlterTable { name, operation } => {
Self::simplify_compound_identifier(&mut name.0);
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);
}
match operation {
AlterTableOperation::AddConstraint(c) => match c {
TableConstraint::Unique { name, columns, .. } => {
if let Some(name) = name {
Self::scrub_name(name);
}
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);
}
for column in columns {
Self::scrub_name(column);
}
TableConstraint::Check { name, .. } => {
if let Some(name) = name {
Self::scrub_name(name);
}
}
TableConstraint::ForeignKey {
name,
columns,
referred_columns,
..
} => {
if let Some(name) = name {
Self::scrub_name(name);
}
TableConstraint::Index { 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::FulltextOrSpatial {
opt_index_name,
columns,
..
} => {
if let Some(name) = opt_index_name {
Self::scrub_name(name);
}
for column in columns {
Self::scrub_name(column);
}
for column in referred_columns {
Self::scrub_name(column);
}
},
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)
TableConstraint::Check { name, .. } => {
if let Some(name) = name {
Self::scrub_name(name);
}
}
AlterTableOperation::RenameColumn {
old_column_name,
new_column_name,
} => {
Self::scrub_name(old_column_name);
Self::scrub_name(new_column_name);
TableConstraint::Index { name, columns, .. } => {
if let Some(name) = name {
Self::scrub_name(name);
}
for column in columns {
Self::scrub_name(column);
}
}
AlterTableOperation::ChangeColumn {
old_name, new_name, ..
TableConstraint::FulltextOrSpatial {
opt_index_name,
columns,
..
} => {
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);
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);
}
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);
}
_ => {}
}
}

Expand Down
Loading