mikemccand commented on a change in pull request #225:
URL: https://github.com/apache/lucene/pull/225#discussion_r725023003
##########
File path: lucene/core/src/java/org/apache/lucene/search/AutomatonQuery.java
##########
@@ -96,12 +110,36 @@ public AutomatonQuery(final Term term, Automaton
automaton, int determinizeWorkL
*/
public AutomatonQuery(
final Term term, Automaton automaton, int determinizeWorkLimit, boolean
isBinary) {
+ this(term, automaton, determinizeWorkLimit, isBinary,
ByteRunnable.TYPE.DFA);
+ }
+
+ /**
+ * Create a new AutomatonQuery from an {@link Automaton}.
+ *
+ * @param term Term containing field and possibly some pattern structure.
The term text is
+ * ignored.
+ * @param automaton Automaton to run, terms that are accepted are considered
a match.
+ * @param determinizeWorkLimit maximum effort to spend determinizing the
automaton. If the
+ * automaton will need more than this much effort,
TooComplexToDeterminizeException is thrown.
+ * Higher numbers require more space but can process more complex
automata.
+ * @param isBinary if true, this automaton is already binary and will not go
through the
+ * UTF32ToUTF8 conversion
+ * @param runnableType NFA or DFA. See {@link
org.apache.lucene.util.automaton.ByteRunnable.TYPE}
+ * for difference between NFA and DFA. Also note * that NFA has
uncertain performance impact
Review comment:
Remove that errant `*` between `note` and `that`?
##########
File path:
lucene/core/src/java/org/apache/lucene/util/automaton/ByteRunnable.java
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.lucene.util.automaton;
+
+/** A runnable automaton accepting byte array as input */
+public interface ByteRunnable {
+
+ /** NFA or DFA */
+ enum TYPE {
+ /**
+ * Determinize the automaton lazily on-demand as terms are intersected.
This option saves the
+ * up-front determinize cost, and can handle some RegExps that DFA cannot,
but intersection will
+ * be a bit slower
Review comment:
Missing period at the end of the sentence?
Maybe link to Russ Cox's famous page
(https://swtch.com/~rsc/regexp/regexp1.html) and point out that this is similar
to the Thompson NFA approach described there?
##########
File path:
lucene/codecs/src/java/org/apache/lucene/codecs/memory/DirectPostingsFormat.java
##########
@@ -962,15 +964,22 @@ public ImpactsEnum impacts(int flags) throws IOException {
private int stateUpto;
public DirectIntersectTermsEnum(CompiledAutomaton compiled, BytesRef
startTerm) {
- runAutomaton = compiled.runAutomaton;
- compiledAutomaton = compiled;
+ if (compiled.nfaRunAutomaton != null) {
+ this.runAutomaton = compiled.nfaRunAutomaton;
Review comment:
OK we can wait on this. Maybe just add a comment explaining why we need
this odd `if` still, here and in the other places where we did this.
##########
File path: lucene/core/src/java/org/apache/lucene/search/AutomatonQuery.java
##########
@@ -65,7 +66,20 @@
* @param automaton Automaton to run, terms that are accepted are considered
a match.
*/
Review comment:
Could you update these javadocs to state that the `runnableType` is
`DFA` by default, and point to the `ByteRunnable.TYPE` javadocs?
##########
File path:
lucene/core/src/java/org/apache/lucene/util/automaton/ByteRunnable.java
##########
@@ -0,0 +1,74 @@
+/*
+ * 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.lucene.util.automaton;
+
+/** A runnable automaton accepting byte array as input */
+public interface ByteRunnable {
+
+ /** NFA or DFA */
+ enum TYPE {
Review comment:
Could you add `@lucene.experimental` to the `TYPE` javadocs, and on each
of the options (`NFA`, `DFA`)?
Also, could you pull out this `enum` into its own class under
`o.a.l.util.automaton`, maybe `AutomatonExecutionMode` or
`AutomatonIntersectionMode` or `AutomatonExecutionStrategy` (getting longish to
type...)? I think it is too obscure living down inside this non-consumable (to
Lucene users who don't know all sorts of details about automata) `ByteRunnable`
now.
##########
File path:
lucene/backward-codecs/src/java/org/apache/lucene/backward_codecs/lucene40/blocktree/FieldReader.java
##########
@@ -187,6 +187,14 @@ public TermsEnum intersect(CompiledAutomaton compiled,
BytesRef startTerm) throw
if (compiled.type != CompiledAutomaton.AUTOMATON_TYPE.NORMAL) {
throw new IllegalArgumentException("please use
CompiledAutomaton.getTermsEnum instead");
}
+ if (compiled.nfaRunAutomaton != null) {
+ return new IntersectTermsEnum(
Review comment:
Oh, nevermind! I got my classes confused :) With this new `if`
statement, we are in fact still using `BlockTree`'s fast intersect
implementation, great! Please disregard...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
