cornucopia-rs / cornucopia

Generate type-checked Rust from your PostgreSQL.
Other
759 stars 31 forks source link

fix: Escape enum members containing reserved keywords #159

Closed jacobsvante closed 1 year ago

jacobsvante commented 1 year ago

Just stumbled upon an issue where one of the enum members of an existing postgres enum was box. Cornucopia happily generated the cornucopia.rs file, but the result was a file with a syntax error in it.

So here comes my quick-fix to that.

This PR is not meant to be merged, just demonstrating one way to resolve the issue. I'm a bit short on time so if anyone with more intimate knowledge of the codebase wants to do it properly, then please go ahead 😉

jacobsvante commented 1 year ago

#[allow(non_camel_case_types)] needs to be added to pub mod types, to avoid warnings.

jacobsvante commented 1 year ago

Hmmm. It seems that postgres/tokio-postgres does not like enums with raw identifiers. Gives me the following error when I try to execute a SELECT with the given column included:

thread 'main' panicked at 'error retrieving column 9: error deserializing column 9: cannot convert between
the Rust type `data_import::cornucopia::types::public::Uom` and the Postgres type `uom`',
/Users/jacob/.cargo/registry/src/github.com-1ecc6299db9ec823/tokio-postgres-0.7.7/src/row.rs:151:25

The enum looks like this:

        pub enum Uom {
            pair,
            pcs,
            r#box,
        }

If I do ALTER TYPE uom RENAME VALUE 'box' TO 'Box' and re-run cornucopia, the issue disappears.

Virgiel commented 1 year ago

Postgres enum are case sensitive which explain the error you saw.

We have two ways of dealing with these problematic cases. Our current method is to error when finding a rust keyword. We do this for tables, and we should do it for enums. It would be better to handle them elegantly by escaping them, which takes more work but would be better for usability.

For the next version, we should cause an error, and we could start working on supporting them for a later version.

jacobsvante commented 1 year ago

Replaced by more thorough version in #161