Skip to content
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

Lucene cross class queries #9855

Open
wants to merge 1 commit into
base: 3.1.x
Choose a base branch
from

Conversation

timw
Copy link
Contributor

@timw timw commented Aug 15, 2022

What does this PR do?

Port the SEARCH_CROSS Lucene cross class index search function from the enterprise agent codebase.

Motivation
SEARCH_CROSS is described in the documentation for enterprise edition, but is not built/distributed with the open source based enterprise agent.

Additional Notes
There's a minor hack (along the lines of the existing minor hack) to allow the LUCENE_CROSS_CLASS index to be created without triggering the manual index deprecation.

Checklist
[x] I have run the build using mvn clean package command
[x] My unit tests cover both failure and success scenarios

This provides the SEARCH_CROSS function, based on the enterprise agent version of the same function.
@timw timw changed the base branch from develop to 3.1.x August 15, 2022 05:11
Copy link
Member

@tglman tglman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Him

This looks good, only one change I would prefer to remove the specific enable of LUCENE_CROSS_INDEX and still rely on the:

  INDEX_ALLOW_MANUAL_INDEXES(
      "index.allowManualIndexes",
      "Switch which allows usage of manual indexes inside OrientDB. "
          + "It is not recommended to switch it on, because manual indexes are deprecated, not supported and will be removed in next versions",
      Boolean.class,
      true),
      ``` 
      that  is the base behaviour as today, I will probably re-enable the manual index when there will be a better support in distributed, as the current develop there are some changes already that would allow the support of manual index to be re-enabled soon.

@@ -129,7 +129,7 @@ public OIndex createIndex(
|| indexDefinition.getClassName() == null
|| indexDefinition.getFields() == null
|| indexDefinition.getFields().isEmpty();
if (manualIndexesAreUsed) {
if (manualIndexesAreUsed && !"LUCENE_CROSS_CLASS".equals(algorithm)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not include the specific check for "LUCENE_CROSS_CLASS" and just leave the default check.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the cross class index isn't really a manual index, it just collides with the manual index logic as it's created as a special case index. This was just replicating the logic in OCreateIndexStatement.execute, which special cases the index.
It should probably be modelled as a distinct type of index (i.e. maybe a virtual index) that has no key fields, but since the existing hack was in place I replicated it rather than extend the index SPI.

As you say, we can work around that by enabling manual indexes, but it's a bit messy either way.

@timw
Copy link
Contributor Author

timw commented Mar 27, 2023

We didn't end up using cross class indexes, so don't have any push to continue this work - happy for you to close the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants