nknize commented on PR #1017: URL: https://github.com/apache/lucene/pull/1017#issuecomment-1187657941
> Early versions of XYShape where using encoded rectangles to perform bounding box queries. That proved to be wrong That has to do with the query not the field. This does not affect the doc value format thus shouldn't prevent the PR. Also, the doc value queries are using the same relate logic as the index queries so they are consistent. > Lucene does not support any geo faceting Correct. Solr implements faceting and since Solr is built on lucene we need a doc value format to support Solr faceting over the LatLonShape format. > still the current PR seems to be focusing on supporting faceting, The PR has nothing to do with faceting since Lucene doesn't implement faceting. The PR is 100% focused on the doc value format and BoundingBox relations only. > Maybe we should take some more time to review the encoding instead of rushing it into 9.3.... My suggestion is to either add it to the sandbox where there are no back-compact requirements The encoding is 100% a new field and marked as experimental. It is in core because since the sandbox module is now in an explicit `.sandbox` package, refactoring to sandbox would require refactoring core class modifiers and methods from package private to public so sandbox classes can extend them. This breaks the API. The key take way here is that the coordinate encoding is the same as the index format that won't change unless we decide to change `XYEncodingUtils` or `GeoEncodingUtils`, at which time we will have to support BWC for much more than the doc value format. Furthermore, the field is marked as experimental (the proper mechanism to communicate that formats might change); there are currently far more critical classes marked as experimental (e.g., [Lucene80DocValuesFormat](https://github.com/apache/lucene/blob/216e38a1596cbc6a036d92e68bc26ee483cd4d2a/lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene80/Lucene80DocValuesFormat.java#L138)) that make this less risky for 9.3. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
