-
Notifications
You must be signed in to change notification settings - Fork 409
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[OAK-10953] #1646
base: trunk
Are you sure you want to change the base?
[OAK-10953] #1646
Conversation
.../src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticIndexHelper.java
Outdated
Show resolved
Hide resolved
...c/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/ElasticRequestHandler.java
Outdated
Show resolved
Hide resolved
caf2331
to
870f64c
Compare
…ch knn search feature
String propertyPath = PathUtils.getParentPath(pd.name); | ||
String propertyName = PathUtils.getName(pd.name); | ||
NodeState tempState = targetNodeState; | ||
for (String token : PathUtils.elements(propertyPath)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could breaking on an empty token break anything? Does it always leave tempState
in a desirable state?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's hard to tell myself. This part existed in the previous implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job! Just a minor.
The failing test in oak-search-elastic is not related to this changes. It's a flaky test we should look at separately.
...rc/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/index/ElasticDocumentMaker.java
Outdated
Show resolved
Hide resolved
Thanks for confirmation. I observed no problem with it on the local environment, so that was my guess. I'll have a look at it closer, because otherwise we might easily forget about it. |
Substitute Elastiknn plugin with native inn query provided by Elasticsearch.