Fix partitioned spatial joins with small spatial index #14485
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This is also a step towards spatial LEFT JOINs, since non-matching rows have to be handled more intentionally. |
aweisberg
reviewed
May 6, 2020
|
I'm interpreting the session property gating is "leave most of the PR the same, but only add the OUTSIDE partition index if a session property is set." If so, sure! |
aweisberg
reviewed
May 6, 2020
| * We assume that the right node is the build side. The CBO is disabled | ||
| * for spatial joins, so this should be a good assumption. Then we have: | ||
| * 1. INNER joins are OK. | ||
| * 2. LEFT joins when the left node is a Point are OK. |
There was a problem hiding this comment.
The implementation doesn't actually allow this yet
aweisberg
reviewed
May 6, 2020
| @@ -101,7 +101,7 @@ public static void output(SpatialPartitioningState state, BlockBuilder out) | |||
| Rectangle envelope = state.getExtent(); | |||
|
|
|||
| // Add a small buffer on the right and upper sides | |||
aweisberg
suggested changes
May 7, 2020
There was a problem hiding this comment.
LGTM modulus the notes and and adding a flag so we can disable this.
We talked about it, and while people can go and update their spatial partitioning my concern is that they might go and do that and then find out there is some other blocker that prevents them from doing it.
jagill
force-pushed
the
spatial-partition-fix
branch
from
5867755
to
022bade
Compare
jagill
force-pushed
the
spatial-partition-fix
branch
2 times, most recently
from
309b201
to
bfb52c6
Compare
jagill
force-pushed
the
spatial-partition-fix
branch
from
bfb52c6
to
c9f8ba1
Compare
jagill
force-pushed
the
spatial-partition-fix
branch
from
c9f8ba1
to
bd88ff1
Compare
jagill
force-pushed
the
spatial-partition-fix
branch
2 times, most recently
from
1e250ef
to
5e5cf8b
Compare
aweisberg
approved these changes
Jul 22, 2020
Currently, if a geometry is outside the bounding box of a KdbTree, it is dropped: it's assigned an empty partition index array, which is unnested, resulting in the row being dropped. This can be an efficiency measure: if one side of the join is much smaller than the other, then the bounds will drop many rows before they are sent to the join worker. However, if the bounds are less than both the build- and probe-side of the join, then rows that would have matched in a non-partitioned join will be dropped when you partition the join. This makes the correctness of the partitioned join dependent on the partition chosen, which can lead to some surprising output changes that could be reasonably viewed as data loss. This commit changes the bounding box of the KdbTree to extend from -Infinity to +Infinity, so that all (non-empty) geometries will get at least one partition.
This commit adds more data to the spatial join tests, to test cases where geometries may be outside the range of those geometries used to build the spatial index.
jagill
force-pushed
the
spatial-partition-fix
branch
from
5e5cf8b
to
b0df9cd
Compare
arhimondr
approved these changes
Oct 5, 2020
4 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In a partitioned spatial join, rows with geometries are assigned 0, 1, or
more partition indexes via the KdbTree spatial partition. The geometry
is assigned one index for each leaf node rectangle it intersects.
Currently, if a geometry is outside the bounding box of a KdbTree, it is
dropped: it's assigned an empty partition index array, which is
unnested, resulting in the row being dropped. This can be an efficiency
measure: if one side of the join is much smaller than the other, then
the bounds will drop many rows before they are sent to the join worker.
However, if the bounds are less than both the build- and probe-side of
the join, then rows that would have matched in a non-partitioned join
will be dropped when you partition the join. This makes the correctness
of the partitioned join dependent on the partition chosen, which can
lead to some surprising output changes that could be reasonably viewed
as data loss.
This change makes the KdbTree "open": its outer boundaries extend to
infinity. This means all points in the plane belong to exactly one leaf node
of the KdbTree.