geokit / geokit-rails

Official Geokit plugin for Rails/ActiveRecord. Provides location-based goodness for your Rails app. Requires the Geokit gem.
MIT License
1.57k stars 245 forks source link

Add OracleEnhanced adapter #62

Closed paulswartz closed 9 years ago

paulswartz commented 10 years ago

Fixes #22 as well, perhaps?

mnoack commented 10 years ago

@paulswartz I don't have oracle and doing some reasearch shows it's not on Travis due to licensing issues so I can't easily test (without probably a bit of downloading and work) so any assistance will be great in terms of getting a test oracle system up and running without a license.

From some quick research this is perhaps appropriate for me? http://www.oracle.com/technetwork/topics/linuxx86-64soft-092277.html

This person 'appears' to use this to get it working on travis-ci.org unofficially https://github.com/SOCI/soci/blob/master/bin/ci/before_install_oracle.sh

mnoack commented 10 years ago

the spehere_distance_sql calculation is quite different to MySQL and SQLServer in geokit-rails

You use ATAN2 way to get there, but the others use ACOS.....

Whilst you can get to many endpoints mathematically I think it would be best to follow the same formula. You should be able to change the calculations to match the other SQL examples and ensure this still gives the same results? (You should be able to run the tests with your oracle database by adding oracle to test/database.yml then running "rake test DB=oracle")

mnoack commented 10 years ago

And make the most of spacing, and putting things indented onto multiple lines.

This will make hound happy but also any future reader of the code.

And if you can't have spaces for some reason consider spacing it but just calling a "gsub" or something on the end of the final SQL string to remove spaces/newlines, etc.

mnoack commented 10 years ago

Thanks @paulswartz

paulswartz commented 10 years ago

Apparently it's tricky/flaky to install Oracle in Travis: travis-ci/travis-ci#689

I can certainly update the code to match what the other backends are doing; I'm not sure that hacking Oracle support into your CI build is a good idea.

Maybe a comment or something indicating that this is an experimental/unstable backend?

paulswartz commented 10 years ago

I also updated the SQL query to use ACOS instead of ATAN2. The code I have here which uses this still passes.

mnoack commented 9 years ago

Thanks @paulswartz