-
Notifications
You must be signed in to change notification settings - Fork 192
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
Add support for native point array properties #404
base: master
Are you sure you want to change the base?
Add support for native point array properties #404
Conversation
3769093
to
848eb29
Compare
848eb29
to
bcb30fd
Compare
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.
Nice enhancement. I did have some suggestions though.
} | ||
double[] coordinate = point.getCoordinate().getCoordinate(); | ||
if (crs.dimensions() == 3) { | ||
return new Coordinate(coordinate[0], coordinate[1], coordinate[2]); |
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.
Should we enforce all points to have the same dimension? So get the dimension from the first, and ensure all others are the same? I see you assert on the CRS, so this would be similar.
Actually, we assert that the CRS is WGS84, which is explicitly 2D, so perhaps we should reject all 3d points, or ignore the third dimension?
|
||
@Override | ||
public String getConfiguration() { | ||
return property ":" bboxProperty ": " crs.getCode(); |
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.
If we support more than WG84, we should also have tests for that.
@@ -1244,6 1244,16 @@ public void find_no_geometries_using_closest_on_empty_layer() { | |||
testCallCount(db, "CALL spatial.closest('geom',{lon:15.2, lat:60.1}, 1.0)", null, 0); | |||
} | |||
|
|||
@Test | |||
public void testNativePoints() { | |||
execute("CREATE (node:Foo { points: [point({latitude: 5.0, longitude: 4.0}), point({latitude: 6.0, longitude: 5.0})]})"); |
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.
Perhaps a test for 3D or non-WGS84 data?
6844765
to
bee28a4
Compare
This introduces a new Encoder that allows to index native point arrays like this: