mkroman commented 8 years ago

Diesel does not currently support enumerated types created with PostgreSQL.

Documentation on enumerated types can be found here.

More documentation on CREATE TYPE can be found here.

Example

CREATE TYPE track_type AS ENUM ('download', 'stream');

CREATE TABLE track (
  id SERIAL,
  type_ track_type
);

When compiling, the generated code expects a type named like the custom data type.

error: type name `Track_type` is undefined or not in scope [--explain E0412]
 --> src/lib.rs:1:1
1 |> #![feature(custom_derive, custom_attribute, plugin)]
  |> ^ undefined or not in scope
<diesel macros>:5:1: 5:71: note: in this expansion of table_body! (defined in <diesel macros>)
src/schema.rs:4:1: 4:37: note: in this expansion of table! (defined in <diesel macros>)
src/schema.rs:4:1: 4:37: note: in this expansion of infer_schema! (defined in src/lib.rs)
help: no candidates by the name of `Track_type` found in your project; maybe you misspelled the name or forgot to import an external crate?

killercup commented 8 years ago

FYI in SQLite you could try to do something similar with a text column and CHECK:

CREATE TABLE track (
  id INTEGER PRIMARY KEY,
  track TEXT CHECK(track IN ('download', 'stream')) NOT NULL
);

carsonmyers commented 7 years ago

Is there currently a good way around this? Like a custom de/serializer?

dstu commented 7 years ago

As an outsider who looked through the code to figure out if there is a way to do this, it appears that supporting a user-defined Postgres type requires knowing the object identifier that is in use for that type. In the case of enums, this requires extracting information from pg_type and pg_enum tables. All told, you're looking at:

Getting your type information out of relevant postgres system catalogs. You'll need to refer to pg_type and pg_enum, along the lines of:

select pt.typname, pt.typtype, pt.typcategory, pe.enumlabel, pt.oid
from pg_type pt
inner join pg_enum pe on pe.enumtypid = pt.oid
where pt.typcategory = 'E';

Creating peer/placeholder types, analogous to diesel::types::Date, for each type so discovered.
Registering these types with diesel. Refer to diesel/src/types/impls/ for examples.

Since runtime database information is needed, some sort of codegen macro along the lines of infer_schema! seems necessary. I'll probably take a crack at this over the next few days, but I can't promise much.

dstu commented 7 years ago

I've put together a rough, partial implementation of Postgres enum support, which creates a macro called infer_enums that acts much like infer_schema, except it creates Rust types for each enum type it discovers in the database. Please be aware that these changes are owned by Google and not actually mine to redistribute until I get a copyright release, so I've (temporarily) deleted my public fork. I hope that I'll get an okay to make my changes public after the Christmas holiday.

Unfortunately, there are currently a few assumptions about type definitions that make it difficult to use custom enum types in tables:

It is necessary to create a fresh impl of HasSqlType for each type that infer_enums discovers, but you cannot do that outside of the diesel crate. I think this can be worked around, but at present I don't see how to do this without a significant refactor.
infer_schema assumes that column types correspond to types declared in the namespace diesel::types, which is no longer true if arbitrary new enum types can be created in other crates.

I haven't actually done the legwork necessary to know how to deal with encoding or decoding enum values to/from raw bytes for handoff with Postgres. I assume that this will essentially entail handling type-tagged integers.

Finally, handling schemas correctly is on the back burner until everything else is taken care of. It'll require an additional join.

dstu commented 7 years ago

I've been able to address (1) and (2), but getting everything working in the integration tests has run head-long into #348. If anyone has successfully figured out the minimal footprint of traits needed to make a simple, atomic SQL type, some documentation or example code would be welcome.

killercup commented 7 years ago

Oh, wow! That sounds great! Thank you for digging into this!

Currently, the SQL types and the Rust types are divided; the table! macro will contain a SQL type and a struct with e.g. Queryable has the ability to deserialize these types into the actual Rust types it contains. What you described sound like for an inferred enum, SQL and Rust type are the same. I haven't thought much about this, maybe it makes sense. Just thought I'd mention it.

@jethrogb was on Gitter a few weeks ago and wanted to implement a custom type. Maybe you can get some additional information from the log there.

Happy holidays!

Stu Black notifications@github.com schrieb am Sa. 24. Dez. 2016 um 07:50:

I've been able to address (1) and (2), but getting everything working in the integration tests has run head-long into #348 https://github.com/diesel-rs/diesel/issues/348. If anyone has successfully figured out the minimal footprint of traits needed to make a simple, atomic SQL type, some documentation or example code would be welcome.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/diesel-rs/diesel/issues/343#issuecomment-269072202, or mute the thread https://github.com/notifications/unsubscribe-auth/AABOX8AJ9cTIqXkGKoakndQpU0iVYeriks5rLMCugaJpZM4IlWGG .

dstu commented 7 years ago

My pleasure. I suspect that it will not be too difficult to extend this work to create Rust struct types automatically from user-created composite types in Postgres, and I'm also considering that.

What you described sound like for an inferred enum, SQL and Rust type are the same. I haven't thought much about this, maybe it makes sense.

That is exactly what the current implementation does. I have wondered a bit as to whether this is really the right thing to do, but I still believe it is. It might be a good idea store some metadata with the generated type (probably in a static impl of some DieselEnum trait), but I see no reason to stick it on a separate "backend" type. Future extensions might be backend-specific, such as when creating proper Rust enums for MySQL enum columns or providing an impl of Cmp that matches the ordering that Postgres imposes on enum variants, but I don't see a particular need for a different type.

There is probably a long-range design issue lurking behind the scenes here. I've already run into a few instances of code assuming that SQL types live in diesel::types, and the proposed change for #429 also does that. I'm working around this for now with a hard-coded list of built-in SQL types, but that seems a poor solution. If types are going to be generated during compilation like this, a compile-time type registry might be a very good idea. Otherwise, we are going to be left with relying on heuristics to determine how to resolve a given type name. For now, the heuristic is: if the type is builtin, resolve it absolutely in diesel::types, or else resolve it locally because a macro just generated it in the current namespace. This is brittle.

@jethrogb was [on Gitter][1] a few weeks ago and wanted to implement a custom type. Maybe you can get some additional information from the log there.

I think I have the necessary traits implemented, but I'm still running into problems. A second pair of eyes will help once I have permission to share a patch.

kybishop commented 7 years ago

Hey @sgrif, does your comment and gist here still apply as a valid workaround until this issue is sorted?

dstu commented 7 years ago

I just got the OK to contribute and copyright release from my friendly neighborhood corporate overlords, so I'll reinstate my fork soon.

dstu commented 7 years ago

Feature bumps and deprecations from the 1.15 release and deprecation of custom_derive (https://github.com/rust-lang/rust/issues/29644) seem to have complicated things a little bit. I note that sh bin/test integration no longer runs successfully on the master branch (under rustc 1.16.0-nightly (47c8d9fdc 2017-01-08)). I believe that I was previously able to run that command to success on top of my changes in late December, on some older rust nightly. Please let me know what I should be doing to run tests.

I've pushed what was (more or less) working back in December to the custom_types_in_database_columns branch of my fork. I'd open a PR, but I don't think things are quite working yet. What's the deal with the test script?

sgrif commented 7 years ago

Try rebasing on master and running on beta

dstu commented 7 years ago

The most significant test failures seem to be #571, which is not directly related to adding support for enums. With a fix patched in locally, the problem I am currently running into is that types which are declared in a dummy namespace by infer_enums! don't seem to be visible when they are used in table declarations generated by infer_schema!. For example, diesel_tests/schema.rs currently reads:

infer_enums!("dotenv:DATABASE_URL");
pub use self::__diesel_infer_enums::UserType;  // Inserted by hand; shows that the type exists.
infer_schema!("dotenv:DATABASE_URL");  // Errors here because UserType isn't known.

I have tried changing the table declarations generated by InferSchema so that they use a relative type name. This is done in diesel_codegen_shared/src/schema_inference/pg.rs, where the path to non-builtin types is constructed. Using a type like super::UserType or super::__diesel_infer_enums::UserType doesn't work. In fact, if a type name like super::UserType is provided, the compiler helpfully says:

error[E0412]: unresolved type `super::UserType`
 --> tests/schema.rs:7:1
  |
7 | infer_schema!("dotenv:DATABASE_URL");                                                                                                                                                        
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no resolution found
  |
  = help: possible candidates are found in other modules, you can import them into scope:
  = help:   `use schema::__diesel_infer_enums::UserType;`
  = help:   `use type_inference::postgres::__diesel_infer_enums::UserType;`
  = note: this error originates in a macro outside of the current crate

I suspect that converting a literal "self" or "super" with syn::PathSegment::from is not quite the right thing to do, since those are keywords that may require special treatment. But that does not explain why UserType wouldn't be visible when it is being exported explicitly with pub use.

A couple of different hacks (running infer_enums! in its own module and importing from that, trying to do enum declaration and table schema declaration in the same procedural macro) haven't panned out so far.

I'll have to try to boil this down to a simple example to verify it, but this is acting an awful lot like the enum types whose declarations I'm generating are not visible in the code that generates the table schema.

dstu commented 7 years ago

You can work around the question of how to look up dynamically generated types by requiring that the user specify the module where any non-builtin types can be found, so that's how things now work. This isn't great (and raises questions about how to deal with types derived from schemas, which one might reasonably want to live in sub-modules), but it looks like it works for now.

I must have been recollecting wishfully when I said that I thought most tests were passing earlier. I'm now running into some deficiencies in implementing the interfaces needed to make a custom type queryable/insertable/updateable/etc. This is all in diesel_codegen/src/type_inference.rs. An experienced eye there would be very helpful in noticing any obvious omissions or deficiencies.

dstu commented 7 years ago

I will continue further discussion of the implementation I'm working on in #580.

dstu commented 7 years ago

580 has been closed as discussed there. (It needed to be factored apart and fell too far behind diesel main.)

I have been swamped with events at work and in my personal life, and this has prevented me from pulling a set of new PRs together. I'd be happy to discuss this further with anyone who is interested in getting a PR together sooner, and I will share whatever I can put together, when I can.

coder543 commented 7 years ago

I just encountered this bug. Not having this feature means I'm (at least for now) switching from strongly-typed enums in my database to varchars... which is not great. I would love to have this feature.

dsvensson commented 7 years ago

@dstu it would be nice if inferring schema is not required, other than perhaps creating a first example of a Rust enum (which might then be renamed, while keeping some annotation or so for the name used in Postgres... mapping)... and then connecting the dots upon actual connect, rather than having any kind of ids hardcoded. Or did I misunderstand you there when you mentioned "Since runtime database information is needed, some sort of codegen macro along the lines of infer_schema! seems necessary" ?

turboladen commented 6 years ago

FWIW, if working with pg enums in diesel meant having to define a rust annotated struct (or... enum?) to represent the pg enum, that'd seem totally reasonable to me.

sgrif commented 6 years ago

So I'm going to close this as a duplicate of #162. At this point, everything needed for absolute minimum "support" for PG enums is there. We even have a test case for this in Diesel itself.

I'm aware that this requires more code than people would like right now (this is true of implementing new types in general, not just enums). #162 is the tracking issue for that, and is going to be the main focus of 1.1.

Ideally you'll just need to provide an impl of ToSql, FromSql, and HasSqlType (I originally wanted to have the rest of the traits come from blanket impls, which requires changes in the language, and is why there hasn't been much movement on this. At this point I'm thinking we'll just provide some derives for the other traits).

