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.

Sign up for GitHub

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

Jump to bottom

Allow model upsertion #324

Merged
merged 1 commit into from
Dec 16, 2020
Merged

Allow model upsertion #324

merged 1 commit into from
Dec 16, 2020

Conversation

frrist
Copy link
Member

@frrist frrist commented Dec 15, 2020

closes #292

@frrist frrist self-assigned this Dec 15, 2020
@frrist frrist force-pushed the frrist/model-upsert branch 2 times, most recently from 80a9fc4 to 02ab6ff Compare December 15, 2020 22:49
@frrist frrist added the kind/feature New feature request label Dec 15, 2020
@frrist frrist requested a review from iand December 15, 2020 22:50
@iand
Copy link
Contributor

iand commented Dec 16, 2020

Love this. Will take a proper look in the morning.

iand
iand approved these changes Dec 16, 2020
View reviewed changes
Copy link
Contributor

@iand iand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this, it's nice and clean and gives an opt-in to upserting. I made a suggestion about parsing the pg struct tags with go-pg but I'm happy to leave it up to you.

storage/sql.go Outdated
var ucf []string

// walk all exported fields in the model
for name := range st.Map() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be safer to use go-pg's orm to parse the pg struct tags. We don't need to keep in sync with any modifiers that are introduced. That assumes go-pg exports all the information we need here.

Copy link
Member Author

@frrist frrist Dec 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're right, I didn't explore go-pg very much wrt this (I probably should have started there :/ ). I'll take another look.

@frrist frrist force-pushed the frrist/model-upsert branch 2 times, most recently from 8294249 to 84128e7 Compare December 16, 2020 20:40
@frrist frrist force-pushed the frrist/model-upsert branch from 84128e7 to b7a48ba Compare December 16, 2020 20:52
@frrist frrist merged commit 442655f into master Dec 16, 2020
@frrist frrist deleted the frrist/model-upsert branch December 16, 2020 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@iand iand iand approved these changes

Assignees

@frrist frrist

Labels
kind/feature New feature request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support upserts
2 participants
@frrist @iand