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

Use pypika's SqlContext to improve performance #1837

Merged
merged 5 commits into from
Jan 10, 2025

Conversation

henadzit
Copy link
Contributor

@henadzit henadzit commented Jan 3, 2025

Description

Motivation and Context

This PR improves the performance of SQL generation by pypika (see the Codspeed comment for more details), hence improving over query performance:

  • 23% improvement in the test_get_many_fields benchmark
  • 9% improvement in the test_filter_many_filters benchmark
  • 4% improvement in test_filter_few_fields

How Has This Been Tested?

make ci

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added the changelog accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link

codspeed-hq bot commented Jan 3, 2025

CodSpeed Performance Report

Merging #1837 will improve performances by 23.49%

Comparing henadzit:feat/pypika-sql-context (d487ebe) with develop (46e3aef)

Summary

⚡ 1 improvements
✅ 7 untouched benchmarks

Benchmarks breakdown

Benchmark develop henadzit:feat/pypika-sql-context Change
test_get_many_fields 1.4 ms 1.2 ms +23.49%

@henadzit henadzit force-pushed the feat/pypika-sql-context branch from abcb490 to 95565ad Compare January 3, 2025 11:13
@coveralls
Copy link

coveralls commented Jan 3, 2025

Pull Request Test Coverage Report for Build 12713770244

Details

  • 26 of 29 (89.66%) changed or added relevant lines in 8 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on feat/pypika-sql-context at 90.252%

Changes Missing Coverage Covered Lines Changed/Added Lines %
tortoise/backends/mysql/executor.py 2 3 66.67%
tortoise/indexes.py 0 2 0.0%
Totals Coverage Status
Change from base Build 12713629151: 90.3%
Covered Lines: 6432
Relevant Lines: 7011

💛 - Coveralls

@henadzit henadzit marked this pull request as ready for review January 3, 2025 11:53
@henadzit henadzit requested a review from abondar January 5, 2025 20:49
@henadzit
Copy link
Contributor Author

@abondar any chance you can have a look at this? Thanks!

abondar
abondar previously approved these changes Jan 10, 2025
Copy link
Member

@abondar abondar left a comment

Choose a reason for hiding this comment

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

Thank you, it looks much better that way, then implicit kwargs

I'll try to not miss when you update PR with actual lib version to reapprove, but if I don't - don't hesitate to tag me please. I am still a little bit in new year vibe (:

@henadzit
Copy link
Contributor Author

@abondar happy new year!

I've updated pypika-tortoise, so it's ready for review again.

@henadzit henadzit merged commit e34c2c0 into tortoise:develop Jan 10, 2025
9 checks passed
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.

3 participants