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

Postgis join hints #1564

Merged
merged 7 commits into from Mar 15, 2024
Merged

Postgis join hints #1564

merged 7 commits into from Mar 15, 2024

Conversation

SpacemanPaul
Copy link
Contributor

Reason for this pull request

For complex queries (as can occur when using search_returning), SQLAlchemy was non-deterministically failing to resolve a join strategy. This was caused some tests to fail intermittently. A re-run (or 2, or 3) of the tests was usually sufficient to bypass but it shouldn't have been happening and needed to be addressed.

Proposed changes

  • Improved type hints in driver.postgis._fields
  • Made NativeFields smart enough to generate an onclause parameter for the SQLA join function, and wire in to the query generation code.

Running a temporary minimal test that triggered the issue, this change took us from failing roughly 1 run in 4 to not failing at all for 25+ sequential test runs.

  • Tests added / passed
  • Fully documented, including docs/about/whats_new.rst for all changes

Copy link

codecov bot commented Mar 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.57%. Comparing base (5b0d0c7) to head (2c96d9f).
Report is 1 commits behind head on develop-1.9.

Additional details and impacted files
@@               Coverage Diff               @@
##           develop-1.9    #1564      +/-   ##
===============================================
- Coverage        85.62%   85.57%   -0.05%     
===============================================
  Files              140      140              
  Lines            15299    15366      +67     
===============================================
+ Hits             13100    13150      +50     
- Misses            2199     2216      +17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@Ariana-B Ariana-B left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me.

@omad omad merged commit ea3068d into develop-1.9 Mar 15, 2024
30 checks passed
@omad omad deleted the postgis-join-hints branch March 15, 2024 04:30
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.

None yet

3 participants