apache / lucene

Apache Lucene open-source search software
https://lucene.apache.org/
Apache License 2.0
2.67k stars 1.03k forks source link

Extend Geoshape interfaces so objects can be copied/serialized [LUCENE-7936] #8985

Closed asfimport closed 7 years ago

asfimport commented 7 years ago

Hi @david.wright@bksv.com,

I would like to propose to extends the GeoShape interfaces to be able to copy/serialized the objects. The current status and propose change is as following:

GeoPoint: It can be serialized by using x, y, z GeoCircle: It can be serialized by using getCenter() and getRadius() and getPlanetModel() GeoCompositeShape: It can be serialized by accesing shapes using size() and GetShape(int index) GeoPath: add methods to the interface getPoints() and getCutoffAngle() GeoPolygon: This is the most complicated one as we have different types: 1.- GeoCompositePolygon is just a composite 2.- GeoConcavePolygon and GeoConvexPolygon: Create a new interface for those polygons which exposes the points, holes, internaledges and concavity/convexity 3.- GeoComplexPolygons: Do nothing, they are too complex to be serialize??

I am intersested in accesing the discreatization of the polygons into convex and concave ones for other reasons as well. I think this should be expose as they end result can be used for other use cases.

Cheers,

I.


Migrated from LUCENE-7936 by Ignacio Vera (@iverase), resolved Aug 25 2017 Attachments: factory.patch, GeoBinaryCodec.patch, LUCENE-7936.patch (versions: 3), LUCENE-7936-code.patch (versions: 2), LUCENE-7936-GeoComplexPolygon.patch, LUCENE-7936-test.patch, Spatial4j.patch

asfimport commented 7 years ago

Ignacio Vera (@iverase) (migrated from JIRA)

Attached the proposed patch.

Note that I have called the new GeoPolygon interface GeoBasicPolygon which I do not particullary like but I could not come up with a better name.

GeoPolygonFactory has been extended to accept internalEdgesFlag.

A random test is added for copying GeoShapes.

asfimport commented 7 years ago

Karl Wright (@DaddyWri) (migrated from JIRA)

Hi @iverase, I'm not happy with exposing internal representational information everywhere.

For copying, I greatly prefer that objects know how to copy themselves. Implementing Cloneable is one standard way to do that, rather than make lots of public accessors. Same goes for serialization – you can either implement Serializable, which has a lot of problems, or you can use one of the java serialization frameworks.

The issue is that I don't know what standards Lucene uses elsewhere. I will do some asking around and get back to you on that.

asfimport commented 7 years ago

Ignacio Vera (@iverase) (migrated from JIRA)

Copying objects is not the main use case, what I want to achieve is what Spatial4j does with the binarycodec:

https://github.com/locationtech/spatial4j/blob/master/src/main/java/org/locationtech/spatial4j/io/BinaryCodec.java

This class is used to generate docValues which at the same time is used by the SerializedDVStrategy to check the relationship between indexed and query shapes. If we achieve this, we can pass through the interfaces the Geo3dShapes and use it with Lucene (I have already done it with very promising results).

It is true that the objects can provide the methods to searialize/ deserialize but I thought Geoshapes should not need to know how to do this.

The second use case is the following: I am using healpix (http://healpix.jpl.nasa.gov) to pixelate polygons on the sphere. The library requires polygons to be convex therefore for concave polygons I need to break them into the equivalent convex ones, which is what the GeoPolygonFactory actually does. This is very powerful but the information is not available.

Cheers,

I.

asfimport commented 7 years ago

Karl Wright (@DaddyWri) (migrated from JIRA)

Hi @iverase, according to Mike McCandless, the Lucene Core approach to serialization and duplication is that there is none. This was because they couldn't figure out a way to do it that maintained backwards compatibility adequately, and also because Lucene Core cannot have any external dependencies (for instance, on any external serialization frameworks).

I have a very busy few days coming up but by next weekend I may have some time to do some research into how best to implement duplication and serialization. The goal would be to do it in a way that could be used as a model for the rest of Lucene core.

Thanks!

asfimport commented 7 years ago

Ignacio Vera (@iverase) (migrated from JIRA)

Understood & Cheers!

asfimport commented 7 years ago

Karl Wright (@DaddyWri) (migrated from JIRA)

This class is used to generate docValues which at the same time is used by the SerializedDVStrategy to check the relationship between indexed and query shapes. If we achieve this, we can pass through the interfaces the Geo3dShapes and use it with Lucene (I have already done it with very promising results).

@iverase, the conversion to docvalues actually should live in the package org.apache.lucene.spatial3d. There are classes in there already that implement doc values fields, distance comparators, and DV logic. If these are insufficient for what you're trying to do, please let me know why.

As for the polygon pixelation, there are two considerations. First, this statement is not correct:

The library requires polygons to be convex therefore for concave polygons I need to break them into the equivalent convex ones, which is what the GeoPolygonFactory actually does.

GeoPolygonFactory generates concave polygons in the case where that's the best representation. It either generates ONE concave polygon or potentially MULTIPLE convex polygons. So perhaps you are going to need to do something different anyway?

In fact, I'm not certain that it's always possible to tile a concave polygon with convex polygons without adding vertices by interpolation.

asfimport commented 7 years ago

Ignacio Vera (@iverase) (migrated from JIRA)

"If these are insufficient for what you're trying to do, please let me know why."

For what I see there, the classes in the package only handle points in Planet model WGS84. I am interested in the SPHERE, indexing polygons as well as points. My approach is more similar to the class in spatial-extras Geo3DShape but creating my custom spatial context which wraps the Geo3dArea shapes.

"So perhaps you are going to need to do something different anyway?"

This is true for generic polygons but the polygons I am working with expand as much as a few degrees. I made an experiment tonight creating random polygons that expand less than Math.PI with many points. It was never decomposed using concave polygons. I do think the library does what I need for my use case.

asfimport commented 7 years ago

Karl Wright (@DaddyWri) (migrated from JIRA)

For what I see there, the classes in the package only handle points in Planet model WGS84. I am interested in the SPHERE, indexing polygons as well as points. My approach is more similar to the class in spatial-extras Geo3DShape but creating my custom spatial context which wraps the Geo3dArea shapes.

Adding planet model as an enum selector to the existing Lucene integration, or even as a set of parallel classes, would be a reasonable approach. Robert Muir wanted to "keep things simple" and very much limit the public API of the integration. It might be worth looking at the current spatial3d integration as a model. We certainly do not want users to have to understand details of the PlanetModel class, though. Hope this makes sense.

The spatial-extras module cannot be the way we do this because of the dependency on spatial-4j. We cannot include that dependency and remain in lucene-core. You might be able to use it as a model only but for reasons of API consistency it would be better to look at the integration in spatial3d as the model.

As for indexing polygons – adding that is OK to, but please do note that, for the Lucene integration, the way we specify polygons for the integration uses Robert's Polygon class, which has no relation at all to GeoPolygon as it is defined in spatial3d.geom.

This is true for generic polygons but the polygons I am working with expand as much as a few degrees. I made an experiment tonight creating random polygons that expand less than Math.PI with many points. It was never decomposed using concave polygons. I do think the library does what I need for my use case.

You are correct to state that, for any polygon less that Math.PI in extent, it will be decomposed solely as GeoConvexPolygon objects. So let's presume that if you had the right iterator you could inspect the GeoComposite and do the right thing. Unfortunately, the variants of GeoPolygon are all package-local, and that's not going to change, so you would need to add something to GeoPolygon itself to allow you to walk over the individual components in a way that does not reference any package-private classes in geom. This hints at a specialized iterator that you'd need to add to GeoPolygon. It's OK to add it at that level since all polygons are defined by points, and MAY be tiled. For example:

TileIterator decompose();

interface TileIterator {
  Tile next();
}

interface Tile {
  boolean isConvex();
  ComponentIterator points();
}

interface ComponentIterator {
  GeoPoint next();
}

That way you can iterate over everything within and not know anything about how it is put together from the outside. What do you think?

asfimport commented 7 years ago

Ignacio Vera (@iverase) (migrated from JIRA)

I am focusing on re-using the RecursiveTreePrefixStrategy on Lucene + serialized strategy which requires Geo3dShapes to be wrapped into Spatial4j shapes. I attach what it is now needed to have a basic implementation. I have created a GeoBinaryCodec and then the implementation is trivial (attached as well with test).

I understand that this is not the ideal implementation and it would be good to have a proper integration where the model for shapes is the Geo3d model.

I like your idea of the iterators and I beleive this is something we should expose as it can be valuable for other people. I will try to implement it.

Thanks!

I.

asfimport commented 7 years ago

Karl Wright (@DaddyWri) (migrated from JIRA)

@iverase, thanks for the code patches.

I think we need to involve @dsmiley if you want to extend what's in spatial-extras; he's the primary author of that module. David, what is your opinion as to the spatial4j patch proposed?

As for the binary codec, I'll have to think about how best to structure this. Usually, as I've indicated, it's better for packages structured as interfaces with multiple implementations to have each object know how to pack itself and unpack itself, rather than supporting deep inspection and writing a wrapper. Too much internal information has to be exposed to do the latter.

I can see adding basic methods to all Geo3D objects for encoding to, and decoding from, a codec binary stream. As long as these methods can be written efficiently, and the streams they read from/write to are standard in Lucene core or in the Java util package, I see no reason not to make this a standard feature of Geo3d. A typical pattern would look something like this:

public MyObject(final InputStream is) throws IOException {
  ...
}

...

`@Override`
public void write(final OutputStream os) throws IOException {
  ...
}

To be fully general, the interface-implementing class name would need to be included in the stream, and instantiated by reflection when the stream is read back. This uses a fair bit of space but for your purposes might well be OK. Please let me know. I'll think on this further.

asfimport commented 7 years ago

Ignacio Vera (@iverase) (migrated from JIRA)

It sounds great! We only need to keep an eye in performance.

Thanks!

asfimport commented 7 years ago

David Smiley (@dsmiley) (migrated from JIRA)

Hello @iverase and @DaddyWri. BTW I'm on travel and on vacation so I'm not as responsive as I'd like.

It's exciting to see some progress in this area. Having Geo3D shapes be able to serialize and deserialize themselves using core Java APIs (only) in particular would be great. This would allow their internal details to be less exposed. I have a strong preference for not using Lucene APIs here since it allows 3rd parties to use the jewel that is Spatial3D by itself without having to bring in Lucene-core. For example perhaps someone is doing pure in-memory calculations without Lucene in the picture. Any way, speaking of the format... would this theoretically write 3 doubles per vertex? :-/ Though if we write lat-lon then there's math to do at read-time.

Additions to the lucene-spatial-extras module – Geo3D SpatialContext impl, with BinaryCodec impl seems like a separate issue to me.

asfimport commented 7 years ago

Karl Wright (@DaddyWri) (migrated from JIRA)

@dsmiley, I don't think we'd thought much about the representation; more interested right now in how we do it than the exact details. We'd clearly want a minimal representation for a GeoPoint and I think that would be a lat and a lon and we'd take the hit on deserialization.

@iverase, I think David's suggestion is a good one: maybe create a new ticket to handle the spatial-extras contributions? The codec should become much simpler and I'd hope that David's interfaces could be extended to allow spatial4j objects to be similarly encoded/decoded. Once again, I'll be looking at the geo3d serialization design later in the week when I actually have time – or, @iverase, if you have time before then you could propose an implementation. But this is separate entirely from the TileIterator stuff. We should do one thing at a time I think.

asfimport commented 7 years ago

Ignacio Vera (@iverase) (migrated from JIRA)

Hi @DaddyWri,

I agree we should separate the different tasks. O leave this ticket for encoded/decoded of geoShapes and open one more ticket for the TileIterator stuff.

I see if I can propose something by the end of the week. One thing is clear is that you already open a can of worms with the GeoPoint stuff. The reason I used x/y/z instead of Lat/Lon is because of the Planet Model as you would need to encode it as well and for generic cases you will need two doubles to encode it.

Thinking more in depth, there is something I am doing wrong and it is trying to use GeoPoint as a GeoShape. There is one shape missig which is a GeoPointShape which is actually a GeoShape containing a point. GeoPoint should never extend GeoShape! So my first proposal is:

What do you think?

asfimport commented 7 years ago

Karl Wright (@DaddyWri) (migrated from JIRA)

@iverase, I took the tack of not serializing the planetmodel, but requiring it on deserialization. This will give a much more compact representation. The attached patch is not complete, and still needs much work (including actual implementations for most objects except GeoPoint).

asfimport commented 7 years ago

Karl Wright (@DaddyWri) (migrated from JIRA)

My thought is that we can explicitly choose to serialize the planet model exactly ONCE, should we want to do that, rather than inside of every point and shape. We also don't have to serialize class names EXCEPT when they can be ambiguous. To GeoPoint, I will be adding read/write methods for strings and for SerializedObjects. I think we still need a separate place for static methods to support serialization/deserialization to live – maybe in the SerializedObject interface itself, rather than in GeoPoint? We can supposedly do that now with Java 8 – although I'd really not want them to be visible outside the package if possible.

I have zero time today and most of tomorrow but if this looks promising please wait on implementing serialization until I'm ready. Tile iteration you can go ahead with. Thanks!

asfimport commented 7 years ago

Ignacio Vera (@iverase) (migrated from JIRA)

Ok I will wait until you are ready, it looking promising. I like what I see in the patch but I would like to have a thought about how we are using GeoPoint. My point is the following:

There are basically two type of Objects for building shapes:

GeoShape components: Everything that is extending the Vector class, GeoPoint and Planes. These do not need to be serialized. Geoshape objects: Everything that implements PlanetObject and hence extends GeoBasePanetObject. The interface PlanetObject should be implementing the SerializeObject.

There should be no Geoshape extending GeoPoint as that is like trying to build a shape by extending a Plane. A GeoShapePoint object should be a GeoShapeObject with one component, being that component a GeoPoint.

In addition, your static methods to support serialization/deserialization should live in GeoBasePanetObject. Then you can have them protected in the package.

Cheers,

I.

asfimport commented 7 years ago

Karl Wright (@DaddyWri) (migrated from JIRA)

GeoDegeneratePoint extends GeoPoint. This is necessary for a number of reasons and should not blow us up.

I thought of putting serialization/deserialization methods in BasePlanetObject but that did not seem like it matched up well with the serialization abstraction. I would like to have a framework for serialization that is more flexible.

I do not think it is necessary to add classes in the derivation hierarchy simply to support serialization. I'd rather that that functionality was more or less universal.

I'll be attaching a revised patch for review shortly, but then I really have to do other things today.

asfimport commented 7 years ago

Karl Wright (@DaddyWri) (migrated from JIRA)

This patch moves everything of import into SerializedObject. That's nice because a wide variety of objects can then be serializable in this framework.

asfimport commented 7 years ago

ASF subversion and git services (migrated from JIRA)

Commit 6b8f98db93689370a6df47a8645c80b1b0b39480 in lucene-solr's branch refs/heads/master from @DaddyWri https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=6b8f98d

LUCENE-7936: Add Geo3d framework for serialization and deserialization.

asfimport commented 7 years ago

ASF subversion and git services (migrated from JIRA)

Commit 8fc72d4a0fbe852813bc183bcc29f7c998f4c44e in lucene-solr's branch refs/heads/branch_6x from @DaddyWri https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=8fc72d4

LUCENE-7936: Add Geo3d framework for serialization and deserialization.

asfimport commented 7 years ago

ASF subversion and git services (migrated from JIRA)

Commit 4ff396427120b343954de8f66873f2c807088729 in lucene-solr's branch refs/heads/branch_7x from @DaddyWri https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=4ff3964

LUCENE-7936: Add Geo3d framework for serialization and deserialization.

asfimport commented 7 years ago

Karl Wright (@DaddyWri) (migrated from JIRA)

@iverase, I've committed the framework. Implementing the framework on an object-by-object basis is the next task, which I'll try to look at on Saturday.

asfimport commented 7 years ago

ASF subversion and git services (migrated from JIRA)

Commit fd11646af2da0302a315e77e3caa3e254681a2f9 in lucene-solr's branch refs/heads/master from @DaddyWri https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=fd11646

LUCENE-7936: Implement serialization/deserialization for all objects except GeoComplexPolygon and composites.

asfimport commented 7 years ago

ASF subversion and git services (migrated from JIRA)

Commit 15a06dfcae966bd32ec2b7f100b875ebcefafdc7 in lucene-solr's branch refs/heads/branch_6x from @DaddyWri https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=15a06df

LUCENE-7936: Implement serialization/deserialization for all objects except GeoComplexPolygon and composites.

asfimport commented 7 years ago

ASF subversion and git services (migrated from JIRA)

Commit 42d7b30dfbae45580165aa7284677d67263e9974 in lucene-solr's branch refs/heads/branch_7x from @DaddyWri https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=42d7b30

LUCENE-7936: Implement serialization/deserialization for all objects except GeoComplexPolygon and composites.

asfimport commented 7 years ago

ASF subversion and git services (migrated from JIRA)

Commit 693db3fe38aa1d9a1e76766cac900df9e22252fe in lucene-solr's branch refs/heads/master from @DaddyWri https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=693db3f

LUCENE-7936: Complete the work to support serialization and deserialization of individual Geo3D objects. This adds support for GeoComplexPolygon and the composites.

asfimport commented 7 years ago

ASF subversion and git services (migrated from JIRA)

Commit abc76f7f2852d9da4a54764321c2386dfb48d8c8 in lucene-solr's branch refs/heads/branch_6x from @DaddyWri https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=abc76f7

LUCENE-7936: Complete the work to support serialization and deserialization of individual Geo3D objects. This adds support for GeoComplexPolygon and the composites.

asfimport commented 7 years ago

ASF subversion and git services (migrated from JIRA)

Commit 5e8890ae1c751fdf285a1593f597716453e61bad in lucene-solr's branch refs/heads/branch_7x from @DaddyWri https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=5e8890a

LUCENE-7936: Complete the work to support serialization and deserialization of individual Geo3D objects. This adds support for GeoComplexPolygon and the composites.

asfimport commented 7 years ago

Karl Wright (@DaddyWri) (migrated from JIRA)

@iverase, the basic code is in place now. There still needs to be tests – and also serialization/deserialization methods in SerializableObject that includes the planet model in the serialization. I will try to get that done today at some point.

The main method you'd want to call for serializing an object is SerializableObject.writeObject(). For deserialization, you would use SerializableObject.readObject(). These do not record the planet model. I will be introducing writeObjectWithPlanetModel() and readObjectWithPlanetModel() to do that. Please let me know what you think.

asfimport commented 7 years ago

Ignacio Vera (@iverase) (migrated from JIRA)

Thanks @DaddyWri,

I attached a patch with a test for serialization. It showed few issues:

1) Holes can be null and need to be dealt with. 2) Java does not allow to cast one type of array to another type of array.

Regarding the current implementation, I think we should not serialize GeoPoint as an object and make it more compact. Objects should just serialize GeoPoints as two doubles. For example for a polygon with 6 points would save 6 GeoPoint strings. And in fact I think points should not be serializable at all, only classes that implement PlanetObject. what do you think?

Cheers,

I.

By the way, should not writeObject and readObject be public?

asfimport commented 7 years ago

Karl Wright (@DaddyWri) (migrated from JIRA)

@iverase, thanks for the patch.

(1) GeoPoints serialize as two doubles. They do not include the planet model. In fact, no serialization yet includes the planet model; it is implied and included as an argument. So there is no benefit in not allowing a GeoPoint to be serialized.

(2) I have not yet made public the statics that need to be public in SerializableObject. But you are correct that readObject and writeObject should be made so.

Thanks!

asfimport commented 7 years ago

ASF subversion and git services (migrated from JIRA)

Commit 26b7644d00ccbb01b82a7ecb456e36e7680a0e2d in lucene-solr's branch refs/heads/master from @DaddyWri https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=26b7644

LUCENE-7936: Committing a randomized serialization test on behalf of Ignacio Vera, along with fixes.

asfimport commented 7 years ago

ASF subversion and git services (migrated from JIRA)

Commit f4b3f55b9de15bca5595adb63f0e266380efd739 in lucene-solr's branch refs/heads/branch_6x from @DaddyWri https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=f4b3f55

LUCENE-7936: Committing a randomized serialization test on behalf of Ignacio Vera, along with fixes.

asfimport commented 7 years ago

ASF subversion and git services (migrated from JIRA)

Commit bdb3c253d8dbbc703d409201ae46297e6bab0c15 in lucene-solr's branch refs/heads/branch_7x from @DaddyWri https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=bdb3c25

LUCENE-7936: Committing a randomized serialization test on behalf of Ignacio Vera, along with fixes.

asfimport commented 7 years ago

Karl Wright (@DaddyWri) (migrated from JIRA)

@iverase, I've committed your test along with fixes so that it succeeds. Also made readObject() and writeObject() public.

asfimport commented 7 years ago

Ignacio Vera (@iverase) (migrated from JIRA)

Thanks @DaddyWri, I hope the test where useful.

I wonder if it is not worthy to implement the equals method of GeoComplexPolygon now. Because we keep the original point list on the object, the implementation should not be difficult. Then we can add those polygons to the test.

The other thing I have on my head is the support to "user defined" GeoShapes. How would users align their shapes in the current format when they want to serialize them? I might be implementing my own very domain specific shapes and I am wondering how to make serialization work.

Cheers,

I.

asfimport commented 7 years ago

Karl Wright (@DaddyWri) (migrated from JIRA)

Hi @iverase, yes please submit a patch for equals() and hashCode() for GeoComplexPolygon.

As far as the contract for serialization/deserialization goes, this is the way I have it set up:

(1) If you created your own shape before this change, you will simply get a "Unsupported operation exception" thrown if you try to serialize it. This is done in BasePlanetObject.

(2) If you want to support serialization, you need to implement the write(OutputStream) method, and you need a constructor with the following signature: MyClass(PlanetModel, InputStream) throws IOException

That's it; everything else should work for you.

asfimport commented 7 years ago

Ignacio Vera (@iverase) (migrated from JIRA)

Attached implementation of equals and hashcode in GeoComplexPolygons and added this type o shape to RandomBinaryCodecTest.

asfimport commented 7 years ago

ASF subversion and git services (migrated from JIRA)

Commit 554f0d5f2009a69389d4dd5d5d3907fa3a4727f5 in lucene-solr's branch refs/heads/master from @DaddyWri https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=554f0d5

LUCENE-7936: Complete the serialization/deserialization implementation of Geo3d.

asfimport commented 7 years ago

ASF subversion and git services (migrated from JIRA)

Commit 665d223c6e059626de568aee166708d6721f80aa in lucene-solr's branch refs/heads/branch_6x from @DaddyWri https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=665d223

LUCENE-7936: Complete the serialization/deserialization implementation of Geo3d.

asfimport commented 7 years ago

ASF subversion and git services (migrated from JIRA)

Commit 1e0b062ca7ba1344d03fb3dd2bed1b529f1d081b in lucene-solr's branch refs/heads/branch_7x from @DaddyWri https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=1e0b062

LUCENE-7936: Complete the serialization/deserialization implementation of Geo3d.

asfimport commented 7 years ago

Karl Wright (@DaddyWri) (migrated from JIRA)

@iverase, I have committed the remainder of what I think we need for this ticket, including your implementation of equals/hashCode for GeoComplexPolygon. The SerializationObject methods that include planet model are: writePlanetObject() and readPlanetObject(). Please let me know of any issues you encounter. Thanks!

asfimport commented 7 years ago

Steven Rowe (@sarowe) (migrated from JIRA)

My Jenkins found a couple reproducing failures for RandomBinaryCodecTest:

On branch_7x:

ant test  -Dtestcase=RandomBinaryCodecTest  -Dtests.seed=AA9F2C61D6F622BD -Dtests.slow=true -Dtests.locale=hr-HR -Dtests.timezone=America/Argentina/La_Rioja -Dtests.asserts=true -Dtests.file.encoding=UTF-8

On master:

ant test  -Dtestcase=RandomBinaryCodecTest  -Dtests.seed=A97EB454B3179214 -Dtests.slow=true -Dtests.locale=fr-LU -Dtests.timezone=WET -Dtests.asserts=true -Dtests.file.encoding=UTF-8
asfimport commented 7 years ago

ASF subversion and git services (migrated from JIRA)

Commit 287ab9dc40fd8abf1a6910e35052634b165b26d9 in lucene-solr's branch refs/heads/master from @DaddyWri https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=287ab9d

LUCENE-7936: Fix broken GeoComplexPolygon serialization.

asfimport commented 7 years ago

ASF subversion and git services (migrated from JIRA)

Commit 00c8d446277e80fe0ca42d8a19c1c45028e86cd7 in lucene-solr's branch refs/heads/branch_6x from @DaddyWri https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=00c8d44

LUCENE-7936: Fix broken GeoComplexPolygon serialization.

asfimport commented 7 years ago

ASF subversion and git services (migrated from JIRA)

Commit 83a32ff132c3e3d15878fca56ce6bfc201840450 in lucene-solr's branch refs/heads/branch_7x from @DaddyWri https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=83a32ff

LUCENE-7936: Fix broken GeoComplexPolygon serialization.

asfimport commented 7 years ago

Karl Wright (@DaddyWri) (migrated from JIRA)

@sarowe: should be fixed now.

asfimport commented 7 years ago

Ignacio Vera (@iverase) (migrated from JIRA)

Hi @DaddyWri,

It have noticed that the serialization constructor is missing in GeoCompositeMembershipShape class.

I have been checking as the performance and I noticed that if we codify the class name into a byte, deserialization of objects can be up to 5 times faster. I think we should explore that option.

I am attaching a factory that codifies the class into a byte. 0 to 127 is reserve for internal classes and -128 to -1 to user define classes. To make this work for objects that serialize other objects we would need to pass the factory into the constructor.

Anyway before going to details I want to check if you are ok to explore other solutions. I really like the simplicity of the current implementation but when I saw that reading GeoPoints can be 5 time faster, I though it is worthy at least a thought.

Let me know what you think.

asfimport commented 7 years ago

ASF subversion and git services (migrated from JIRA)

Commit 8fc61e56288b6058a007ea651b0e2897039726da in lucene-solr's branch refs/heads/master from @DaddyWri https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=8fc61e5

LUCENE-7936: Missed a constructor for deserialization support.