-
Notifications
You must be signed in to change notification settings - Fork 1
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
implement additional context specific sql interfaces #4
Conversation
} | ||
|
||
func (st *stmt) cancel() error { | ||
return st.cn.cancel() |
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.
Discussed this offline - we need to double check that this cancel
function is thread safe. It is a very real possibility that it is called multiple times since watchCancel
spawns multiple go routines without stopping/killing them
Well this has been working fine in prod for a while. Is there any other vetting you want to do @bstein-clever |
I'm fine with 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.
@bstein-clever before you merge can you make sure you're happy with dd5efc1
This removes references to the root lib/pq
we forked from. I think it's a decent protection to have, but I'm not sure if it makes this fork that much harder to maintain for the build protection it gives us
Cherry-picked from here: lib#768