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

Perf: Enable cache, fix value casting for MySQL and MariaDB and add plugin for Engine creation #529

Merged
merged 11 commits into from
Jan 9, 2025

Conversation

adrien-berchet
Copy link
Member

@adrien-berchet adrien-berchet commented Jan 4, 2025

Description

Fixes #436
Cache could not be enabled because of conversion performed in compilation step. Now this conversion is performed during the before_cursor_execute event.
A SQLAlchemy plugin was created for the sqlalchemy.Engine objects in order to automatically attach the relevant events based on the dialect of the Engine.

Checklist

This pull request is:

  • A documentation / typographical error fix
    • Good to go, no issue or tests are needed
  • A short code fix
    • Please include: Fixes: #<issue number> in the description if it solves an existing issue
      (which must include a complete example of the issue).
    • Please include tests that fail with the main branch and pass with the provided fix.
  • A new feature implementation or update an existing feature
    • Please include: Fixes: #<issue number> in the description if it solves an existing issue
      (which must include a complete example of the feature).
    • Please include tests that cover every lines of the new/updated feature.
    • Please update the documentation to describe the new/updated feature.

@adrien-berchet adrien-berchet marked this pull request as draft January 4, 2025 20:27
@adrien-berchet
Copy link
Member Author

To make things simpler we should add a general function like enable_geoalchemy(engine) that would listen for proper events depending on the dialect of the engine.

@adrien-berchet
Copy link
Member Author

To make things simpler we should add a general function like enable_geoalchemy(engine) that would listen for proper events depending on the dialect of the engine.

This was made using a plugin for Engine objects.

@adrien-berchet adrien-berchet changed the title Perf: Fix caching issue for MySQL and MariaDB Perf: Enable cache, fix value casting for MySQL and MariaDB and add plugin for Engine creation Jan 8, 2025
@calebj
Copy link

calebj commented Jan 8, 2025

See ec72f5f for my proposed changes

@adrien-berchet
Copy link
Member Author

adrien-berchet commented Jan 9, 2025

See ec72f5f for my proposed changes

I updated according to your suggestion. I added two parameters, one for MySQL and one for MariaDB, just in case some drivers for MariaDB change their behavior.
Does it look good to you now?

@calebj
Copy link

calebj commented Jan 9, 2025

Good idea, looks good to me.

@adrien-berchet
Copy link
Member Author

Great!
Thank you very much for all of this, this is a great improvement for GeoAlchemy2! ❤️

@adrien-berchet adrien-berchet merged commit c2d8f10 into geoalchemy:master Jan 9, 2025
10 checks passed
@adrien-berchet adrien-berchet deleted the improve_perf branch January 9, 2025 15:13
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.

Bad performance for SQLAlchemy >= 1.4 due to disabled caching
2 participants