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

ISPN-5729 Remote query - org/apache/lucene/uninverting/UninvertingRea… #4758

Merged
merged 4 commits into from Jan 23, 2017

Conversation

anistor
Copy link
Member

@anistor anistor commented Jan 12, 2017

…der missing on "order by" query

https://issues.jboss.org/browse/ISPN-5729

@@ -8,6 +8,7 @@
<module name="javax.transaction.api" />
<module name="org.hibernate.commons-annotations" />
<module name="org.apache.lucene" export="true" />
<module name="org.apache.lucene.internal" export="true" />

Choose a reason for hiding this comment

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

This sounds a bit strange, an "internal" that is exported? Maybe we should move whatever need to be exported to the "public" module?

Choose a reason for hiding this comment

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

Also, I suppose this change would need to go to the modules we distribute as well https://github.com/infinispan/infinispan/tree/master/wildfly-modules
In that case, we simply overlay whatever the hibernate search module zip gives us, probably we should tell them to export the missing classes...

Choose a reason for hiding this comment

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

@anistor answering your question from the chat, the lucene-internal module was originally created as part of https://hibernate.atlassian.net/browse/HSEARCH-1849 and judging by the contents, the goal is to host older codecs and compatibility with older Lucene versions.

So maybe we could follow the wirings done in hibernate search that it'd work. Check here, here and here

WRT wildfly-modules, this not required since we simply explode hibernate search modules in the our zip. Probably we just need to test that the Uninverter works.

Copy link
Member Author

Choose a reason for hiding this comment

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

It seems exporting "org.apache.lucene.internal" was not need. I've added it as a regular dependency and it's enough to fix the problem.

No need to do anything for the wildfly-modules; they work as they are because the hibernate search modules do not have this issue.

I think it should be all right now.

@@ -68,4 +68,31 @@ public void testRemoteQuery() throws Exception {

rcm.stop();
}

@Test
public void testUninverting() throws Exception {

Choose a reason for hiding this comment

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

It's maybe not trivial to figure out what this test is doing, maybe put a comment like "Using a sort on a field that does not contain DocValues so Lucene is forced to uninvert it" to the posterity?

@anistor
Copy link
Member Author

anistor commented Jan 13, 2017

Fair points. I'll address them once I get at least current version of this PR to run successfully in CI....

@galderz
Copy link
Member

galderz commented Jan 23, 2017

@anistor Have you addressed the questions?

@galderz
Copy link
Member

galderz commented Jan 23, 2017

As far as CI, RemoteQueryIspnDirIT is failing, but it's a flaky one.

@anistor
Copy link
Member Author

anistor commented Jan 23, 2017

Only minor comments addressed. Still need to cleanup de module.xml. I'm on it.

@galderz galderz merged commit 4ab805f into infinispan:master Jan 23, 2017
@anistor anistor deleted the t_5729_m branch January 23, 2017 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants