-
Notifications
You must be signed in to change notification settings - Fork 46
Fix issues with @Selection-based updates #233
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
Fix issues with @Selection-based updates #233
Conversation
| assertInlineSnapshot( | ||
| of: Root.update { | ||
| $0.fields.honestCount = honestValue | ||
| //$0.fields.optionalCount = honestValue |
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.
compile error here
| ) { | ||
| """ | ||
| UPDATE "roots" | ||
| SET "optionalCount" = "nestedFieldses"."optionalCount", "honestCount" = "nestedFieldses"."honestCount" |
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.
using selections table name instead of root's table name
|
@stephencelis I cannot figure out how to fix, but here are two failing tests for you. |
stephencelis
left a comment
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 for taking the lead on sharing failing tests. I've gone ahead and explored the issues and believe I have a workaround for one and a fix for another.
| assertInlineSnapshot( | ||
| of: Root.update { | ||
| $0.fields.honestCount = honestValue | ||
| $0.fields.optionalCount = #bind(honestValue) |
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.
@rcarver I'm not sure if I mentioned before, but #bind is the best way to coerce a query expression's underlying value. You could also have done Optional(honestValue) here.
While I think it's possible to add overloads to fix this. It's a pervasive problem and I'm not sure overloads are the solution...open to discuss, though!
| ) { | ||
| """ | ||
| UPDATE "roots" | ||
| SET "optionalCount" = "roots"."optionalCount", "honestCount" = "roots"."honestCount" |
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.
This should now be fixed! Thanks for the test case!
| public struct _TableAliasName<Base: Table>: AliasName { | ||
| public static var aliasName: String { Base.tableName } | ||
| } |
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.
This is the simplest way I could come up with to fix the nested update issue. Underscored type for now since I'm not sure folks should be using it, especially if we come up with a better way to handle things in the future. But if we come up with more use cases for this type, we could consider making a proper public API.
Tests for a few issues I've found while using @selection, fixes #232