-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
ST_Within function #7990
ST_Within function #7990
Conversation
Codecov Report
@@ Coverage Diff @@
## master #7990 +/- ##
============================================
- Coverage 71.23% 70.33% -0.90%
- Complexity 4218 4222 +4
============================================
Files 1595 1597 +2
Lines 82743 82900 +157
Branches 12347 12369 +22
============================================
- Hits 58942 58308 -634
- Misses 19813 20591 +778
- Partials 3988 4001 +13
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
if (_results == null) { | ||
_results = new int[DocIdSetPlanNode.MAX_DOC_PER_CALL]; | ||
} |
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.
This is a common pattern but one I am trying to move transform functions away from - use projectionBlock.getNumDocs()
in preference to DocIdSetPlanNode.MAX_DOC_PER_CALL
because it might be much smaller than 10k if there is any filtering.
|
||
public class StWithinFunctionTest extends GeoFunctionTest { | ||
@Test | ||
public void testWithin() |
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.
😍
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.
👍🏻
Description
The ST_Within function is described in the geospatial docs (https://docs.pinot.apache.org/basics/indexing/geospatial-support) but I don't think it actually exists in the code.
Upgrade Notes
Does this PR prevent a zero down-time upgrade? (Assume upgrade order: Controller, Broker, Server, Minion)
backward-incompat
, and complete the section below on Release Notes)Does this PR fix a zero-downtime upgrade introduced earlier?
backward-incompat
, and complete the section below on Release Notes)Does this PR otherwise need attention when creating release notes? Things to consider:
release-notes
and complete the section on Release Notes)Release Notes
Documentation