will / crystal-pg

a postgres driver for crystal
BSD 3-Clause "New" or "Revised" License
462 stars 77 forks source link

Read Slice(UInt8) as String if given as type #221

Closed matthewmcgarvey closed 3 years ago

matthewmcgarvey commented 3 years ago

Fixes #43

In order to support the citext extension in Avram, we need to be able to treat the column as a String. Two solutions were laid out in the connected issue:

B was considered the more "complete solution" but also the more complicated. Since it's been 4 years since the issue was opened and over a year since the potential solutions were laid out, I went with the more immediate option A. If the read type is Slice(UInt8), which is the type of a citext column, and #read was called with a type of String, then convert it to String. This helps with citext interop and I'm sure it helps with other column types as well.

matthewmcgarvey commented 3 years ago

This is related to https://github.com/will/crystal-pg/pull/89 which is attempting a more complete implementation laid out here https://github.com/will/crystal-pg/issues/43#issuecomment-221906216 but stalled out.

will commented 3 years ago

Thanks! This looks good to me. I don’t myself use citext though, so I'm going to leave this open for a couple of days in case anyone has comments. But I expect to merge this, then do a release sometime early next week.

bcardiff commented 3 years ago

Oh! I remember. Double check that when using rs.read(String?) the conversion will work. It will probably wont. In crystal-sqlite3 there are a couple of these IIRC.

will commented 3 years ago

Thanks!