search

lib / pq

Pure Go Postgres driver for database/sql
https://pkg.go.dev/github.com/lib/pq
MIT License
9.01k stars 910 forks source link

implement additional context specific sql interfaces #921

Closed kylejbrock closed 3 years ago

maddyblue commented 4 years ago

This failed with:

=== RUN   TestContextCancelBegin
    TestContextCancelBegin: go18_test.go:212: unexpected error: pq: current transaction is aborted, commands ignored until end of transaction block
--- FAIL: TestContextCancelBegin (0.02s)

Looks like this has bugs.

kylejbrock commented 4 years ago

Yea. I’ve not had time to look into why. I tried testing it locally and it passed. So, it’ll require some deeper investigation.

On Dec 10, 2019, at 3:16 PM, Matt Jibson notifications@github.com wrote:

This failed with:

=== RUN TestContextCancelBegin TestContextCancelBegin: go18_test.go:212: unexpected error: pq: current transaction is aborted, commands ignored until end of transaction block --- FAIL: TestContextCancelBegin (0.02s) Looks like this has bugs.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/lib/pq/pull/921?email_source=notifications&email_token=ABG7PMWEYS4TORKBBQVVM73QYABJ5A5CNFSM4JULIIIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGQ7JIA#issuecomment-564262048, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABG7PMVCZXGX6AHYH7FBCPDQYABJ5ANCNFSM4JULIIIA.

maddyblue commented 4 years ago

You may need to stress it locally. Could be a race condition.

autarch commented 4 years ago

Oops, ignore that PR I linked against this one. I meant to link to a different one.

jalan commented 4 years ago 
=== RUN   TestContextCancelBegin
    TestContextCancelBegin: go18_test.go:212: unexpected error: pq: current transaction is aborted, commands ignored until end of transaction block
--- FAIL: TestContextCancelBegin (0.02s)

The failure here seems unrelated to this PR. I spent some time trying to reproduce the failure and was unable to do so with either this PR or master. I did find one other failure for this test on Travis, https://travis-ci.org/lib/pq/jobs/355557649, so maybe the test itself has a race.

Spidey sense says this is problematic. Way too many go routines and chan listens to make me feel safe here.

I agree that this code looks scary! But it's the same code as used here https://github.com/lib/pq/blob/master/conn_go18.go#L90

I have been test-driving this PR on a real project in a test environment, and I also wrote a couple of toy servers to check that the context cancellation works as I expect. Everything so far looks good :+1: