vapor / fluent-postgres-driver

🐘 PostgreSQL driver for Fluent.
MIT License
146 stars 53 forks source link

Change default postgreSQLEnumTypeName to include full type path #83

Closed clayellis closed 6 years ago

clayellis commented 6 years ago

This PR changes the default postgreSQLEnumTypeName by adding the entire type's path to the name.

Example:

Consider the following case:

class Foo {
    var kind: Kind

    enum Kind: PostgreSQLEnum {
        ...
    }
}

class Bar {
    var kind: Kind

    enum Kind: PostgreSQLEnum {
        ...
    }
}

The two classes, Foo and Bar, each have a nested enum called Kind, each a PostgreSQLEnum.

Currently the default postgreSQLEnumTypeName for both of these nested enums is the same: "KIND". Meaning that the resulting Postgres ENUM type for the first would be overridden by the second with no indication to the user that this happened.

This PR would resolve that nuance by changing the default postgreSQLEnumTypeName to return unique names: "FOO_KIND" and "BAR_KIND".

Motivation:

This change is related to an issue discussed here. The motivating factor being, that in Swift, the types Foo.Kind and Bar.Kind are unique and distinguishable by the type system. It should then be reasonable to expect the default postgreSQLEnumTypeNames to be unique and the resulting Postgres ENUM types to be unique and distinguishable whereas currently, the first ENUM type created will be overridden by the second. This is counter-intuitive behavior, because it should be expected that two unique Swift enums will result in two unique Postgres ENUMs. In order to fully resolve the linked issue, and do so by default, this is a necessary change and will have a companion PR at Fluent to update the default migrationName (https://github.com/vapor/fluent/pull/544).

Impact:

This could potentially have an impact on existing codebases by changing postgreSQLEnumTypeNames that are already in place.

penny-coin commented 6 years ago

Hey @clayellis, you just merged a pull request, have a coin!

You now have 15 coins.