vapor-community/postgresql-driver

Seems that the id could only be int?

hyouuu opened this issue · 7 comments

I tried with a string in model like this, to make the id string typed "natural key":

init(node: Node, in context: Context) throws {
       id = try node.extract("id") ?? uid().makeNode()

Then I got this error when triggering that:

Uncaught Error: DatabaseError.invalidSQL("ERROR:  invalid input syntax for integer: \"IC719W\"\n"). Use middleware to catch this error and provide a better response. Otherwise, a 500 error page will be returned in the production environment

and looking at the SQL table, id column is int type

In fluent schema, I see this:

public enum DataType {
            case id
            case int
            case string(length: Int?)
            case double
            case bool
            case data
        }

and seems id is a special type. Is it possible to support varchar as id type? If not, any reason not to? Thanks!

The driver determines which type is most appropriate for the Fluent id type. For MySQL and Mongo they have specific types that the ID must be.

For postgres I'm not sure if that is the case. Can you make a Varchar column the primary key and tie foreign relations to it?

I'm not expert on Postgres at all, but my impression is that using varchar as primary id has been supported by MySQL and Postgres for many years. Couldn't directly find the doc on that, but from the threads it seems it's already there: https://www.postgresql.org/message-id/5A03DE8B-8FF6-4CFE-95AD-D9019437462C@decibel.org https://www.postgresql.org/message-id/CAH_wARuc1cLVCE49F3sPErBMkWYnMgi2tqBMGD22HdG1RDCybA@mail.gmail.com

mz2 commented

Yeah, I don't think there's a database level constraint there for PostgreSQL for using primary keys of those types. I've been able to use a field with the name "id" of type string (so when creating the schema I used creator.string instead of creator.id. Things seem to work for my simple needs at least now.

I did have to make the following modification to the query method in PostgreSQLDriver (see the code after my added comment) to make query successfully ever return the primary ID of the record that was inserted:

switch query.action {
        case .create:
            // fetch the last inserted value
            do {
                let lastval = try _execute("SELECT LASTVAL();", [], connection)
                // check if it contains an id
                if let id = lastval[0, "lastval"]?.int {
                    // return the id to fluent to
                    // be the model's new id
                    return .number(.int(id))
                }
            }
            catch PostgreSQL.DatabaseError.invalidSQL(message: _) {
                // When receiving a invalidSQL error, there is basically no LASTVAL, so ignore.
            }
            
            // I added this bit to return the id from the sent data
            // if we failed to get it via LASTVAL (which would always fail for non-integral id's) - Matias P:
            switch query.sql {
            case .insert(_, let data):
                if let id = data?["id"] {
                    return id
                }
            default: ()
            }
            // no id found, return whatever
            // the results are
            return result
        default:
            // return the results of the query
            return result
        }|

This is obviously not yet quite production grade solution, welcome for ideas for how to expand / change approach for something I could PR.

mz2 commented

I should say, another related issue on the same theme of "id could only be int" is that this route of using creator.string("id", unique: …) doesn't allow for actually marking the field a primary key, so it's definitely still a hack, more pointing that this works because I was surprised not many more obvious things didn't obviously break.

vzsg commented

The integer primary key is actually coming from the general SQL serializer, which is extended, but not overridden by the concrete SQL drivers.

mz2 commented

Yeah I guess the more appropriate way to describe the limitation that I've now tried lifting in vapor/fluent#165 is from having a single identifier kind per DB driver to allowing optionally a hint to be given per case.

As of the 2.0 beta the driver now supports using UUID for the primary key as well as Int.

https://github.com/vapor-community/postgresql-driver/blob/master/Sources/PostgreSQLDriver/PostgreSQLSerializer.swift#L71