At this point, I do not want to provide any special support for PG enums (e.g. infer_enums!(), diesel print-enums, or #[derive(PgEnum)]). Such a feature would have to have some very strong opinions on the mapping, that I don't want to have in Diesel. That said, there's no reason that one of those features couldn't be supplied by another crate. I'm happy to help someone with the initial implementation of such a crate, but I wouldn't want it under the diesel org.

TL;DR: Custom types in general are "supported". It's going to get more ergonomic in 1.1. We aren't going to provide special handling for PG enums.

coder543 commented 6 years ago

I don't necessarily agree with the decision, but I appreciate the way you're approaching the situation. I personally think enums are important enough to be a special case, but as long as it is possible to support them in Diesel, that's good enough for me. As you said, an outside crate could be developed to provide an opinionated solution to this issue that could act as a way to reduce boilerplate for those who are interested.

Looking at the example code you linked to, it seems pretty reasonable to me. My only real question at this point: is using infer_schema! is possible with a database that has custom types in some/all of the tables? I've been busy with other projects for awhile now, but from what I recall, infer_schema! might have actually generated code which could not compile in such cases, and I don't remember finding a way to inject the custom types into the namespace that infer_schema! created.

sgrif commented 6 years ago

Not at the moment, but using diesel print-schema works

On Sat, Dec 16, 2017, 3:11 PM Josh Leverette notifications@github.com wrote:

I don't necessarily agree with the decision, but I appreciate the way you're approaching the situation. I personally think enums are important enough to be a special case, but as long as it is possible to support them in Diesel, that's good enough for me. As you said, an outside crate could be developed to provide an opinionated solution to this issue that could act as a way to reduce boilerplate for those who are interested.

Looking at the example code you linked to, it seems pretty reasonable to me. My only real question at this point: is using infer_schema! is possible with a database that has custom types in some/all of the tables? I've been busy with other projects for awhile now, but from what I recall, infer_schema! might have actually generated code which could not compile in such cases, and I don't remember finding a way to inject the custom types into the namespace that infer_schema! created.

— You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub https://github.com/diesel-rs/diesel/issues/343#issuecomment-352215549, or mute the thread https://github.com/notifications/unsubscribe-auth/ABdWK3CVhaMcOvmOk96IWmiPhbmxc-mqks5tBD_6gaJpZM4IlWGG .

adwhit commented 6 years ago

Just thought I'd mention to anyone who comes across this issue that I've made a crate to derive the necessary trait impls, it's up on crates.io. Feedback welcome.

l4l commented 4 years ago

As @weiznich pointed out in gitter, it's inconvenient to provide custom types (including enums), so probably the best way to do is via the crates:

Each backend has a completely different way to represent values send over the "wire"

Only postgres supports real custom types, neither sqlite nor mysql do this ("enums" can be done with check constraints there)

Even on postgres it's not clear how to represent a enum on sql side, as there are multiple solutions:

map to an custom enum type

map to a foreign key reference

map to an integer value

map to an string value

…

Given those point's it's just not possible to write a generic FromSql/ToSql impl, as those impls just depend on your own requirements, so you need to define how to translate them to/from the database.

(Just to clarify that: It's not different to for example Int4, diesel does internally just the same for the built-in types, it just defines how the conversion happens there, as it is clear for those cases . The answer is: You can exactly tell how to map postgres Int4 to a rust i32 as you know the representation of both types while writing diesel. You cannot tell this for arbitrary custom types, as you neither know the rust nor the sql representation for the general case.)

diesel-rs / diesel

Support enumerated types #343

Example

580 has been closed as discussed there. (It needed to be factored apart and fell too far behind diesel main.)