Skip to content

Conversation

@cgitr
Copy link

@cgitr cgitr commented Jun 14, 2012

our application has specific queries, which don't have any "@XX" placeholders. The current explain codes drops part of the query string after the last comma, which results in a very invalid query or even exceptions.

The patch only removes the last segment if there are placeholders in the sql.

I'm not sure if this works for every case but it does for our application.

@metaskills
Copy link
Member

Would you be interested in writing a test? There is a file called showplan_test_sqlserver.rb which we add these too.

@cgitr
Copy link
Author

cgitr commented Jun 14, 2012

I was not able to run the test suite, due to several errors (including missing files etc.).
Right now, I can not spend more time on this topic.

I could send the problematic query to you, if necessary.

@metaskills
Copy link
Member

That would help. BTW, I recently setup the master repo so that all you would have to do us bundle install and everything would be setup to run the tests (minus creating the two DBs and users). That aside, the query would help.

@cgitr
Copy link
Author

cgitr commented Jun 14, 2012

The input sql was...

SELECT TOP (1) [comments].* FROM [comments] WHERE [comments].[id] = 382066 AND [comments].[author_id] = 37081 AND (comments.okay=1 AND ( comments.date IS NULL
 OR comments.date = CONVERT(DATETIME, ''01.01.1900'') OR
 comments.date >= GETDATE()))

the resulting sql was...

SELECT TOP (1) [comments].* FROM [comments] WHERE [comments].[id] = 382066 AND [comments].[author_id] = 37081 AND (comments.okay=1 AND ( comments.date IS NULL
 OR comments.date = CONVERT(DATETIME

Thanks!

@cgitr cgitr closed this Jun 14, 2012
@cgitr cgitr reopened this Jun 14, 2012
@cgitr
Copy link
Author

cgitr commented Jun 14, 2012

I recently setup the master repo so that all you would have to do us bundle install and everything would be setup to run the tests

Ah okay, great! Gave it one more try... came up to the point where

creating the two DBs and users

would help.

Setting these up isn't as easy in our infrastructure, so sorry!
I'll organize that for future requests!

Thanks a lot!

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

Successfully merging this pull request may close these issues.

2 participants