-
Notifications
You must be signed in to change notification settings - Fork 872
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
base: 3.1.x
Are you sure you want to change the base?
Conversation
This provides the SEARCH_CROSS function, based on the enterprise agent version of the same function.
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.
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)) { |
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.
I would not include the specific check for "LUCENE_CROSS_CLASS" and just leave the default check.
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 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.
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. |
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