alexreisner/geocoder — pull request #719 by lipanski (2014-09-20 22:36:00+00:00)
Title: refactor MaxmindLocal lookup

We were trying to use this in a production system when we encountered a big performance issue. I tracked down the main cause: calling `ActiveRecord::Base.connection.execute(query).first` will query the entire table, then produce the `first` result of the results array. This was probably not the intention of the person who wrote this and by calling `first` on the actual query (as in `LIMIT 1`) response time can be reduced from a constant 400-500ms to something around 10-50ms (depending on the position of the looked up IP address in the index).

Further improvements include enabling caching (if configured). Even if this query runs on the local database, it's still a pretty huge table, especially if you're using `request.location` on every request.

My intention was to get rid of the SQL in there, in favour of proper ActiveRecord queries. The `ActiveRecord::Base.connection.execute` call made me think that ActiveRecord is a dependency here so I could make use of it. However I stumbled upon a problem while running the test suite, that made me discover that ActiveRecord is actually mocked in the tests. So I tried my own mocking, maybe you can suggest something else.

The database layout is not very ActiveRecord-ish (e.g. I had to use `table_name=` because of the singular), but I figured that renaming the tables and columns would be a breaking change. Anyhow, I think it looks better with the Models in there and with proper relationships.

Let me know what you think :squirrel: