janusgraph icon indicating copy to clipboard operation
janusgraph copied to clipboard

Solr textContainsPhrase & Solr Client Tokenizer Alignment

Open criminosis opened this issue 1 year ago • 4 comments


Thank you for contributing to JanusGraph!

In order to streamline the review of the contribution we ask you to ensure the following steps have been taken:

For all changes:

  • [X] Is there an issue associated with this PR? Is it referenced in the commit message?
  • [X] Does your PR body contain #xyz where xyz is the issue number you are trying to resolve?
  • [X] Has your PR been rebased against the latest commit within the target branch (typically master)?
  • [X] Is your initial contribution a single, squashed commit?

For code changes:

  • [X] Have you written and/or updated unit tests to verify your changes?
  • [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • [ ] If applicable, have you updated the LICENSE.txt file, including the main LICENSE.txt file in the root of this repository?
  • [ ] If applicable, have you updated the NOTICE.txt file, including the main NOTICE.txt file found in the root of this repository?

For documentation related changes:

  • [ ] Have you ensured that format looks appropriate for the output in which it is rendered?

Fixes https://github.com/JanusGraph/janusgraph/issues/4165 Closes https://github.com/JanusGraph/janusgraph/issues/4164

criminosis avatar Dec 03 '23 05:12 criminosis

@VladimirBogomolov Would you like to review?

li-boxuan avatar Dec 06 '23 06:12 li-boxuan

I believe running the test class I modified without the modified Solr client should demonstrate it. The test class I modified appeared to be executed upon all providers if I'm not mistaken.

criminosis avatar Dec 06 '23 07:12 criminosis

The test class I modified appeared to be executed upon all providers if I'm not mistaken.

Oh yeah you are right, never mind then!

li-boxuan avatar Dec 06 '23 07:12 li-boxuan

Restoring the original client will opt it out of the containsPhrase testing based on the support flags evaluated by the supports method. But I believe it should demonstrate the breakage with regards to the textContains predicate.

If not I woefully missed up in my preliminary investigation 😅

criminosis avatar Dec 06 '23 07:12 criminosis