Rails Ruby on Rails → 2.x

Comments and changes to this ticket

• 

  azimux July 13th, 2008 @ 11:52 AM

  • no changes were found...

  • Butterfly.JPG 2.1 MB

• You flagged this item as spam.
  

  Kim Toms August 18th, 2008 @ 10:04 PM

  • Tag changed from “2.1, activerecord, bug” to “2.1, activerecord, bug, tests”

  This patch modifies the binary test to cause the postgres adapter to fail this test.

  • Unprocessed: my-patch.gif 852 Bytes
• 

  Kim Toms August 19th, 2008 @ 12:57 AM

  • no changes were found...

  • postgres-611.patch 4.1 KB

• You flagged this item as spam.
  

  Kim Toms August 19th, 2008 @ 12:57 AM

  • Tag changed from “2.1, activerecord, bug, tests” to “2.1, activerecord, bug, patch, tests”

  This patch fixes it, but may break compatibility with older version of the database.

• You flagged this item as spam.
  

  azimux August 19th, 2008 @ 09:04 AM

  Nice patch kim!

  I'm going to try applying it and giving it a whirl. One thing I notice right off the bat is your new code assumes unescape_bytea will be defined by the connection, but I know of at least one pg adapter that doesn't implement this method (postgres-pr)

  It might be worth keeping part of the old code in a new method, maybe like this:

  
  def non_native_unescape_bytea(value)
                    result = ''
                    i, max = 0, value.size
                    while i < max
                      char = value[i]
                      if char == ?\\
                        if value[i+1] == ?\\
                          char = ?\\
                          i += 1
                        else
                          char = value[i+1..i+3].oct
                          i += 3
                        end
                      end
                      result << char
                      i += 1
                    end
                    result
  end
  

  and then in your code you could do something like:

  
                  elsif res.ftype(cell_index) == BYTEA_COLUMN_TYPE_OID
                    row[cell_index] = if PGconn.respond_to?(:unescape_bytea)
                      PGconn.unescape_bytea(row[cell_index])
                    else
                      non_native_unescape_bytea(row[cell_index])
                    end
                  end
  

  I spent the better part of a weekend a while back battling with this bug. The solution I came up with was no where near as clean as your patch, so I'm pretty excited to try this out. Thanks again!

  I noticed you left string_to_binary alive, but took binary_to_string out. I'm guessing this code is used only when building queries? Is it safe to at least remove the regular expressions from binary_to_string?

  I remember I was also going to write a test for this as well, but when I ran the tests I got 6 failures from existing tests, which discouraged me from spending anymore time on it. Were you able to have every single test pass?

• You flagged this item as spam.
  

  azimux August 19th, 2008 @ 12:34 PM

  Everything I tested out with kim's patch seemed to work

  here's a patch that, if applied after kim's patches, allows it to work with postgres-pr

  • 0001-allows-kim-tom-s-bytea-fix-to-work-with-postgres-pr.patch 2 KB

• You flagged this item as spam.
  

  capellamusic October 7th, 2008 @ 05:00 PM

  • Tag changed from “2.1, activerecord, bug, patch, tests” to “2.1, activerecord, bug, patch, tests”

  Hi, I've tried your patches but although the objects with bytea columns are correctly inserted into the DB, the listings (with will_paginate) aren't working anymore. Any idea about it?

• You flagged this item as spam.
  

  Kim Toms November 3rd, 2008 @ 10:40 PM

  • Tag changed from “2.1, activerecord, bug, patch, tests” to “2.1, activerecord, bug, patch, tests”

  Do you have a specific failing test? That is the first thing to work out. I don't see any test with paginate in its text.

• You flagged this item as spam.
  

  azimux November 3rd, 2008 @ 10:49 PM

  Note to rails committers: It looks like postgres-pr is no longer supported by rails and only ruby-pg and ruby-postgres work anymore. So the patch I submitted to make Kim's patch work with postgres-pr is probably irrelevant at this point.

• You flagged this item as spam.
  

  Kim Toms November 5th, 2008 @ 08:44 PM

  After reviewing will_paginate, I don't see any reason for it not to work, so I will require more details of the failure. If you'll be at rubyconf 5-8 Nov, I'll be there too.

• 

  Frederick Cheung December 21st, 2008 @ 05:24 PM

  • State changed from “new” to “resolved”

  Should have been resolved by #1063. Do reopen if it's still a problem