You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@lucenenet.apache.org by GitBox <gi...@apache.org> on 2022/11/02 14:49:56 UTC
[GitHub] [lucenenet] chenhh021 opened a new pull request, #738: Robustness issue in QuadPrefixTree
chenhh021 opened a new pull request, #738:
URL: https://github.com/apache/lucenenet/pull/738
Fix [#644](https://github.com/apache/lucenenet/issues/644) by mimic lucene code. For reference, see this [commit](https://github.com/apache/lucene/commit/26628b2717a73235db56fde94f7f5b64cbc5b8b2) on lucene side. It was for lucene 9.4, but much of the change is valid for the present version of lucene.net. Contents are:
1. Reimplement QuadPrefixTree's getCell method.
2. Add a version check so the older versions still use the older implementation.
I have confirmed that all spatial tests pass after these changes.
Could anyone help review this PR?
--
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: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] chenhh021 commented on pull request #738: Robustness issue in QuadPrefixTree
Posted by GitBox <gi...@apache.org>.
chenhh021 commented on PR #738:
URL: https://github.com/apache/lucenenet/pull/738#issuecomment-1302078346
Thanks for the review and telling the reasons that this PR cannot get accepted. I'll try to fix the related problem in my own fork.
I'm wondering how to treat the related issue, is it okay that just let it open there?
--
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: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] NightOwl888 commented on a diff in pull request #738: Robustness issue in QuadPrefixTree
Posted by GitBox <gi...@apache.org>.
NightOwl888 commented on code in PR #738:
URL: https://github.com/apache/lucenenet/pull/738#discussion_r1012537773
##########
src/Lucene.Net.Spatial/Prefix/Tree/QuadPrefixTree.cs:
##########
@@ -379,7 +442,10 @@ protected internal override SpatialPrefixTree NewSPT()
if (m_ctx is null)
throw new InvalidOperationException($"{nameof(m_ctx)} must be set prior to calling NewSPT().");
- return new QuadPrefixTree(m_ctx, m_maxLevels ?? QuadPrefixTree.MAX_LEVELS_POSSIBLE);
+ //return new QuadPrefixTree(m_ctx, m_maxLevels ?? QuadPrefixTree.MAX_LEVELS_POSSIBLE);
+ QuadPrefixTree tree = new QuadPrefixTree(m_ctx, m_maxLevels ?? QuadPrefixTree.MAX_LEVELS_POSSIBLE);
+ tree.m_robust = getVersion().OnOrAfter(LuceneVersion.LUCENE_48);
Review Comment:
This breaks behavioral compatibility with LUCENE_48. If the user requests LUCENE_48 compatibility, why would we return behavior that didn't exist until LUCENE_8_3_0?
--
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: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] NightOwl888 commented on pull request #738: Robustness issue in QuadPrefixTree
Posted by GitBox <gi...@apache.org>.
NightOwl888 commented on PR #738:
URL: https://github.com/apache/lucenenet/pull/738#issuecomment-1302489013
> Thanks for the review and telling the reasons that this PR cannot get accepted. I'll try to fix the related problem in my own fork.
>
> I'm just wondering how to treat the related issue, is it okay that just let it open there?
Being that this issue is technically a duplicate of [LUCENE-8755(https://issues.apache.org/jira/browse/LUCENE-8755) and will go away when we upgrade the port, I have just closed it. However, if you could work out how to extend `QuadPrefixTree` (or replace it, whichever is sensible) and share your final solution there, I am sure others looking for an answer to this would find it helpful.
--
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: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] rclabo commented on pull request #738: Robustness issue in QuadPrefixTree
Posted by GitBox <gi...@apache.org>.
rclabo commented on PR #738:
URL: https://github.com/apache/lucenenet/pull/738#issuecomment-1302103214
I agree with @NightOwl888 . That said, @chenhh021 I'd also like to thank you for the PR. While it doesn't make sense at this point to merge this into LuceneNET 4.8, having this PR linked from the related issue provides code that may help someone in the future if they run up against this issue in LuceneNET 4.8. So your contribution may well end up helping others (even me!). So it's very much appreciated. Thank you.
--
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: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [lucenenet] NightOwl888 closed pull request #738: Robustness issue in QuadPrefixTree
Posted by GitBox <gi...@apache.org>.
NightOwl888 closed pull request #738: Robustness issue in QuadPrefixTree
URL: https://github.com/apache/lucenenet/pull/738
--
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: dev-unsubscribe@lucenenet.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org