jeremyevans / home_run

Fast Date/DateTime classes for ruby :: Unmaintained, unnecessary on ruby 1.9.3+
Other
465 stars 10 forks source link

Improve ragel parser to handle postgresql and sqlite3 variations of iso format #24

Closed funny-falcon closed 13 years ago

funny-falcon commented 13 years ago

It improves ragel parser to handle formats returned by this two databases. With this patch, home_run is just a bit slower than annoying monkey patch for sequel https://gist.github.com/871634

Times on my current machine:

~/tmp/ruby-sequel$ ruby seq_home.rb
            user system total real
base 3.760000   0.080000   3.840000 (  3.841594)
~/tmp/ruby-sequel$ ruby seq_home.rb fix
            user system total real
fix 0.870000   0.080000   0.950000 (  0.955666)
~/tmp/ruby-sequel$ ruby seq_home.rb home_run
            user system total real
home_run  1.070000   0.070000   1.140000 (  1.135983)

With unpatched home run:

~/tmp/ruby-sequel$ ruby seq_home.rb home_run
            user     system      total        real
home_run  2.690000   0.080000   2.770000 (  2.755941)
jeremyevans commented 13 years ago

Looks good. I'll see if I can test and merge this tomorrow, and put out a 1.0.2 gem. Thanks for the help!

jeremyevans commented 13 years ago

Tests well except on JRuby, as the JRuby cext support doesn't include rb_cstr_to_dbl. I'll see if I can come up with a different implementation so it works on JRuby 1.6.0.

funny-falcon commented 13 years ago

Well, initially atof was there and worked well. I thought rb_cstr_to_dbl would be more portable. It seems that I was mistaken :(

jeremyevans commented 13 years ago

I went with atoi/pow, but probably should have used atof. Anyway, it appears to work. I also changed to the code to require at least one digit after the decimal, and support up to 9 digits.

funny-falcon commented 13 years ago

atoi were my first attempt :) well it would be a lesson for me - use first workable solution and do not spend time :)

to require at least one digit - is a good catch.

By the way, please help me with my english: which phrase could also be used with a meaning similar to is a good catch?

jeremyevans commented 13 years ago

"is a good catch" is probably the most appropriate phrase. "is a better solution" has a similar meaning. The difference is that "is a good catch" implies a problem with the existing code, while "is a better solution" doesn't imply a problem. In this case, either would work. I required at least one digit because I think that's more correct, but all that really happens is it falls back to the pure ruby parser. Also, the use of atoi requires at least one digit to work correctly, I think.

Don't worry, your English is certainly understandable, and pretty good for a non-native speaker. :)

funny-falcon commented 13 years ago

I look to the dictionary often :)