sqlx icon indicating copy to clipboard operation
sqlx copied to clipboard

[PostgreSQL] citext is not compatable with text

Open izik1 opened this issue 5 years ago • 14 comments

[2020-05-01T19:29:25Z ERROR actix_http::response] Internal Server Error: mismatched types; Rust type `alloc::string::String` (as SQL type TEXT) is not compatible with SQL type 
    Caused by:
        mismatched types; Rust type `alloc::string::String` (as SQL type TEXT) is not compatible with SQL type 

query:

select email, password from "user" where id = $1

Where email is citext. As a workaround, the following cast is required to cause this not to error:

select email::text, password from "user" where id = $1

However, sqlx should be able to handle citext natively.

izik1 avatar May 01 '20 19:05 izik1

String should support decoding from citext without much change here as it's the same binary and text encoding as regular strings. However, a challenge here is that citext does not have a fixed OID because it's an extension type, so we need to match on the type name instead. This might cause weird things if someone else defines a type with the same name but citext still implies some sort of text compatibility so it has a good chance of just doing the right thing anyway.

abonander avatar May 01 '20 19:05 abonander

Other adaptors often to query the metadata on initial connection to discover these unstable OIDs.

This will also come up if supporting Postgres DOMAINs and similar. I think this might also be an issue with supporting hstore but my memory is a bit fuzzy on that one. I did work on another pg adapter a few years ago where I was wrestling with this a bit.

bjeanes avatar Jun 03 '20 05:06 bjeanes

Even though this is for a different driver in a different language, the query and pg_type overview over at https://github.com/will/crystal-pg/issues/88 may be useful.

bjeanes avatar Jun 03 '20 05:06 bjeanes

Is there a workaround that I can do in my code to get CITEXT (single) + CITEXT[] (arrays) columns working? I get an error like:

mismatched types; Rust type `alloc::string::String` (as SQL type `TEXT`) is not compatible with SQL type citext

I've been digging through some github issues... can I use something like PgTypeInfo::with_name("citext"); ? (I'll opt for doing it by name if possible, rather than OID)

Not sure what else I need to do though? How do I tell sqlx that the "citext" type is to be treated the same way as regular "text" ? I'm pretty new to Rust. :)

hi2u avatar Nov 01 '20 10:11 hi2u

For sql to Rust, you can cast to text: (<thing>::TEXT), for Rust to sql, you can help it figure out that it wants text and then cast it to citext: $1::TEXT::CITEXT

izik1 avatar Nov 01 '20 14:11 izik1

Is there a way to tell sqlx how to deal with CITEXT globally, without needing to manually cast in all of the actual SQL queries individually?

The PgTypeInfo::with_name() thing seems to be relevant, but I haven't found any example of how to actually do it.

hi2u avatar Nov 01 '20 15:11 hi2u

Honestly I think we can just add a couple checks:

  • if the type name is present and equals citext here: https://github.com/launchbadge/sqlx/blob/master/sqlx-core/src/postgres/types/str.rs#L13

  • if the type name is present and equals citext[] here: https://github.com/launchbadge/sqlx/blob/master/sqlx-core/src/postgres/types/str.rs#L31

This technically isn't robust if someone adds a type named citext which isn't compatible with text but... does that really matter?

abonander avatar Nov 05 '20 07:11 abonander

@abonander ...seems sensible to me! :)

I'd be very surprised if there is even a single person out there anywhere who has not only created their own custom type named CITEXT... but also used it for something other than some kind of "text"... let alone also also happens using this package.

And even if they did exist... I don't see why everyone else should miss out on this simple fix due to them doing something pretty silly (and that they could change anyway, they could just use a different type name for example).

And wouldn't sqlx already be broken for them anyway?

But for everyone else using CITEXT in its standard & common way (which in reality I reckon is 100% of them)... this seems like a no brainer.

Otherwise you're dealing with having to look up the OID on every new connection... which is wasteful both in terms of the extra query... but more importantly all the extra effort writing more complex code to develop that feature... which will probably never happen (and really doesn't need to). But even then... wouldn't we just using the name "citext" to convert to an OID anyway? So what's the difference in the end given that sqlx can already go by name without the OID lookup query?

if the type name is present and equals citext[]

Depending on where this library is getting the postgres type name from, it might instead be named _citext for arrays... postgres seems to use an underscore prefix for array types in some places rather than the [] suffix.

#[derive(sqlx::Type, Debug, Serialize)]
#[sqlx(rename = "_citext")]
pub struct CiTextArray(Vec<String>);

...is what worked for me in terms of detecting CITEXT[] array columns, so if the code being changed also gets the type name in the same fashion, then yeah _citext might be the type name you need to detect on.

hi2u avatar Nov 05 '20 09:11 hi2u

We still have to do a lookup because all we get in the responses from the protocol is the type OID, but the connection will automatically resolve and cache unknown type OIDs.

abonander avatar Nov 05 '20 10:11 abonander

Ah right, fair enough.

But yeah still sounds like a great solution to me. I'd really love to see this solved once and for all, as I'm having to do lots of workarounds and extra codegen right now. It's also less performant doing unnecessary casting in postgres for all inserts/updates/selects.

I'd help out myself, but I'm still very new to Rust.

Also I'm still a bit fuzzy on whether I can actually use PgTypeInfo::with_name("citext"); in my own project to control this or not, in terms of just getting regular String + Vec<String> types in Rust? Or can that only be used to map to custom Rust data types? I'd really like to not have to deal with special custom Rust types if possible, seeing they really just are regular strings getting copied in/out of other regular strings everywhere.

Can I somehow use PgTypeInfo::with_name to map to a standard Rust String? Or do I need to wait until the sqlx crate itself fixes this internally?

Thanks for all your efforts & consideration!

hi2u avatar Nov 14 '20 07:11 hi2u

For sql to Rust, you can cast to text: (<thing>::TEXT), for Rust to sql, you can help it figure out that it wants text and then cast it to citext: $1::TEXT::CITEXT

Thanks. That was helpful!

Is there a way to perform the cast without SQLx thinking a non nullable column is now nullable?

blazzy avatar Jan 31 '21 00:01 blazzy

Is there a way to perform the cast without SQLx thinking a non nullable column is now nullable?

In case anyone else is looking for an answer, you can force a column to be treated as NOT NULL: https://docs.rs/sqlx/0.5.2/sqlx/macro.query.html#force-not-null

e.g. if column 'email' is of type CITEXT NOT NULL: r#"SELECT email::TEXT as "email!" FROM public.user"#

SilentByte avatar Apr 28 '21 12:04 SilentByte

Is there any way to fix this that doesn't involve copious hacks right now? I'd really like to just declare the type once and move on. Otherwise it makes using the query_as! and other macros incredibly painful.

Or alternatively, is there a PR I could submit to get this fixed in sqlx directly?

autarch avatar Dec 04 '21 22:12 autarch

Just came across this myself, too. Is anyone working on this?

mfreeborn avatar Sep 09 '22 14:09 mfreeborn

Also interested in this.

chessai avatar Oct 17 '22 22:10 chessai

Honestly I think we can just add a couple checks:

  • if the type name is present and equals citext here: https://github.com/launchbadge/sqlx/blob/master/sqlx-core/src/postgres/types/str.rs#L13
  • if the type name is present and equals citext[] here: https://github.com/launchbadge/sqlx/blob/master/sqlx-core/src/postgres/types/str.rs#L31

This technically isn't robust if someone adds a type named citext which isn't compatible with text but... does that really matter?

This sounds like a good idea to me. Is there anyone working on this?

billy1624 avatar Dec 14 '22 07:12 billy1624