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 2015/11/04 19:38:08 UTC
[GitHub] jena pull request: JENA-1062: configurable Lucene analyzer for jen...
GitHub user osma opened a pull request:
https://github.com/apache/jena/pull/97
JENA-1062: configurable Lucene analyzer for jena-text
This is a configurable Analyzer implementation for jena-text / Lucene. It is similar to what can be achieved in [Solr configuration](https://wiki.apache.org/solr/AnalyzersTokenizersTokenFilters). The current implementation only supports a few basic tokenizers and filters included in Lucene. More can be added later if necessary, though some tokenizers and filters require extra configuration parameters and currently there is no mechanism for specifying these.
Tokenizers:
* StandardTokenizer
* KeywordTokenizer
* WhitespaceTokenizer
* LetterTokenizer
Filters:
* StandardFilter
* LowerCaseFilter
* ASCIIFoldingFilter
Configuration can be done in the assembler like this:
```
text:analyzer [
a text:ConfigurableAnalyzer ;
text:tokenizer text:KeywordTokenizer ;
text:filters (text:ASCIIFoldingFilter, text:LowerCaseFilter)
]
```
When used directly from Java code, the ConfigurableAnalyzer accepts one String parameter specifying the Tokenizer name and another parameter which is a List<String> specifying (optional) filters in the order they should be applied.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/osma/jena jena-text-configurable-analyzer
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/jena/pull/97.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 #97
----
commit 168165a4d2801734fe2551c3c585ba118327e863
Author: Osma Suominen <os...@aalto.fi>
Date: 2015-11-04T18:32:03Z
JENA-1062: configurable Lucene analyzer for jena-text
----
---
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: JENA-1062: configurable Lucene analyzer for jen...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/97#discussion_r44015231
--- Diff: jena-text/src/main/java/org/apache/jena/query/text/assembler/ConfigurableAnalyzerAssembler.java ---
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.query.text.assembler;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.jena.assembler.Assembler;
+import org.apache.jena.assembler.Mode;
+import org.apache.jena.assembler.assemblers.AssemblerBase;
+import org.apache.jena.query.text.TextIndexException;
+import org.apache.jena.query.text.TextIndexLucene;
+import org.apache.jena.query.text.analyzer.ConfigurableAnalyzer;
+import org.apache.jena.rdf.model.RDFNode;
+import org.apache.jena.rdf.model.Resource;
+import org.apache.jena.rdf.model.Statement ;
+import org.apache.jena.vocabulary.RDF ;
+import org.apache.lucene.analysis.Analyzer;
+
+
+/**
+ * Assembler to create a configurable analyzer.
+ */
+public class ConfigurableAnalyzerAssembler extends AssemblerBase {
+ /*
+ text:map (
+ [ text:field "text" ;
+ text:predicate rdfs:label;
+ text:analyzer [
+ a text:ConfigurableAnalyzer ;
+ text:tokenizer text:LetterTokenizer ;
+ text:filters (text:LowerCaseFilter)
+ ]
+ ]
+ .
+ */
+
+
+ @Override
+ public Analyzer open(Assembler a, Resource root, Mode mode) {
+ if (root.hasProperty(TextVocab.pTokenizer)) {
+ Resource tokenizerResource = (Resource) root.getProperty(TextVocab.pTokenizer).getObject();
+ String tokenizer = tokenizerResource.getLocalName();
+ List<String> filters;
+ if (root.hasProperty(TextVocab.pFilters)) {
+ Resource filtersResource = (Resource) root.getProperty(TextVocab.pFilters).getObject();
+ filters = toFilterList(filtersResource);
+ } else {
+ filters = new ArrayList<>();
+ }
+ return new ConfigurableAnalyzer(TextIndexLucene.VER, tokenizer, filters);
+ } else {
+ throw new TextIndexException("text:tokenizer setting is required by ConfigurableAnalyzer");
+ }
+ }
+
+ private List<String> toFilterList(Resource list) {
+ List<String> result = new ArrayList<>();
+ Resource current = list;
+ while (current != null && ! current.equals(RDF.nil)){
+ Statement stmt = current.getProperty(RDF.first);
+ if (stmt == null) {
+ throw new TextIndexException("filter list not well formed");
+ }
+ RDFNode node = stmt.getObject();
+ if (! node.isResource()) {
+ throw new TextIndexException("filter is not a resource : " + node);
+ }
+
+ Resource res = (Resource)node;
--- End diff --
Isn't `Node::asResource` a little better here and below?
---
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: JENA-1062: configurable Lucene analyzer for jen...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/jena/pull/97
---
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: JENA-1062: configurable Lucene analyzer for jen...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the pull request:
https://github.com/apache/jena/pull/97#issuecomment-154099939
Sure, there's nothing wrong with what's there. All I meant is that `newHashSet` seems as clear as `new HashSet<>()` and allows elements, so is shorter.
---
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: JENA-1062: configurable Lucene analyzer for jen...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/97#discussion_r44012158
--- Diff: jena-text/src/main/java/org/apache/jena/query/text/analyzer/ConfigurableAnalyzer.java ---
@@ -0,0 +1,93 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.query.text.analyzer ;
+
+import java.io.Reader ;
+import java.util.List ;
+
+import org.apache.jena.query.text.TextIndexException;
+import org.apache.lucene.analysis.Analyzer ;
+import org.apache.lucene.analysis.TokenFilter ;
+import org.apache.lucene.analysis.Tokenizer ;
+import org.apache.lucene.analysis.TokenStream ;
+import org.apache.lucene.analysis.core.KeywordTokenizer ;
+import org.apache.lucene.analysis.core.LetterTokenizer;
+import org.apache.lucene.analysis.core.LowerCaseFilter ;
+import org.apache.lucene.analysis.core.WhitespaceTokenizer ;
+import org.apache.lucene.analysis.miscellaneous.ASCIIFoldingFilter;
+import org.apache.lucene.analysis.standard.StandardFilter;
+import org.apache.lucene.analysis.standard.StandardTokenizer;
+import org.apache.lucene.util.Version ;
+
+
+/**
+ * Lucene Analyzer implementation that can be configured with different
+ * Tokenizer and (optionally) TokenFilter implementations.
+ */
+
+public class ConfigurableAnalyzer extends Analyzer {
+ private Version version;
--- End diff --
Couldn't these files be `final`?
---
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: JENA-1062: configurable Lucene analyzer for jen...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on the pull request:
https://github.com/apache/jena/pull/97#issuecomment-154376006
Looks good. With a documentation modifications to follow? The text index seems to becoming quite popular.
---
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: JENA-1062: configurable Lucene analyzer for jen...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the pull request:
https://github.com/apache/jena/pull/97#issuecomment-154093794
@osma I am a nitpicker of the worst kind! {grin}
If you would like me to make a PR against `jena-text` looking for this kind of thing (using Guava or Java 8 idioms to shorten things up) I'm happy to do that sometime in the next week or three. I'm a born "code janitor".
---
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: JENA-1062: configurable Lucene analyzer for jen...
Posted by osma <gi...@git.apache.org>.
Github user osma commented on the pull request:
https://github.com/apache/jena/pull/97#issuecomment-154063083
@ajs6f You're right, I did that now.
---
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: JENA-1062: configurable Lucene analyzer for jen...
Posted by osma <gi...@git.apache.org>.
Github user osma commented on the pull request:
https://github.com/apache/jena/pull/97#issuecomment-157324689
Rebased on current master and squashed my commits into one, preparing to merge to Apache git
---
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: JENA-1062: configurable Lucene analyzer for jen...
Posted by osma <gi...@git.apache.org>.
Github user osma commented on the pull request:
https://github.com/apache/jena/pull/97#issuecomment-154353023
@ajs6f :
>If you would like me to make a PR against jena-text looking for this kind of thing (using Guava or Java 8 idioms to shorten things up) I'm happy to do that sometime in the next week or three. I'm a born "code janitor".
Sure, that would be great!
About this PR: good for merging? I can do the merging myself, but just wanted to check first with others.
---
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: JENA-1062: configurable Lucene analyzer for jen...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/97#discussion_r44015443
--- Diff: jena-text/src/test/java/org/apache/jena/query/text/TestDatasetWithConfigurableAnalyzer.java ---
@@ -0,0 +1,63 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.query.text;
+
+import java.util.Arrays ;
+import java.util.HashSet ;
+import java.util.Set ;
+
+import org.apache.jena.atlas.lib.StrUtils ;
+import org.junit.Before ;
+import org.junit.Test ;
+
+/**
+ * This class defines a setup configuration for a dataset that uses an ASCII folding lowercase keyword analyzer with a Lucene index.
+ */
+public class TestDatasetWithConfigurableAnalyzer extends TestDatasetWithLowerCaseKeywordAnalyzer {
+ @Override
+ @Before
+ public void before() {
+ init(StrUtils.strjoinNL(
+ "text:ConfigurableAnalyzer ;",
+ "text:tokenizer text:KeywordTokenizer ;",
+ "text:filters (text:ASCIIFoldingFilter text:LowerCaseFilter)"
+ ));
+ }
+
+ @Test
+ public void testConfigurableAnalyzerIsCaseAndAccentInsensitive() {
+ final String testName = "testConfigurableAnalyzerIsCaseAndAccentInsensitive";
+ final String turtle = StrUtils.strjoinNL(
+ TURTLE_PROLOG,
+ "<" + RESOURCE_BASE + testName + ">",
+ " rdfs:label 'Feeling a déjà vu'",
+ "."
+ );
+ String queryString = StrUtils.strjoinNL(
+ QUERY_PROLOG,
+ "SELECT ?s",
+ "WHERE {",
+ " ?s text:query ( rdfs:label '\"feeling ä déja\"*' 10 ) .",
+ "}"
+ );
+ Set<String> expectedURIs = new HashSet<>() ;
--- End diff --
Our shaded Guava offers various `Sets::newHashSet` functions for this kind of use, which read a pinch better.
---
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: JENA-1062: configurable Lucene analyzer for jen...
Posted by osma <gi...@git.apache.org>.
Github user osma commented on the pull request:
https://github.com/apache/jena/pull/97#issuecomment-154092054
Thanks @ajs6f for your detailed notes! I fixed the things you pointed out but I think especially the Guava Sets.newHashSet pattern could be applied in many other places in the jena-text code as well. My test case code was just adapted from similar other jena-text tests and they all use the same pattern. Fixing those as well isn't really in the scope for this PR but could be done separately.
---
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: JENA-1062: configurable Lucene analyzer for jen...
Posted by osma <gi...@git.apache.org>.
Github user osma commented on the pull request:
https://github.com/apache/jena/pull/97#issuecomment-154468394
Yes, of course I will also update the jena-text documentation.
---
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: JENA-1062: configurable Lucene analyzer for jen...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on the pull request:
https://github.com/apache/jena/pull/97#issuecomment-154098822
The test should be "is it clear?". `new HashSet<>()` is common usage.
---
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: JENA-1062: configurable Lucene analyzer for jen...
Posted by osma <gi...@git.apache.org>.
Github user osma commented on the pull request:
https://github.com/apache/jena/pull/97#issuecomment-153981033
Good questions @rvesse !
Right now (before this PR) one can either use a few generic, non-language-specific Analyzers: StandardAnalyzer, SimpleAnalyzer, KeywordAnalyzer and LowerCaseKeywordAnalyzer.
Then there is MultilingualAnalyzer, which looks at the language tag of literals and picks a language-specific Analyzer based on the language tag (falling back on StandardAnalyzer in case there's no suitable Analyzer implementation for the language). The list of language-specific Analyzers is hardwired in the implementation though.
What this adds is a non-language-specific Analyzer that can be configured in a little bit more detail: it is possible to select a Tokenizer and zero or more TokenFilters. However, it does not look at the language tags at all and it is also limited to a few recognized Tokenizers and Filters, none of which require any special parameters.
Things that were possible before:
* use StandardAnalyzer/SimpleAnalyzer/KeywordAnalyzer for everything
* use EnglishAnalyzer for "en" and FrenchAnalyzer for "fr" literals (MultilingualAnalyzer does this)
Things that become possible with this PR:
* use KeywordTokenizer (i.e. don't split into tokens), but drop accents with ASCIIFoldingFilter and make everything lowercase with LowerCaseFilter (my original use case for JENA-1058)
* use WhitespaceTokenizer without filters (perhaps good for handling e.g. a whitespace-separated list of product codes or URIs)
* dozens of other combinations of the non-language-specific Tokenizers and TokenFilters, though probably only some combinations make any sense
Things that are still not possible:
* use EnglishAnalyzer for "en" language but StandardAnalyzer for everything else (in MultilingualAnalyzer the analyzers are hardwired)
* use language-specific analyzers when available but fall back on SimpleAnalyzer (ditto, the fallback to StandardAnalyzers in hardwired in MultilingualAnalyzer)
* use StandardAnalyzer with LengthFilter to remove excessively short or long words (LengthFilter requires `min` and `max` parameters and there is no way to pass those parameters to ConfigurableAnalyzer, so it doesn't support LenghtFilter)
In short, the universe of Analyzers (Tokenizer + TokenFilter combinations, with or without special treatment for language tags) is potentially huge and this PR tackles only one rather small part of it, but it expands the options in a way that I think is useful.
---
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: JENA-1062: configurable Lucene analyzer for jen...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/97#discussion_r44013590
--- Diff: jena-text/src/main/java/org/apache/jena/query/text/assembler/ConfigurableAnalyzerAssembler.java ---
@@ -0,0 +1,101 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.jena.query.text.assembler;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import org.apache.jena.assembler.Assembler;
+import org.apache.jena.assembler.Mode;
+import org.apache.jena.assembler.assemblers.AssemblerBase;
+import org.apache.jena.query.text.TextIndexException;
+import org.apache.jena.query.text.TextIndexLucene;
+import org.apache.jena.query.text.analyzer.ConfigurableAnalyzer;
+import org.apache.jena.rdf.model.RDFNode;
+import org.apache.jena.rdf.model.Resource;
+import org.apache.jena.rdf.model.Statement ;
+import org.apache.jena.vocabulary.RDF ;
+import org.apache.lucene.analysis.Analyzer;
+
+
+/**
+ * Assembler to create a configurable analyzer.
+ */
+public class ConfigurableAnalyzerAssembler extends AssemblerBase {
+ /*
+ text:map (
+ [ text:field "text" ;
+ text:predicate rdfs:label;
+ text:analyzer [
+ a text:ConfigurableAnalyzer ;
+ text:tokenizer text:LetterTokenizer ;
+ text:filters (text:LowerCaseFilter)
+ ]
+ ]
+ .
+ */
+
+
+ @Override
+ public Analyzer open(Assembler a, Resource root, Mode mode) {
+ if (root.hasProperty(TextVocab.pTokenizer)) {
+ Resource tokenizerResource = (Resource) root.getProperty(TextVocab.pTokenizer).getObject();
--- End diff --
Would `Resource::getPropertyResourceValue` be more readable here? (No cast.)
---
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: JENA-1062: configurable Lucene analyzer for jen...
Posted by rvesse <gi...@git.apache.org>.
Github user rvesse commented on the pull request:
https://github.com/apache/jena/pull/97#issuecomment-153874598
Looks good to me
One open question, how does this interact with past work for language specific indexing and multi-lingual indexing in general?
It's been a while since I poked around Lucene but I seem to remember that it was often necessary to use alternative analysers particularly when you get into languages with compound words, non-Latin alphabets, symbolic alphabets etc. This shouldn't be a requirement for merging this work but just wanted to check that the current design won't preclude support for this in the future?
Looking over the code it looks like it should be relatively easy to add new analysers and filters as needed but just wanted to make sure I had understood the code correctly
---
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.
---