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

performance issue with SQL Adapter #553

Closed
sschwing3 opened this issue Jun 23, 2015 · 5 comments
Closed

performance issue with SQL Adapter #553

sschwing3 opened this issue Jun 23, 2015 · 5 comments
Assignees

Comments

@sschwing3
Copy link

I had a performance problem when i used ransack (1.6.6) with rails 4.2 and activerecord-sqlserver-adapter (4.2.5 b917f4b)

When i put executed this query in rails console
Todo.joins(:user).ransack({"user_id_eq"=>"11"}).result

the sql server got the following queries

EXEC sp_executesql N'SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_TYPE = ''BASE TABLE'' ORDER BY TABLE_NAME' go
EXEC sp_executesql N'SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_TYPE = ''BASE TABLE'' ORDER BY TABLE_NAME' go
EXEC sp_executesql N'SELECT TABLE_NAME FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_TYPE = ''BASE TABLE'' ORDER BY TABLE_NAME' go
EXEC sp_executesql N'SELECT [todos].* FROM [todos] INNER JOIN [users] ON [users].[id] = [todos].[user_id] WHERE [todos].[user_id] = 11' go

In the config, I enabled schema cache dump (config.active_record.use_schema_cache_dump = true) but nothing happend.

I took a look on the code and found the line, which is responsible for the table name look up:

lib/ransack/adapters/active_record/context.rb: at line 28:
28: unless connection.table_exists?(table)
I changed it to connection.schema_cache.table_exists?(table) and the table name look up stopped.

My Question: Is this performance fix correct or do i get some problems with it?

@jonatack
Copy link
Contributor

Thank you @sschwing3 for this report. You may have found a regression caused by this commit.

Would you like to make a pull request for a fix, with a test spec and update to the changelog?

@jonatack jonatack self-assigned this Aug 20, 2015
@zhangsoledad
Copy link

1d245bc8-31c9-4657-b3e0-37fb455d5b0a

it's terrible, one query , execute so many time `table_exists`

@jonatack
Copy link
Contributor

@zhangsoledad I have never used SQL Server. You are free to make a pull request to fix it 😸

@sschwing3
Copy link
Author

hi guys,

my code at the top fixed the problem for me, but i don't know how to write a test spec, that's why I didn't pulled my solution.

@zhangsoledadhttps://github.com/zhangsoledad

Am 11.09.2015 um 11:36 schrieb Jon Atack <notifications@github.commailto:notifications@github.com>:

@zhangsoledadhttps://github.com/zhangsoledad I have never used SQL Server. Feel free to make a pull request to fix it instead of complaining [:smile_cat:]


Reply to this email directly or view it on GitHubhttps://github.com//issues/553#issuecomment-139500322.

@jonatack
Copy link
Contributor

Hi @sschwing3 I think it's safe to use your fix because it reverts back to the original implementation. Please let me know if you'd like to make a PR.

jonatack added a commit that referenced this issue Sep 11, 2015
Credit @sschwing3 for reporting and fixing #553.
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

3 participants