-
Notifications
You must be signed in to change notification settings - Fork 54
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
Fix the cost estimate of the Spatial Join
operation
#1700
Conversation
The cost estimate didn't include the cost of the children previously. This bug led to non-sense output in the runtime information JSON output in some cases, because the RuntimeInformation::getOperationCostEstimate subtracted child cost estimates that were larger than the spatial join cost. The resulting negative number interpreted as size_t (unsigned) caused an output near 2**64. This is now fixed by adding the child cost to the spatial join cost estimate.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1700 +/- ##
=======================================
Coverage 89.85% 89.86%
=======================================
Files 389 389
Lines 37287 37288 +1
Branches 4204 4204
=======================================
+ Hits 33505 33507 +2
+ Misses 2483 2478 -5
- Partials 1299 1303 +4 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A very tiny suggestion just to make you aware of good and modern coding practices.
Conformance check passed ✅No test result changes. |
Quality Gate passedIssues Measures |
Spatial Join
operation
Previously, the cost estimate of the
SpatialJoin
class didn't account for the cost of computing the children. This is now fixed.