Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Question: What reform uses for UUID support #202

Open
rumyantseva opened this issue Apr 23, 2019 · 8 comments
Open

Question: What reform uses for UUID support #202

rumyantseva opened this issue Apr 23, 2019 · 8 comments

Comments

@rumyantseva
Copy link
Collaborator

rumyantseva commented Apr 23, 2019

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
Copy link
Member

AlekSi commented Apr 23, 2019

See #43 and #56.

@AlekSi
Copy link
Member

AlekSi commented Apr 28, 2019

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
Copy link
Collaborator Author

@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
Copy link
Member

AlekSi commented Apr 29, 2019

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

@rumyantseva
Copy link
Collaborator Author

@AlekSi what do you mean "never used by reform"? (:

record.SetPK(id)

@AlekSi
Copy link
Member

AlekSi commented Apr 30, 2019

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

@rumyantseva
Copy link
Collaborator Author

@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
Copy link
Member

AlekSi commented Apr 30, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants