OAK-11555 Elastic: support dot in property and function names#2145
Merged
thomasmueller merged 8 commits intotrunkfrom Mar 13, 2025
Merged
OAK-11555 Elastic: support dot in property and function names#2145thomasmueller merged 8 commits intotrunkfrom
thomasmueller merged 8 commits intotrunkfrom
Conversation
Commit-Check ✔️ |
nfsantos
reviewed
Mar 7, 2025
...ic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexDefinition.java
Outdated
Show resolved
Hide resolved
chibulcu
reviewed
Mar 7, 2025
...ic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/ElasticIndexUtils.java
Outdated
Show resolved
Hide resolved
chibulcu
reviewed
Mar 7, 2025
...ic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/ElasticIndexUtils.java
Outdated
Show resolved
Hide resolved
nfsantos
reviewed
Mar 7, 2025
...ic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/ElasticIndexDefinition.java
Outdated
Show resolved
Hide resolved
...ic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/ElasticIndexUtils.java
Outdated
Show resolved
Hide resolved
...ic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/ElasticIndexUtils.java
Outdated
Show resolved
Hide resolved
...ic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/ElasticIndexUtils.java
Show resolved
Hide resolved
...c/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/query/ElasticRequestHandler.java
Outdated
Show resolved
Hide resolved
...abbit/oak/plugins/index/elastic/query/async/facets/ElasticStatisticalFacetAsyncProvider.java
Show resolved
Hide resolved
nfsantos
reviewed
Mar 7, 2025
...ic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/ElasticIndexUtils.java
Outdated
Show resolved
Hide resolved
...ic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/ElasticIndexUtils.java
Outdated
Show resolved
Hide resolved
...ic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/ElasticIndexUtils.java
Outdated
Show resolved
Hide resolved
…gins/index/elastic/util/ElasticIndexUtils.java Co-authored-by: Nuno Santos <nsantos@adobe.com>
|
nfsantos
approved these changes
Mar 12, 2025
Contributor
nfsantos
left a comment
nfsantos
left a comment
There was a problem hiding this comment.
Seems correct, I just have a few suggestions for optimization.
...ic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/ElasticIndexUtils.java
Show resolved
Hide resolved
...ic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/ElasticIndexUtils.java
Outdated
Show resolved
Hide resolved
...ic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/ElasticIndexUtils.java
Outdated
Show resolved
Hide resolved
...ic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/ElasticIndexUtils.java
Outdated
Show resolved
Hide resolved
chibulcuteanu
approved these changes
Mar 12, 2025
...jackrabbit/oak/plugins/index/elastic/query/async/facets/ElasticSecureFacetAsyncProvider.java
Show resolved
Hide resolved
...abbit/oak/plugins/index/elastic/query/async/facets/ElasticStatisticalFacetAsyncProvider.java
Outdated
Show resolved
Hide resolved
fabriziofortino
approved these changes
Mar 12, 2025
daniancu
pushed a commit
to daniancu/jackrabbit-oak
that referenced
this pull request
Mar 13, 2025
…#2145) * OAK-11555 Elastic: support dot in property and function names * OAK-11555 Elastic: support dot in property and function names * OAK-11555 Elastic: support dot in property and function names * Update oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/ElasticIndexUtils.java Co-authored-by: Nuno Santos <nsantos@adobe.com> * OAK-11555 Elastic: support dot in property and function names * OAK-11555 Elastic: support dot in property and function names * OAK-11555 Elastic: support dot in property and function names * OAK-11555 Elastic: support dot in property and function names --------- Co-authored-by: Nuno Santos <nsantos@adobe.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is longer than expected. The problem is that there was no conversion function from "JCR property name" to "Elasticsearch field name" so far, so I had to add it everywhere. That also affects tests etc.
Side node: ElasticIndexUtilsTest: I also added unit tests of the existing functions. I find it really weird that we convert randomly distributed bytes to a string like so:
... and then use that as the String value for "_id" field in Elasticsearch. But... (a) changing it would introduce incompatibility, and (b) it seems to work.
How I tested: I changed the ElasticIndexUtils.fieldName method to return "X" + the actual result, and like this could find other places where the method is needed.
I added a method to undo (revert / unescape) the conversion, so that we can verify it is always possible to do so. Even thought, right now it is not needed.