Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Insert Select With Conflict for postgres #155
Insert Select With Conflict for postgres #155
Changes from 2 commits
cedc79d
1b51722
f0cbbc6
236e5b0
1c7d459
6414a12
e62d350
2cb98e9
ff3b5d5
0dc3af9
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've used a trick like this before that matches on the final result type of a value - see
KnowResult
here which allows you to only provide the data constructor. With another type class, you can even pass in the requiredundefined
s automatically so that the user is not concerned with this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! nice recommendation, I have not use type families much, but seeing
KnowResult
did help me avoid the undefined, though I think I should moveKnowResult
to another place (I copyKnowResult
to Database.Esqueleto.PostgreSQL), any recommendations on where to place it and if it is possible to make the firm for insertSelectWithConflict prettier?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
head . filter p === find p
It would be nice to have a function
toUniqueDef :: PersistEntity record => Unique record -> UniqueDef
, but this is basically how I'd implement it 🤷♂️There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
smart guy, I always forget
find
exists, but it keeps being a list search, but I think it should not be too difficult to addtoUniqueDef
on persist, though I will have to look at the code to be sure.Also, the thing with the undefined, I would also like it to be automatic, but the Unique creation function may have any amount of parameters, when defining a unique in persist we can:
so I can't just change the firm to
(a -> Unique val) -> SqlQuery (SqlExpr (Insertion val)) -> (SqlExpr (Entity val) -> SqlExpr (Entity val) -> [SqlExpr (Update val)]) -> SqlWriteT m Int64
because I would not be able to useUniqueFooNum23
on this new function, if there is a way receive a variable parameter function and then fill all parameters with undefined I will add it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I linked to
KnowResult
in another comment. You can implement a type class that will "fill in" the unused values withundefined
also. It's a bit of a trick of type class programming so if you'd like help on that please let me know :)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice, thanks for the tests!