Allow raw identifiers for SqlIdentifier (column-name)

[z33ky]

This allows using the r#identifier syntax for SqlIdentifier (column-name), which is used in derive macros.

table! {
    things {
        id -> Integer,
        r#identifier -> Integer,
    }
}

#[derive(diesel::Queryable)]
struct Thing {
    id: i32,
    r#identifier: i32
}

Previously the derive macros would panic when encountering such an identifier:

`"r#identifier"` is not a valid identifier

[weiznich]

Thanks for opening this PR. I think the change itself looks fine, but we want to have some tests that show that everything is parsed correctly.

[z33ky]

Sure. I added it for the Queryable test, since that probably the simplest. Is that sufficient?
Trying to run the tests locally didn't work, I'm getting lots of compiler errors (also without my change). Here's to hoping it runs just like that :D

[weiznich]

I think I would prefer to have these kind of test for Insertable, AsChangeset, Selectable and QueryableByName as well.

[z33ky]

Alright, I added raw identifiers to the respective test cases.
Sorry for the force-push spam due to my local setup being unable to execute the tests locally.
The spell-check complains about the Spanish "tipe", though Spanish is also used for the other names of that struct. I'm not sure what's up with that or how I would go about solving this.

[weiznich]

No worries about the force pushes, that's fine.

For the spell checker: You can just add an exception here: https://github.com/diesel-rs/diesel/blob/master/.typos.toml#L8

As for running the tests: There is a bash script in bin/test that should allow to execute all tests as long as you've installed the build dependencies and configured the required database urls.

[z33ky]

As for running the tests: There is a bash script in bin/test that should allow to execute all tests as long as you've installed the build dependencies and configured the required database urls.

Ah thanks, that works.

[z33ky]

QueryableByName perhaps poses an interesting question. Should the query indeed contain `r#type` to match or should the SQL query contain type and map to the raw identifier r#type?

[weiznich]

I would expect type there.

[Ten0]

I feel like the above QueryableByName should try to read type, not r#type.

[Ten0]

I think to be consistent with previous comment we probably should search for type as name if passing the r#type ident

and ideally also allow passing column_name = "some arbitrary string" where we would then parse 3 as "some arbitrary string" even if some arbitrary string is not an ident to support other weird things users might want. (I'm not sure if that is already supported and it's not a blocker here)

[z33ky]

I have lifted the restriction on column_name insofar, that a raw identifier is now allowed, however it must still correspond to a valid Rust identifier.
The previous error-message for invalid names is retained, though it now emits an unhelpful

error: Expected valid identifier, found `ty pe`. Diesel automatically renames invalid identifiers, perhaps you meant to write `ty pe_`?
   --> diesel_derives/tests/as_changeset.rs:287:32
    |
287 |         #[diesel(column_name = "ty pe")]
    |                                ^^^^^^^

The error-message should probably use inference::rust_name_for_sql_name()?