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

Field tags now allow you to declare a primary key or autoincrement field #26

Closed
wants to merge 1 commit into from

Conversation

meetmauro
Copy link
Contributor

With this patch you can declaratively say:

type Product struct {
Id int64 db:"product_id" flags:"pk, autoincrement"
Price int64 db:"unit_price"
IgnoreMe string db:"-"
}

so that your AddTableWithName will be simpler like:
t1 := dbmap.AddTableWithName(Invoice{}, "invoice_test")

instead of using SetKeys to specify keys on fields.

@coopernurse
Copy link
Contributor

Apologies I haven't commented. I like the idea, but I wonder if we should use the same "db" tag for all of these attributes, rather than introducing another tag. For example:

type Product struct {
    Id int64 db:"product_id,pk,autoincrement"
    Price int64 db:"unit_price"
    IgnoreMe string db:"-"
}

That would follow the convention used in the encoding/json package (and probably others).

Thoughts?

@coopernurse
Copy link
Contributor

I should clarify: the first item in the comma separated list would always be the column name, similar to the way the json tag works.

@sqs
Copy link
Contributor

sqs commented Jun 23, 2013

+1 to using the same tag name, following the convention of encoding/json (which, by the way, has some unexported helper functions you could copy to parse the tag).

@lukescott
Copy link

I would personally prefer "name,autoincrement" and "name,primary" over "name,pk,autoincrement" and "name,pk". In the case of "autoincrement", "primary" would naturally be assumed.

@insasho
Copy link

insasho commented Feb 12, 2014

@lukescott auto-increment columns are not always primary keys.

@GeertJohan
Copy link
Member

@meetmauro Can you see if the requested changes can be made to this PR? I'd be happy to add this feature to the codebase.
If so, please:

  • Rebase the PR on the latest master
  • Use attributes in the same db tag
  • Change pk to primarykey

@meetmauro
Copy link
Contributor Author

OK. Rebased and made suggested changes in a new PR.

@GeertJohan
Copy link
Member

New PR: #233

@GeertJohan GeertJohan closed this Feb 26, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants