Skip to content

Allow the connection pool's #table_exists? method to give the connections #1636

Merged
merged 1 commit into from Jun 17, 2011

3 participants

@metaskills

The use case for the default abstract adapter's #table_exists? method is to give adapters a chance to return true in special cases where the core #tables does not include a name. Common use cases are for views and tables in other schemas. The connection pool should give a connection to respond true to with this matching name. If this was not done, I get errors in the SQL Server Adapter where column objects are not cached and respond properly for either the entire table or specific attribute of columns for said table, like @primary. This fixes that.

I did not include a test because this looked to be too low level white-box testing and I could not see any parallel in the connection_pool_test.rb file All tests for MySQL, PostgreSQL, and SQLite3 still pass with this change.

@tenderlove
Ruby on Rails member

@rsim just pinged me today about this for the oracle adapter. Though his suggestion was to just delegate to the connection all the time.

wdyt?

@metaskills

That would be fine! The method seemed to possibly optimize multiple #with_connection blocks by taking a first stab thru the #tables and just filling in true. I did not want to get into assuming and measuring if that was a big deal or not. My guess is my patch of just adding an extra line would be more considerate to performance vs asking #table_exists? for each table that would be in #tables. So if a DB has 200 tables, that would potentially be 200 SQL calls as compared to the single one SQL call the method optimized for.

The more I think about it, the better I like what it does and the one line addition I did. Your call tho.

@rsim
rsim commented Jun 10, 2011

I think this proposed solution is fine for me as well. We had the same issue that Oracle database view based models did not cache view columns - this patch solves this problem.

@metaskills

Aaron, will this make it in the release?

@tenderlove tenderlove merged commit ba1b88f into rails:master Jun 17, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.