search

lib / pq

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

implement ConnPrepareContext/StmtQueryContext/StmtExecContext interfaces #1047

Closed michaelshobbs closed 2 years ago

michaelshobbs commented 3 years ago

See #1046 for more context. (pun intended)

This is almost a line for line copy of #921 with test cases (thanks @kylejbrock). Please let me know what else needs to be done to get this merged.

Also dropped ci testing of unsupported versions (9.5 and 9.4) per pg docs here and added ci testing of versions 11, 12, and 13.

closes #921 closes #1046

michaelshobbs commented 3 years ago

@otan @mjibson please let me know if i can be of any more assistance in getting this merged

michaelshobbs commented 2 years ago

@otan @mjibson i know pgx is the preferred path forward. however, i also know there are many folks still using pq and this addition would certainly be a value add for those users. please let me know how i can help move this forward.

thank you 🙏

otan commented 2 years ago

well looks like CI no longer works :\

michaelshobbs commented 2 years ago

Yea looks like they deprecated travis-ci.org in favor of travis-ci-com. not sure what needs to happen to move this project over. Let me know if I can help.

michaelshobbs commented 2 years ago

Looks like all you have to do is sign up on the .com?

https://blog.travis-ci.com/2021-05-07-orgshutdown

otan commented 2 years ago

i've tried that and resent the webhooks but no bueno, perhaps try git commit --amend --reset-author -a and git push --force your commit to try retrigger it.

i don't have admin permissions to debug this further unfortunately (travis-ci.com doesn't show this repo when i try configure it there), may have to move to a new CI system. /shrug

michaelshobbs commented 2 years ago

I was able to add my fork here: https://travis-ci.com/github/michaelshobbs/pq/builds/235721359

I assume that's because I own it. Who is still an admin on this repo? 😬

michaelshobbs commented 2 years ago

@otan where did we end up and is there anything i can do to help?

otan commented 2 years ago

You'd need to find who the admin is :) Idk who it is. Maybe @mjibson knows.

maddyblue commented 2 years ago

I do not!

otan commented 2 years ago

Well I think the answer to get CI working (and for me to have confidence in merging) we'd need to migrate to Github Actions. I don't have the time to do this, such is maintenance life :\

maddyblue commented 2 years ago

FYI I previously tried really hard to make github actions work at https://github.com/lib/pq/commit/8cec54691dd09a5323a614565c97d6622dcad10d. I was never able to get the pg initial configuration files correctly installed before the pg server started running. But that commit is like almost all there.

cbandy commented 2 years ago

I was never able to get the pg initial configuration files correctly installed before the pg server started running.

It looks like the scripts should be mounted at /docker-entrypoint-initdb.d. Those scripts can copy the certs, write conf, createdb, etc. It looks like you can rely on $PGDATA rather than hard-coding /var/lib/postgresql/data, too.

michaelshobbs commented 2 years ago

@otan @mjibson i'll give this a look this week

michaelshobbs commented 2 years ago

@otan @mjibson can one of you approve me for running workflows?

otan commented 2 years ago

gotcha

michaelshobbs commented 2 years ago

Sweet! I'll start working on this in the morning

DoubleDi commented 2 years ago

Hi! Thank you very much for this PR. Just found out our code has the same problems with cancelling requests from prepared statements. Would be awesome to get this into master ASAP.

DoubleDi commented 2 years ago

Quick update @michaelshobbs. We have tested your patch (just replaced the go.mod to your fork) and it seems still not to work on prepared statements.

Our usecase: add 10sec timeout to context from and send a db query on a prepared statement which last longer, than 1 minute.

michaelshobbs commented 2 years ago

@DoubleDi can you provide some code that demonstrates the incorrect behavior? this PR includes canceling statement due to user request test assertions on stmt.QueryContext(), and stmt.ExecContext(). i'd like to better understand where we might be missing something here. thanks 🙏

DoubleDi commented 2 years ago

@michaelshobbs I am not exactly sure, but in some cases, this works. I can see the canceling statement due to user request error in the logs. But for some reason, we still have long queries, maybe it's an internal problem. I will let you know if I find out anything.

michaelshobbs commented 2 years ago

@otan @mjibson https://github.com/lib/pq/pull/1054

michaelshobbs commented 2 years ago

@otan @mjibson rebased master. this should be good to go now

DoubleDi commented 2 years ago

@otan thanks! Please also tag a new release

otan commented 2 years ago

v1.10.3, you have to be a tad more patient than 90s :P

DoubleDi commented 2 years ago

thank you so much!