[PATCH] Multiple fixes in PostgreSQL adapter (makes ruby-postgres usable)

The patch

Ok, I've changed my approach to solve this problem.

Instead of extending DateTime with to_time, to_date, xmlschema, etc. I've *fixed* the PostgreSQL adapter which was really broken when using ruby-postgres (about 17 tests failed).

It was when running the tests when I realized that the Rails way to cast timestamp columns is returning Time objects, so keeping the DateTime objects made no sense anyway.

I've also solved a bug when using postgres-pr which arised when calling the method active? with no connection (test_underlying_adapter_no_longer_active uncovered this bug ;-)).

This patch should make "rake test_postgres" (in activerecord) return no errors/failures now. Tested with the latest version of the following drivers: postgres, postgres-pr and ruby-postgres.

Watch out for the new patch (diff) which should appear attached in a few secs.

Proper fix (ignore the previous one)

I forgot to mention some other bits I have fixed when using the ruby-postgres adapter:

• Boolean columns are returned as "t", "f" or nil (when the column has a NULL value).
• Bytea columns don't need to go throught unescape_bytea.

I've just optimized my previous patch, based on the experiences by Lugovoi Nikolai with ruby-postgres (as seen on the Rails mailing list):

If I add to test/connections/native_postgresql/connection.rb:

require 'postgres'
PGconn.translate_results = false

then driver doesn't parse query results and only one test fails
[./test/adapter_test.rb:56].

Using PGconn.translate_results = false makes it a little less wordy to translate columns. As a side-effect, bytea columns need to be unescaped again (as they currently do now).

Once again, I made sure all the tests pass using the postgres, postgres-pr and ruby-postgres drivers.

Proper fix optimized (ignore the previous one)

Having these fixes will make it a lot easier for me to convince other people at work to use rails. This will close 2 other tickets that were getting in the way.

Thanks again for all the hard work! Especially over the weekend! Would love to see this in a new rails release soon.

Isn't there the possibility of using Time instead of DateTime?

http://www.recentrambles.com/pragmatic/view/33

An 800% speed decrease when using DateTime seems worringly high.

Isn't there the possibility of using Time instead of DateTime??

Yes. This is the current behaviour with the latest patch.

See this comment from #4540

"After applying the last patch on #4461 to Rails 1.1 the error remains the same (but I've had to overload DateTime? to support a to_i method so it's still not working and now it also is slower)."

I will take a look at it tomorrow.

But, I think he's using the *oldest* patch and not the newest (postgres_adapter_multiple_fixes_optimized.diff), somehow.

Has anyone else had troubles using the newest patch?

I applied "postgres_adapter_multiple_fixes_optimized.diff (2.9 kB) - added by ruben.nine@gmail.com on 03/31/06 00:29:57." (it's the only patch I've on my desktop so no mistakes there). One chunk failed, I think it was the fix for postgres-rb. I will recheck again tomorrow.

My previous comment about DateTime was because after applying the patch I got an error caused by a line similar to this one:

<td class="timestamp"><%=format_interval((@item.updated_on.to_i + 86400 * 14) - Time.now.to_i, 'dias')%></td>

updated_on was now a DateTime instance instead of a Time instance and so it didn't had to_i.

To dharana@dharana.net:

I've been double-checking that the timestamp (with or w/o timezone) columns were properly converted to Time instances. This time, apart from testing it on my ppc/mac i did test it on a linux machine (using the 3 postgres drivers), and in all the cases I'm getting Time instances as expected.

What driver (and version) is causing you troubles?

ruben, I found dharana's setup at #4540 where he says:

"I am using ruby-postgres 0.7.1. (Gentoo official ebuild)"

There is more detailed info in #4540 if needed.

Also, you can add your email to the CC field if you want to be notified of changes on this ticket.

Ok, that's not a recent ruby-postgres driver.

I would encourage dharana using the ruby-postgres gem instead.

I've, somehow tested my patch with the driver he appears to be using, (ruby-postgres 0.7.1 downloaded from http://www.postgresql.jp/interfaces/ruby/archive/ruby-postgres-0.7.1.tar.gz) getting the expected results again:

843 tests, 2843 assertions, 0 failures, 0 errors

Regarding the troubles he's having on #4540, my patch does not fix that.

I replaced the previous driver with "postgres" driver ("gem install postgres") and now it's working again.

Switching back to ruby-postgres-0.7.1.2005.12.21 (currently 2nd option when doing a gem install "ruby-postgres") it converts attributes into DateTime. I can replicate this. I can tell you for sure that it did worked differently with Rails 1.0 ( http://gamersmafia.com/ this app is using ruby-postgres 0.7.1 installed from source with Rails 1.0 and exactly the same code crashes after updating to rails 1.1).

Is 0.7.1.2005.12.21 an outdated version of the driver? I can't find a newer version in it's homepage http://ruby.scripting.ca/postgres/

I don't mind switching to another driver and with more reasons if it's the updated version but ruby-postgres still comes first when searching with google. It's a little messy.

Has anyone done a performance comparison between the different drivers? (I'm sorry if this is a little bit offtopic, feel free to ignore)

If I'm not mistaken, these are the 3 postgresql drivers in ruby:

1. postgres - old and stale but reliable 2. ruby-postgres - new, beta, improving version of postgres 3. postgres-pr - pure ruby, slowest, works on all OS

I believe ruby-postgres 0.7.1.2005.12.21 is a beta snapshot--not an official stable release (even if it turns out to be bugfree).

I just posted this info into the rails FAQ on the wiki today. If this info is not correct, feel free to change the FAQ.

Ideally, I'd like to see a ruby-postgres 7.2 that everyone can rally around as the recommended driver.

I really hope this patch is included in 1.1.1 and works with all three drivers.

The current page for ruby-postgres driver is at:

http://ruby.scripting.ca/postgres/

Maintainer is Dave Lee. dave@cherryville.org

Great work guys, The latest patch works great on all adapters ( ruby-postgres, postgres and postgres-pr ) and even survives shopify tests scrutiny.

(In [4156]) Multiple fixes and optimizations in PostgreSQL adapter, allowing ruby-postgres gem to work properly. Closes #4461

FYI. I added postgres to keywords of various tickets and mentioned this ticket a few times. Adding note to this closed ticket to view this report for other postgresql issues using this report:

http://dev.rubyonrails.org/query?status=new&status=assigned&status=reopened&keywords=%7Epostgres&order=id&desc=1

Ruby 1.8.2 Rails 1.1.2 (which seems to include postgres_adapter_multiple_fixes_optimized.diff above) PostgreSQL 8.1.3 postgres 0.7.1, ruby-postgres 0.7.1.2006.04.06, postgres-pr 0.4.0 (each one, separately)

Problem is, the class of the parameter passed to cast_to_time is String, not DateTime (not sure why), so it isn't processed there, but later in string_to_time, where the time zone is ditched.

Further addition, this is using TIMESTAMP WITH TIME ZONE.

string_to_time just discards time zone info (also cast_to_time seems to do). Also, where is Time.send defined? (can't find it in the documentation)

Hmm, I'm not sure extracting the tz from the TIMESTAMP WITH TIME ZONE would cope well with the current Rails design.

Rails lets you define the timezone to be used for your app in the environment.rb -- check config.active_record.default_timezone.

As for Time.send, this is a method inherited from Object that basically let's you call any method on the receiver class:

Time.send(:now)

is the same as

Time.now

Thanks for the explanation!

But setting Base.default_timezone is not enough. You get this:

2006-04-20 17:14:53 CDT 2006 (defaulti_timezone = :local) 2006-04-20 17:14:53 UTC 2006 (default_timezone = :utc).

Possible solutions:

Tell postgres to use UTC as default when timezone is utc. ("set time zone UTC" would work, maybe at connection time) Use AT TIME ZONE in each query, would work too, but is ugly.

Or, do it in str_to_time............found something interesting. changed

Time.send(Base.default_timezone, *time_array) rescue nil

with

time = Time.mktime(*time_array) rescue nil if (Base.default_timezone == :utc) then time = time.utc rescue nil end time

and it works as (I) intended. But nothing short of that.

Time.utc disregards the tz parameter when acting as a class method (so does Time.local), but converts to UTC (also, as I intend), when used as an instance method.

oops, should had preview it, meant

time = Time.mktime(*time_array) rescue nil
if (Base.default_timezone == :utc) then time = time.utc rescue nil
end
time

Also, cast_to_time is the proper place for this. But this solve only the retrieving from the DB, not putting the values there.

This, in configure_connection, solves both:

if (Base.default_timezone == :utc)
  execute("SET TIME ZONE 'UTC'")
end

}}}

To Gerardo:

Please submit a well tested patch (that means testing it with other adapters but PostgreSQL) and check that nothing breaks.

Also, check the ticket #4492.

That's where you should submit the patch if you can come up with a proper solution.

Good luck!

Done, thanks.

Submitted the second way, which is PostgreSQL contained, and works both for retrieving from and putting data on the DB.