pop icon indicating copy to clipboard operation
pop copied to clipboard

Tables whos primary key isn't called ID

Open cursey opened this issue 8 years ago • 8 comments

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?

cursey avatar Apr 12 '18 10:04 cursey

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 avatar Apr 12 '18 11:04 stanislas-m

https://github.com/gobuffalo/pop/pull/97

petar-dambovaliev avatar May 14 '18 20:05 petar-dambovaliev

@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.

petar-dambovaliev avatar May 18 '18 11:05 petar-dambovaliev

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.

nithin-bose avatar May 19 '18 07:05 nithin-bose

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

petar-dambovaliev avatar May 20 '18 17:05 petar-dambovaliev

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 ?

KrzysztofMadejski avatar Sep 26 '19 16:09 KrzysztofMadejski

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 avatar Dec 29 '19 02:12 MaMrEzO

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"`
}

MaMrEzO avatar Dec 29 '19 03:12 MaMrEzO