Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Postgres enum support #2

Closed
Follpvosten opened this issue Jan 28, 2021 · 5 comments
Closed

Postgres enum support #2

Follpvosten opened this issue Jan 28, 2021 · 5 comments

Comments

@Follpvosten
Copy link
Contributor

Postgres enums are basically strings that can only have a fixed set of values; on insert they check if the value is in that set.
These are basically custom types (created with a CREATE TYPE statement).

Currently the API of sea-query doesn't really support them; there is no way to create a CREATE TYPE statement, so I would have to manually write that, and there's also no way to reference a custom type in a ColumnDef, so I would also have to write my CREATE TABLE and ALTER TABLE statements manually whenever an enum is involved.

Are there plans to support this functionality?
In the short term, it would probably be enough to provide a .custom<T: Iden>(typ: T) or .custom(typ: &str) method on ColumnDef, that would allow me to still use the query builder for non-CREATE TYPE statements and would also be database-independent.
In the future, full support for CREATE TYPE/ALTER TYPE/DROP TYPE statements would also be nice, maybe behind a postgres feature.

@tyt2y3
Copy link
Member

tyt2y3 commented Jan 30, 2021

Would #5 work for you for now? If so, we'd merge.
I think PG's enum is a nice feature indeed. We might implement that in the future, after considering other create type variants.

@Follpvosten
Copy link
Contributor Author

Yes, that looks perfect for now! Thanks a lot!

@Follpvosten
Copy link
Contributor Author

First of all, thanks for implementing this! I was ready to contribute it myself once I got to it, but this is very helpful in any case.

(Quoted from #19)

@Follpvosten Please try 0.6.0

use sea_query::extension::postgres::Type;

assert_eq!(
    Type::create()
        .as_enum(Font::Table)
        .values(vec![Font::Name, Font::Variant, Font::Language])
        .to_string(PostgresQueryBuilder),
    r#"CREATE TYPE "font" AS ENUM ('name', 'variant', 'language')"#
);

assert_eq!(
    Type::drop()
        .if_exists()
        .name(Font::Table)
        .restrict()
        .to_string(PostgresQueryBuilder),
    r#"DROP TYPE IF EXISTS "font" RESTRICT"#
);

So with the current implementation of sea-query-derive and postgres-derive, a definition for this enum would have to look something like this:

// You can derive FromSql and ToSql on C-like enums via postgres_derive
// But neither of the derives allows customizing the casing for all fields, so...
#[derive(FromSql, ToSql, Iden)]
#[postgres(name = "asset_kind")]
pub enum Font {
    #[iden = "asset_kind"]
    Type,
    #[postgres(name = "name")]
    Name,
    #[postgres(name = "variant")]
    Variant,
    #[postgres(name = "language")]
    Language,
}

This gets a bit long in the tooth fairly fast, especially with large arrays, and postgres will also generate a variant for Type (which we can just ignore in this case). This is not a real issue tho, and I would probably be the one to contribute a solution (the easiest one would be a #[iden(rename_all = "PascalCase")]).

For using the values in Insert statements or Select queries, I would then probably use the Iden::to_string() method, so that's not an issue either, which is good. (enum values are treated as strings anyways.)

The only real issue I see with this first implementation is that there's no ALTER TYPE statement: https://www.postgresql.org/docs/current/sql-altertype.html
Especially the ALTER TYPE ADD VALUE and ALTER TYPE RENAME VALUE statements are important for enums. For now tho, I will not need these, and I can contribute them when I get to that point.

So far, the implementation does work for me tho; I can create separate issues for AlterTypeStatement and changes to derive in the future.

@tyt2y3
Copy link
Member

tyt2y3 commented Feb 23, 2021

Nice thanks! 🍻

@Follpvosten
Copy link
Contributor Author

Follpvosten commented Feb 25, 2021

Update: There's still an issue, namely that deriving ToSql creates code like this:

fn accepts(...) -> bool {
    // ...
    if variants.len() != 7usize {
        return false;
    }

So that means having an EnumType::Type variant doesn't work. I can work around this by using a separate unit struct tho.

EDIT: Note that this isn't fixable at all unless postgres adds something like #[postgres(skip)].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants