+1

Commit 022877fefabadd5865c335a5b289874d182ed852 in lucene-solr's branch refs/heads/master from Ryan McKinley
[ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=022877f ]

SOLR-8859: read/write Shapes to String

Commit 7687667b5ff7867249762d104707a91834d30ce3 in lucene-solr's branch refs/heads/branch_6x from Ryan McKinley
[ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=7687667 ]

SOLR-8859: read/write Shapes to String

Manually correcting fixVersion per Step #S5 of LUCENE-7271

While looking at the code for this again, I saw a couple problems, which I verified through the debugger.

The former behavior used to support 2 formats: "lat,lon" and WKT, and it did this very efficiently – it could tell it's WKT immediately and if not it parsed using SpatialUtils.parsePointSolrException (the lat,lon format).

Now, it'll use the Spatial4j shapeReader configured, which if not configured (true for existing configs and I suspect for the majority of folks going forward still?) it'll be arbitrarily the first one. By coincidence that's GeoJSON. No matter what it is, however, parseShape will try all ShapeReaders in Spatial4j registered with the SpatialContext. This even includes, sadly, LegacyShapeReader which parses "lat,lon" and some stuff like "Circle(x y d=dist)" that we stopped supporting many releases ago. Some of these readIfSupported impls throw exceptions to signal they don't support it. So in summary, (1) this is slow, and (2) IMO if you configure= format X then we should only parse as format X. I have reservations about the notion of cycling through all formats to see what it is.

RptWithGeometrySpatialField.init doesn't call super.init and thus the shapeReader & shapeWriter isn't initialized. That's perhaps not a bug in this issue but it was a non-issue before and now it is. It's a challenging conundrum; ideally it would but I had difficulty with a cach-22 of extending it and also supplying it to CompositeSpatialStrategy. I can revisit that in a separate issue.

The attached patch rectifies the aforementioned problems includes more testing. Fixing RptWithGeometrySpatialField was actually pretty easy so I just did it here. The patch also clarifies the CHANGES.txt to make it more apparent that this adds GeoJSON:

* SOLR-8859: Spatial fields like RPT can now be configured to use Spatial4j registered shape formats
  e.g. via format="GeoJSON".  (ryan, David Smiley)

I plan to commit this tomorrow.

Commit fb37b3eb8c4130c8b5f53e1741e9585743b26e4d in lucene-solr's branch refs/heads/master from David Smiley
[ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=fb37b3e ]

SOLR-8859: Fix AbstractSpatialFieldType to not cycle through all Spatial4j provided formats.
And Fix RptWithGeometrySpatialField to be less brittle on init()

Commit 3a57beaa9e202c848061881d3c887ec076f19282 in lucene-solr's branch refs/heads/branch_6x from David Smiley
[ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=3a57bea ]

SOLR-8859: Fix AbstractSpatialFieldType to not cycle through all Spatial4j provided formats.
And Fix RptWithGeometrySpatialField to be less brittle on init()
(cherry picked from commit fb37b3e)

Commit fb37b3eb8c4130c8b5f53e1741e9585743b26e4d in lucene-solr's branch refs/heads/SOLR-9191 from David Smiley
[ https://git-wip-us.apache.org/repos/asf?p=lucene-solr.git;h=fb37b3e ]

SOLR-8859: Fix AbstractSpatialFieldType to not cycle through all Spatial4j provided formats.
And Fix RptWithGeometrySpatialField to be less brittle on init()