Introduce GeometryTree writer/reader #42331
Conversation
The GeometryTree represent an Elastisearch Geometry object. This includes collections like MultiPoint and GeometryCollection. For the initial implementation, only polygons without holes are supported. In a follow-up PR, the GeometryTree will be the object that interacts with doc-value reading and writing.
talevy
added
the
:Analytics/Geo
|
Pinging @elastic/es-analytics-geo |
|
run elasticsearch-ci/2 |
| @Override | ||
| public Void visit(Point point) { | ||
| // TODO | ||
| return null; |
There was a problem hiding this comment.
I think this should throw UnsupportedOperationException for now to make sure that if we ever hit this we know about it. The same for other visit methods.
| output.writeInt(builder.minLon); | ||
| output.writeInt(builder.minLat); | ||
| output.writeInt(builder.maxLon); | ||
| output.writeInt(builder.maxLat); |
There was a problem hiding this comment.
It feels like we could use a Writable bounding box class that knows how to read and write itself as well as check for intersections with other bounding boxes. We could then reuse it and in EdgeTreeWriter, which can also become just an EdgeTree.
| */ | ||
| public class GeometryTreeWriter { | ||
|
|
||
| private final GeometryTreeBuilder builder; |
There was a problem hiding this comment.
I think this builder can become a Writable GeometryTree that knows how to write and partially read itself.
| class GeometryTreeBuilder implements GeometryVisitor<Void, RuntimeException> { | ||
|
|
||
| private List<EdgeTreeWriter> shapeWriters; | ||
| int minLat; |
There was a problem hiding this comment.
Could you add a comment why we are using int here.
|
|
||
| @Override | ||
| public Void visit(Rectangle r) { | ||
| int[] lats = new int[] { (int) r.getMinLat(), (int) r.getMinLat(), (int) r.getMaxLat(), (int) r.getMaxLat(), |
There was a problem hiding this comment.
Is this conversion to (int) a temporary workaround?
There was a problem hiding this comment.
oh I see. this is a bug
There was a problem hiding this comment.
I will address in followup
| for (int i = 0; i < numTrees; i++) { | ||
| ShapeType shapeType = input.readEnum(ShapeType.class); | ||
| if (ShapeType.POLYGON.equals(shapeType)) { | ||
| BytesRef treeRef = input.readBytesRef(); |
There was a problem hiding this comment.
Wouldn't this create a copy of the entire byte array for the shape? Maybe edgeTree should know how to read itself from ByteBufferStreamInput and calculate relative position or even use skip() instead of position()?
|
thank you Igor, I will follow-up with the move to a Writeable interface for the tree objects and add more tests around real int-encoded polygons using GeoEncodingUtils. |
The GeometryTree represent an Elastisearch Geometry object. This includes collections like MultiPoint and GeometryCollection. For the initial implementation, only polygons without holes are supported. In a follow-up PR, the GeometryTree will be the object that interacts with doc-value reading and writing.
The GeometryTree represent an Elastisearch Geometry
object. This includes collections like MultiPoint
and GeometryCollection.
For the initial implementation, only polygons without
holes are supported. Later commits will fill in those
holes (see what I did there) and support the other geometries
In a follow-up PR, the GeometryTree will be the object that
interacts with doc-value reading and writing.
relates to #37206