go-reform / reform

A better ORM for Go, based on non-empty interfaces and code generation.
https://gopkg.in/reform.v1
MIT License
1.44k stars 73 forks source link

Question: What reform uses for UUID support #202

Open rumyantseva opened 5 years ago

rumyantseva commented 5 years ago

When my model has a UUID-based field and when I do reform models I have the following situation:

  1. Reform generates the code but doesn't add anything to imports. Personally, I use github.com/satori/go.uuid for uuid. I wonder, if I can add something to reform to be able to support this import automatically.

  2. Reform generates the following method:

    // SetPK sets record primary key.
    func (s *Profile) SetPK(pk interface{}) {
    if i64, ok := pk.(int64); ok {
        s.ID = uuid.UUID(i64)
    } else {
        s.ID = pk.(uuid.UUID)
    }
    }

    As nothing has been added to imports, I wonder which library supports uuid.UUID(i64) (as this is not what satori/go.uuid can offer, it gives me cannot convert i64 (type int64) to type uuid.UUID). I found a way to make my code working:

func (s *Profile) SetPK(pk interface{}) {
    s.ID = pk.(uuid.UUID)
}

But I wonder if this is a suitable approach and how can I improve the whole story.

AlekSi commented 5 years ago

See https://github.com/go-reform/reform/issues/43 and https://github.com/go-reform/reform/issues/56.

AlekSi commented 5 years ago

I responded in the other issue, but want to highlight one thing there:

func (s *Profile) SetPK(pk interface{}) {
    s.ID = pk.(uuid.UUID)
}

You could replace that code with:

func (s *Profile) SetPK(pk interface{}) {
    panic("should not be used")
}

SetPK is used only with sql.Result.LastInsertId that only works for integer autogenerated keys.

rumyantseva commented 5 years ago

@AlekSi thanks! Actually, I was pretty sure that pq doesn't support LastInsertId at all:

pq does not support the LastInsertId() method of the Result type in database/sql. To return the identifier of an INSERT (or UPDATE or DELETE), use the Postgres RETURNING clause with a standard Query or QueryRow call

(from https://godoc.org/github.com/lib/pq, also: https://github.com/lib/pq/blob/51e2106eed1cea199c802d2a49e91e2491b02056/conn.go#L694)

So, do you mean that for the serial type LastInsertId should work even with lib/pq?

AlekSi commented 5 years ago

Yes, PostgreSQL uses INSERT … RETURNING id syntax. SetPK method is never used by reform.

rumyantseva commented 5 years ago

@AlekSi what do you mean "never used by reform"? (: https://github.com/go-reform/reform/blob/c8eea006559b6f50a00404d9f81b191da8fd02cc/querier_commands.go#L97

AlekSi commented 5 years ago

… for PostgreSQL. Reform always uses the code above that line for PostgreSQL.

rumyantseva commented 5 years ago

@AlekSi ok, then I agree :) (but, for example, it's not possible to simply remove SetPK method from the code generated for the model)

AlekSi commented 5 years ago

That's why this and other tickets are still open.