table macro that allows remapping table and field names

[kardeiz]

This PR is mostly meant as a starting point for resolving #342 (using table or field names that are reserved keywords in Rust). I didn't think too hard about how to do the field/table name specification. The macro can still be used as before.

If nothing else, this may be helpful as a starting point for others who have to work with tables that have fields like type.

You can use the macro like:

table! {
    objects as ["objects"] (id) {
        id -> Uuid,
        label -> Nullable<VarChar>,
        type_ as ["type"] -> Nullable<VarChar>,
        created_at -> Timestamp,
        updated_at -> Timestamp,
    }
}

[sgrif]

So I'm not entirely opposed to this feature, and would like to hear what @diesel-rs/core have to say on it. However I'm against it personally. I think it adds unnecessary complexity for a simple problem. The only use case where this is necessary are to have column names which are the same as keywords in Rust. This is a relatively limited set, and I think we can avoid the complexity simply by having a defined convention in those cases (e.g. type -> ty). This solution also has the benefit of being something we can do automatically in codegen.

[kardeiz]

Even if it is a small set of protected words, there are cases (e.g., working with a legacy database) where you can't avoid them.

There are other cases (though along the same legacy line) where you might want a re-mapping even when not strictly required (e.g., non-underscored field name to snakecase).

As I noted on #342, I think having a convention for infer_schema would be great; I just think it would be ideal to be able to configure this relatively easily when desired (e.g., some people might prefer to work with a type_ field rather than ty).

Also, how would the convention work when going in the ty -> type direction (e.g., when using the table macro)? Would

table! {
    objects {
        ty -> Nullable<VarChar>
    }
}

map to a field named "type" or "ty"?

[sgrif]

Ok so I've put some more thought into this. I like the idea but I don't like the API. It's not clear to immediately which side is the rust name and which side is the SQL name. as doesn't imply thats the aliasing which is occurring either.

[killercup]

I think this would be useful to have. I don't like the as ["name"] syntax though. In other contexts, it seems to work in the opposite way (use external::stuff as my_name).

As there is currently no way in a stable macro to match on "type" as ty -> VarChar, we need to think of another syntax (or wait for lit to be stabilized: rust-lang/rust#35625).

How about ty (aka "type") -> VarChar or ty (column "type") -> VarChar? I especially like the last one, as it may be verbose but is absolutely clear. If you want to go even more verbose, (using column "type") or (using table "with_weird_table_name") reads nice as well.

table! {
    objects
    (table "weirdly_long_table_name_for_something_as_simple_as_objects")
    (id)
    {
        id -> Uuid,
        label -> Nullable<VarChar>,
        ty (column "type") -> Nullable<VarChar>,
        created_at -> Timestamp,
        updated_at -> Timestamp,
    }
}

[kardeiz]

@killercup I think your suggestion is good (especially the more verbose one), but what I was thinking (after my initial PR but before your comment) was the serde-like [rename="type"], which could either be in a position analogous to standard field annotation:

table! {
    objects {
        id -> Uuid,
        [rename="type"]
        type_  -> Nullable<VarChar>
    }
}

or like:

table! {
    objects {
        id -> Uuid,        
        type_ [rename="type"] -> Nullable<VarChar>
    }
}

or using column_name/table_name instead of rename.

[killercup]

@kardeiz Why not use #[rename=$name:expr] so it looks like a regular attribute?

I think is actually more confusing—because we are in a macro context where there are no items that can have actual attributes (as seen by rustc). If the macro was using struct syntax (basically : instead of ->), it might make more sense to use attributes-like syntax. (This is probably harder to parse, too.)

[sgrif]

I still don't like rename as the API. It's unclear to me which side is being renamed.

[belst]

I do like the way serde solves this: https://github.com/serde-rs/serde/blob/4a0bf4de65b7787088c6c6ce3cc3b69fc1cc1a16/serde_codegen_internals/src/attr.rs#L109

basically it would look like this:

#[derive(Queryable)]
struct Foo {
    id: i32,
    #[diesel(rename="type")]
    type_: String,
    foo: bool,
}

[sgrif]

So we discussed this feature a bit more internally. The consensus was that there's not really a strong motivation for this feature in terms of general renaming of columns. Without a stronger motivation, we want to avoid introducing new features like this.

What we do need to address are the table/column names that cannot be represented in Rust because they overlap with keywords. (type being the most obviously common one). What we'd like to do in order to solve that problem is take the same approach that bindgen took, and put an underscore at the end of the name if it conflicts with a Rust keyword. This approach is simple, consistent, and easy to document.

[sgrif]

Forgot to mention -- part of the reason for not wanting to introduce this as an API in the table! macro for now is that any code which does successfully compile but doesn't have the name you want can always be renamed in Rust by pub use foo as bar

[kardeiz]

Is it even possible to create a new ident type_ from a given ident type inside macro_rules?

Or (as noted above) if you are requiring the user to provide the type_ ident to map a SQL column "type", how would a user map a column named "type_" (however unlikely a column name that may be)?

In any case, I think I fundamentally disagree with this, and it seems like renaming Rust keywords would add more complexity than this PR (or something similar).

As a potential Diesel user I would prefer to be required to do some configuration rather than work with some relatively arbitrary convention (appending underscore to reserved name).

However, I do agree that the rename-on-use workaround would be acceptable for most cases.

In any case, thanks for considering and for the explanations—

[nambrot]

I'm a newbie to Diesel, but is there any recommended workaround for having columns named type right now?

I'm trying to connect to just a specific table to avoid that problematic column by using infer_table_from_schema! but then get

error[E0412]: cannot find type `Datetime` in this scope
 --> src/schema.rs:2:1

[Fiedzia]

The consensus was that there's not really a strong motivation for this feature in terms of general renaming of columns.

It is necessary when working with existing databases. The table names are often out of touch with reality (or reality of specific application), but cannot be changed, they may be inconsistent and we want consistency in codebase, or were automatically generated and we want human-friendly names for them in our app.
I've seen all those cases and I can't imagine an ORM that can't cope with that. To rephrase: I have no control over a database, and I want to use an ORM. Therefore ORM should be able to fix as many db issues as possible.

any code which does successfully compile but doesn't have the name you want can always be renamed in Rust by pub use foo as bar

This is a) horribly ugly and b) involves looking at (and changing) two places instead of one, while every single ORM I've ever worked with this was always stated once at the place of model definition, Therefore it has the horrible feature of surprising users.