Postgres Citext Support #4021
Postgres Citext Support #4021
Conversation
|
Apologies, I'd created this PR against the Diesel repository rather than my own fork by accident. I had meant to ask in Gitter whether it would be acceptable first. |
diesel_tests/tests/types.rs
Outdated
| let rows_inserted = insert_into(case_insensitive::table) | ||
| .values(( | ||
| case_insensitive::non_null_ci.eq("UPPERCASE_VALUE".to_string()), | ||
| case_insensitive::nullable_ci.eq("lowercase_value".to_string()), |
There was a problem hiding this comment.
Maybe we can put not a string but an &str on this one so that both types are tested?
There was a problem hiding this comment.
Sounds like a good call 👍
| #[cfg(feature = "postgres_backend")] | ||
| #[derive(Debug, Clone, Copy, Default, QueryId, SqlType)] | ||
| #[diesel(postgres_type(name = "citext"))] | ||
| pub struct Citext; |
There was a problem hiding this comment.
I think we need to add this type to the list of types that are known by diesel in the cli when generating the schema, so that the cli doesn't generate the corresponding type in the schema.rs.
Im surprised you haven't encountered issues due to this during tests.
Did you run tests using the cli yet?
There was a problem hiding this comment.
Thanks, I'll have a look into that. I started working backwards using a schema generated without Citext support, I still have to test starting with schema generation. I'll check that and add get back to you with the results.
As I mentioned in Gitter, I'd jumped the gun a little on creating the PR before I had a chance to test properly but I'll make sure it all works before pushing any more changes.
There was a problem hiding this comment.
Nice catch, I've updated the list of known types to include Citext and added a test to validate it generates the correct schema. Let me know if there's a better place I could put it.
I've also tested with our application in work and the test suite (which include case insensitively fetching a record by a citext field) passes.
Are there any other areas I need to consider?
tommymcc
force-pushed
the
feature/citext-support
branch
from
2350f87 to
423847b
Compare
Adds support for the Postgres citext type.