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

Substitute oid in parse #541

Merged
merged 8 commits into from
May 24, 2022
Merged

Conversation

G1gg1L3s
Copy link
Contributor

This PR fixes substitution of OID in packets. Before, it substituted them in the data messages, which are returned by select. But there is another place where they can arrive - in parse packets. We missed it, because they are optional and most frontends do not specify them.

To test it, I've added a new dependency to our python tests - psycopg3. The first stable version was released on 2021-10-13, but I think we can start to use it because of its support of various features. For example in this case, this frontend sets the type of parameters in the parse packets. Also it helped to discover bug with encoding of i32, so in some cases tests could fall. I will fix it as soon as possible.

Checklist

This also brings new dependency - psycopg v3. This is a lot better
version of psycopg2, with new supported features, like extended
and prepared queries. In our case, we are interested in additional
type parameters in a Parse packets, which psyco specifies by default.
tests/test.py Outdated Show resolved Hide resolved
tests/test.py Outdated
@@ -10077,6 +10116,34 @@ async def test():
loop.run_until_complete(test())


class Psycopg3ExecutorMixin:
def setUp(self):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, I see such pattern not first time. So maybe we should introduce some Base class with such approach of initializing executors?) Like BaseExecutorInitialization and require to override get_executor method with proper class or correct configuration?) We can require to set class via class variable and introduce one more method get_executor_connection_args if some child class needs not common args.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't quite understand how you see it. Can you provide an example of usage of such executor?

Beside that, the idea is cool!

tests/test.py Outdated Show resolved Hide resolved
G1gg1L3s and others added 3 commits May 20, 2022 19:20
It is DB_PORT, not TEST_DB_PORT, oops :)
Replace $[0-9]+ with %s and place arguments in the correct order.
@G1gg1L3s
Copy link
Contributor Author

The broken tests are the result of T2568. The fix is at #542

@G1gg1L3s G1gg1L3s mentioned this pull request May 23, 2022
7 tasks
* Extract base class from asyncpg3 mixin

* Move executors to the top

* Add asyncpg executor mixin

* Add mysql executor mixin
@G1gg1L3s G1gg1L3s merged commit dcade01 into master May 24, 2022
@G1gg1L3s G1gg1L3s deleted the G1gg1L3s/T2568-substitute-oid-in-parse branch May 24, 2022 09:54
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