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.

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

Negative numbers in SQL queries aren't properly normalized #67

Closed
pvande opened this issue Jul 1, 2016 · 3 comments
Closed

Negative numbers in SQL queries aren't properly normalized #67

pvande opened this issue Jul 1, 2016 · 3 comments

Comments

@pvande
Copy link

pvande commented Jul 1, 2016

This implies that if an unknown number is inlined into an SQL query, multiple traces may be generated for the same source query.

Failing test case:

diff --git a/spec/unit/normalizers/active_record_spec.rb b/spec/unit/normalizers/active_record_spec.rb
index 063a34e..5513750 100644
--- a/spec/unit/normalizers/active_record_spec.rb
+++ b/spec/unit/normalizers/active_record_spec.rb
@@ -65,6 +65,15 @@ module Skylight
       expect(desc).to eq("select * from foo where id = ?")
     end

+    it "Handles negative numbers in embedded binds" do
+      name, title, desc =
+        normalize(name: "Foo Load", sql: "select * from foo where id = -1")
+
+      expect(name).to eq("db.sql.query")
+      expect(title).to eq("SELECT FROM foo")
+      expect(desc).to eq("select * from foo where id = ?")
+    end
+
     it "handles some precomputed binds" do
       sql = %{INSERT INTO "agent_errors" ("body", "created_at", "value", "hostname", "reason") VALUES ($1, $2, NULL, $3, $4) RETURNING "id"}
       extracted = %{INSERT INTO "agent_errors" ("body", "created_at", "value", "hostname", "reason") VALUES (?, ?, ?, ?, ?) RETURNING "id"}
@wagenet
Copy link
Contributor

wagenet commented Jul 1, 2016

Thanks for reporting this. It's a known issue on our end, though this is the first time anyone else has noticed it. Is it causing problem for you?

@pvande
Copy link
Author

pvande commented Jul 1, 2016

Unfortunately, it is, though not a significant one.

@wagenet
Copy link
Contributor

wagenet commented Sep 28, 2017

This is fixed in the new SQL lexer that we're hoping to roll out soon.

@wagenet wagenet closed this as completed Sep 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants