Skip to content
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

added Do method for raw query by single conn from pool.Conn() #3182

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Oleglacto
Copy link

No description provided.

@Oleglacto
Copy link
Author

Hello! I want to call Do method when I got Conn from redis.Pool. This PR can help me do this

@Oleglacto
Copy link
Author

@vmihailenco can u make review please?

@ndyakov
Copy link
Collaborator

ndyakov commented Feb 20, 2025

@Oleglacto can you please describe your usecase and add tests?

@Oleglacto
Copy link
Author

@ndyakov It is necessary to execute a raw query on a connection from the pool. Currently, this capability is not available, and workarounds such as Get, Set, etc., have to be used.

@ndyakov
Copy link
Collaborator

ndyakov commented Feb 21, 2025

@Oleglacto on top of my head i cannot think why this should not be possible. Any ideas? How will this behave if you are working with a connection that is in a pub/sub state?

p.s. Also the test that is added is failing cc @Oleglacto

@ndyakov ndyakov self-requested a review February 21, 2025 11:59
@Oleglacto
Copy link
Author

Oleglacto commented Feb 28, 2025

@ndyakov Thanks for waiting. Type Conn from redis.go doesn't support pub/sub. I added this method for cmdble interface and it should work fine

I will trying to fix my test.

upd. fixed

@Oleglacto Oleglacto requested a review from ndyakov March 3, 2025 10:37
@ndyakov
Copy link
Collaborator

ndyakov commented Mar 4, 2025

@Oleglacto any specific reason why this is not in commands.go ? I don't see the need to add additional file for it.

@Oleglacto
Copy link
Author

I created a new file because I didn't know where to add the changes.

I'll move it now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants