You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by osma <gi...@git.apache.org> on 2017/03/14 15:08:01 UTC
[GitHub] jena pull request #226: Refactor TextIndex and TextQueryPF in jena-text (pre...
GitHub user osma opened a pull request:
https://github.com/apache/jena/pull/226
Refactor TextIndex and TextQueryPF in jena-text (preparation for JENA-1305)
In the development of JENA-1305 (Elasticsearch backend for jena-text) it surfaced that the current division of labor between TextQueryPF and TextIndex is awkward. The Lucene (or Solr) query string is mostly formed already on the TextQueryPF side, which makes it difficult to implement a backend that uses a different query language.
I have refactored the TextIndex interface, separating out the graph and language information away from the query string into separate method parameters, and changed the code in TextQueryPF and TextIndexLucene correspondingly.
I'm opening this PR to ask for comments on the change. If there is no opposition, I will merge it to master within a few days.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/osma/jena jena-1305-refactor-textindex
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/jena/pull/226.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #226
----
commit 4bb8a393efd670a38246a6e9788adf117a01155d
Author: Osma Suominen <os...@helsinki.fi>
Date: 2017-03-13T15:42:48Z
fix warnings / remove dead code
commit 52f60589c59498cb7a7f52402f0515140952ab40
Author: Osma Suominen <os...@helsinki.fi>
Date: 2017-03-13T15:55:34Z
JENA-1305: refactor TextIndex to take a separate graphURI parameter
commit 91eabe77d1e9414529a85598a6709cda54196d2a
Author: Osma Suominen <os...@helsinki.fi>
Date: 2017-03-14T14:37:44Z
JENA-1305: refactor TextIndex so that lang is passed as a separate argument
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request #226: Refactor TextIndex and TextQueryPF in jena-text (pre...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/jena/pull/226
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #226: Refactor TextIndex and TextQueryPF in jena-text (preparatio...
Posted by osma <gi...@git.apache.org>.
Github user osma commented on the issue:
https://github.com/apache/jena/pull/226
@anujgandharv forgot to ping you...see above
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #226: Refactor TextIndex and TextQueryPF in jena-text (preparatio...
Posted by anujgandharv <gi...@git.apache.org>.
Github user anujgandharv commented on the issue:
https://github.com/apache/jena/pull/226
Thanks @osma . I learned from you that: _It's always good to have Unit Tests for the changes to make sure we are not missing any corner cases :)_
So just to clarify, if the Sparql query string is:
```
SELECT ?s
{ ?s text:query (rdfs:label 'word' 10) ;
rdfs:label ?label
}
```
then the query method will receive the following attributes: `RDFS.label.asNode(), "word", null, null, 10`
And if the query string is: `?s text:query (rdfs:label 'word' 'lang:en' )`, then the query method will receive the following attributes: `RDFS.label.asNode(), "word", null, "en"`
And finally if the query string is: `?s text:query ('word' 'lang:en' )`, then the query method will receive the following attributes: `RDFS.label.asNode(), "word", null, "en"` , where the Node property is the default Node defined at the configuration time.
Can you confirm all the scenarios above please. I will modify my test cases and logic accordingly.
NOTE: I say NULL for graph attribute, mainly because I don't care about it in the ES implementation.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #226: Refactor TextIndex and TextQueryPF in jena-text (preparatio...
Posted by anujgandharv <gi...@git.apache.org>.
Github user anujgandharv commented on the issue:
https://github.com/apache/jena/pull/226
@osma I do not see any Unit/Integration tests updated for Lucene for the changes you have done in this PR. Do they come in a separate PR? In the mean time, can you provide me some examples as to how the information will flow into the query endpoint. Will the lang be like: `en` or `lang:en`. I assume it would be `en. Is that the correct assumption.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #226: Refactor TextIndex and TextQueryPF in jena-text (preparatio...
Posted by osma <gi...@git.apache.org>.
Github user osma commented on the issue:
https://github.com/apache/jena/pull/226
The jena-text tests are written on a higher level (queries written as SPARQL) so they were unaffected by this PR and continue to work after the changes.
The query method now takes the plain query without field name prefix, language (eg "en" or "none" or null) and graph as URI string.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena issue #226: Refactor TextIndex and TextQueryPF in jena-text (preparatio...
Posted by osma <gi...@git.apache.org>.
Github user osma commented on the issue:
https://github.com/apache/jena/pull/226
@anujgandharv All correct! And if in doubt, just try it out...
Thank the Jena project, I've learned the lesson myself while working on Jena!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---