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

Tables whos primary key isn't called ID #55

Open
cursey opened this issue Apr 12, 2018 · 8 comments
Open

Tables whos primary key isn't called ID #55

cursey opened this issue Apr 12, 2018 · 8 comments
Labels
help wanted Feel free to contribute! proposal A suggestion for a change, feature, enhancement, etc s: triage Some tests need to be run to confirm the issue

Comments

@cursey
Copy link

cursey commented Apr 12, 2018

Hi,

I'm new to both Go and Buffalo. I don't know what to do when trying to interface with an existing database who's tables primary key's aren't called ID. For instance there is a user table and the primary key is called user_id. From what I can tell pop.Query.Find is the responsible function because it's inserting id as a hard-coded string. Maybe there could be a way of overriding the primary key name like there is a way of overriding the table name by implementing TableNameAble?

Thanks!

Edit: Looking further I guess I can just use pop.Query.Where instead. Is that the proper way of handling this?

@stanislas-m
Copy link
Member

Hello @cursey,

You're right, right now this one is hard-coded, but should support an override.

In the meanwhile, you can use Where instead, since Find is mostly a WHERE id = ? and a First to get the only row.

@stanislas-m stanislas-m added enhancement help wanted Feel free to contribute! labels Apr 12, 2018
@petar-dambovaliev
Copy link
Contributor

petar-dambovaliev commented May 14, 2018

#97

@petar-dambovaliev
Copy link
Contributor

petar-dambovaliev commented May 18, 2018

@markbates @stanislas-m Some strategy needs to be laid out for these id mapping issues.
Are they going to be done individually or all in a single enhancement?
I can see pros and cons of both.
Too many changes at once is risky.
On the other hand, if would be confusing and possibly breaking for users who have models that map their ID field for the query but it isn't for the associations.

Also, we need a better test coverage of the associations.

@nithin-bose
Copy link

I am using postgres and I am getting the pq: column "id" does not exist. The following is my model:

type SellOrder struct {
	ID            uuid.UUID `json:"seller_order_id" db:"seller_order_id"`
	CreatedAt     time.Time `json:"created_at" db:"created_at"`
	Quantity      float64   `json:"quantity" db:"quantity"`
}

I have tried using the db tag to specify the correct column name however I dont think the tag is taken into consideration.

@petar-dambovaliev
Copy link
Contributor

@nithin-bose No, the tag on the ID is not taken into consideration. It is hardcoded currently.

@stanislas-m stanislas-m added proposal A suggestion for a change, feature, enhancement, etc and removed proposal A suggestion for a change, feature, enhancement, etc enhancement labels Oct 31, 2018
@KrzysztofMadejski
Copy link

Some strategy needs to be laid out for these id mapping issues.
Are they going to be done individually or all in a single enhancement?

I'd vote for a primary_key struct tag that can override defaults. That way it could be set in one place instead of using that in every Find. It could also hlp with migrations generation (is it a feature?)

On the other hand, if would be confusing and possibly breaking for users who have models that map their ID field for the query but it isn't for the associations.

Could you elaborate @petar-dambovaliev ?

@MaMrEzO
Copy link

MaMrEzO commented Dec 29, 2019

Hi there
As pop documentation`s IDField :

/* pop/model.go:55 */
// IDField returns the name of the DB field used for the ID.
// By default, it will return "id".
func (m *Model) IDField() string {
	field, ok := reflect.TypeOf(m.Value).Elem().FieldByName("ID")
	if !ok {
		return "id"
	}
	dbField := field.Tag.Get("db")
	if dbField == "" {
		return "id"
	}
	return dbField
}

It would respect Db design BUT what I found in codes pop/dialect_postgresql.go:60:

/*  pop/dialect_postgresql.go:60 */
		if len(w.Cols) > 0 {
			query = fmt.Sprintf("INSERT INTO %s (%s) VALUES (%s) returning id", p.Quote(model.TableName()), w.QuotedString(p), w.SymbolizedString())
		} else {
			query = fmt.Sprintf("INSERT INTO %s DEFAULT VALUES returning id", p.Quote(model.TableName()))
		}

BUT SHOULD BE: ... returning %s", p.Quote(model.TableName()), p.Quote(model.IDField())...

		if len(w.Cols) > 0 {
			query = fmt.Sprintf("INSERT INTO %s (%s) VALUES (%s) returning %s", p.Quote(model.TableName()), w.QuotedString(p), w.SymbolizedString(), p.Quote(model.IDField()))
		} else {
			query = fmt.Sprintf("INSERT INTO %s DEFAULT VALUES returning %s", p.Quote(model.TableName()), p.Quote(model.IDField()))
		}

Not event "dialect_postgresql.go" but:

dialect_postgresql.go:61:                       query = fmt.Sprintf("INSERT INTO %s (%s) VALUES (%s) returning id", p.Quote(model.TableName()), w.QuotedString(p), w.SymbolizedString())
dialect_postgresql.go:63:                       query = fmt.Sprintf("INSERT INTO %s DEFAULT VALUES returning id", p.Quote(model.TableName()))
dialect_cockroach.go:75:                        query = fmt.Sprintf("INSERT INTO %s (%s) VALUES (%s) returning id", p.Quote(model.TableName()), w.QuotedString(p), w.SymbolizedString())
dialect_cockroach.go:77:                        query = fmt.Sprintf("INSERT INTO %s DEFAULT VALUES returning id", p.Quote(model.TableName()))

Anyone can do a favor to me, and tell me how to change them locally till these updates done?

@MaMrEzO
Copy link

MaMrEzO commented Dec 29, 2019

After some googling finally found that (git init, git add .) able me to change files!
I can confirm that the above code (my previous comment) works as expected!

This will works:

type SellOrder struct {
	ID            uuid.UUID `json:"seller_order_id" db:"seller_order_id"`
	CreatedAt     time.Time `json:"created_at" db:"created_at"`
	Quantity      float64   `json:"quantity" db:"quantity"`
}

@sio4 sio4 added this to the Proposal milestone Sep 20, 2022
@sio4 sio4 added the s: triage Some tests need to be run to confirm the issue label Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Feel free to contribute! proposal A suggestion for a change, feature, enhancement, etc s: triage Some tests need to be run to confirm the issue
Projects
None yet
Development

No branches or pull requests

7 participants