Mailing-List: contact dev-help@lucene.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@lucene.apache.org
Received-SPF: pass (nike.apache.org: domain of serera@gmail.com designates
 74.125.83.48 as permitted sender)
DomainKey-Signature: a=rsa-sha1; c=nofws;
        d=gmail.com; s=gamma;
        h=mime-version:date:message-id:subject:from:to:content-type;
        b=tyLFKMYz/6zd82RNsf1YwAa6/Wy3t4SfRkRlhdoeLvjEwp1Jt/kLW1D1+cwhozHWe4
         TQDNpLtY7KdIKf/h5uSzh1m93bN7T2C1PO19UM+lHtO5BLI5fPPsvEBMglVeVud7DJs7
         7WD0uMhIocX99PEEUDR8xVPjwKsmUfOsv0yIU=
MIME-Version: 1.0
Date: Tue, 18 May 2010 10:06:17 +0300
Message-ID: <AANLkTimIT037jheAVZUq2lU2ysFzMz7yWRH4O0UW5xrk@mail.gmail.com>
Subject: ReadTask and its hierarchy needs some house cleaning
From: Shai Erera <serera@gmail.com>
To: dev@lucene.apache.org
Content-Type: multipart/alternative; boundary=0016e68fd0644cf09f0486d8fa4f

--0016e68fd0644cf09f0486d8fa4f
Content-Type: text/plain; charset=ISO-8859-1

Hi

I wanted to run a benchmark .alg which will take a Filter into account.
However, ReadTask, which is the base for a variety of search related tasks,
does not support a Filter. When I reviewed the class, to understand how I
can easily add such Filter support, I discovered a whole set of classes
which IMO are completely unnecessary. ReadTask defines some with*() methods,
such as withSearch, withTraverse etc. and many classes override ReadTask
just to return true/false in those methods. WarmTask for example, returns
true in withWarm() and false otherwise, while SearchTask returns true in
withSearch and false otherwise.

This created a whole set of extensions that you either need to run in
sequence (e.g. Warm, SearchWithCollector) or create your own extension just
to get the right recipe for the operations to perform.

I suggest we do the following changes:
* Rename ReadTask to SearchTask -- that's because RT uses IndexSearcher,
QueryMaker -- all that suggests it's about Searching and not Reading. It's
only semantics, I know, but I think SearchTask is clearer than ReadTask
* Get rid of all the with*() methods, and instead move to use properties:
search.with.warm, search.with.traverse, search.with.collector etc.
* Introduce protected createCollector, createFilter, createSort, for custom
extensions
* Create a completely new hierarchy for this task, throwing away everything
that can be handled through properties only (like SearchTask, WarmTask etc.)

If we do this, then extensions of the new SearchTask will need to ask
themselves "do I want to search w/ a Collector/Filter/custom Sort?" and not
"do I Warm to be executed?" The core operation behind this task is
IndexSearcher.search. The rest are just settings, or configuration, as well
as some added ops like warm, and traverse. If it makes sense, I can factor
warm() and traverse() into their own protected methods, for extensions to
override as well. It might make sense for warm because custom warms is
something I'm sure will be needed.

This will also allow running algorithms with rounds - different properties
for different rounds.

This approach does not prevent one from creating MySearchTask with
pre-defined and hard-coded settings. But for many others, the question of
which task to execute will go away - you execute SearchTask for the basic
search operations, or w/ the default Collector/Sort, and you control it via
properties. To create your own *SearchTask extension which hard-codes a
recipe, you'll need access to all the do<OP> members, so I'll make them
protected. But that's IMO is a rare requirement, than say running a search
with warm + traverse, and you shouldn't be forced to create a ReadTask
extension for that.

What do you think?

Shai

--0016e68fd0644cf09f0486d8fa4f
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr">Hi<br><br>I wanted to run a benchmark .alg which will take=
 a Filter into account. However, ReadTask, which is the base for a variety =
of search related tasks, does not support a Filter. When I reviewed the cla=
ss, to understand how I can easily add such Filter support, I discovered a =
whole set of classes which IMO are completely unnecessary. ReadTask defines=
 some with*() methods, such as withSearch, withTraverse etc. and many class=
es override ReadTask just to return true/false in those methods. WarmTask f=
or example, returns true in withWarm() and false otherwise, while SearchTas=
k returns true in withSearch and false otherwise.<br>
<br>This created a whole set of extensions that you either need to run in s=
equence (e.g. Warm, SearchWithCollector) or create your own extension just =
to get the right recipe for the operations to perform.<br><br>I suggest we =
do the following changes:<br>
* Rename ReadTask to SearchTask -- that&#39;s because RT uses IndexSearcher=
, QueryMaker -- all that suggests it&#39;s about Searching and not Reading.=
 It&#39;s only semantics, I know, but I think SearchTask is clearer than Re=
adTask<br>
* Get rid of all the with*() methods, and instead move to use properties: s=
earch.with.warm, search.with.traverse, search.with.collector etc.<br>* Intr=
oduce protected createCollector, createFilter, createSort, for custom exten=
sions<br>
* Create a completely new hierarchy for this task, throwing away everything=
 that can be handled through properties only (like SearchTask, WarmTask etc=
.)<br><br>If we do this, then extensions of the new SearchTask will need to=
 ask themselves &quot;do I want to search w/ a Collector/Filter/custom Sort=
?&quot; and not &quot;do I Warm to be executed?&quot; The core operation be=
hind this task is IndexSearcher.search. The rest are just settings, or conf=
iguration, as well as some added ops like warm, and traverse. If it makes s=
ense, I can factor warm() and traverse() into their own protected methods, =
for extensions to override as well. It might make sense for warm because cu=
stom warms is something I&#39;m sure will be needed.<br>
<br>This will also allow running algorithms with rounds - different propert=
ies for different rounds.<br><br>This approach does not prevent one from cr=
eating MySearchTask with pre-defined and hard-coded settings. But for many =
others, the question of which task to execute will go away - you execute Se=
archTask for the basic search operations, or w/ the default Collector/Sort,=
 and you control it via properties. To create your own *SearchTask extensio=
n which hard-codes a recipe, you&#39;ll need access to all the do&lt;OP&gt;=
 members, so I&#39;ll make them protected. But that&#39;s IMO is a rare req=
uirement, than say running a search with warm + traverse, and you shouldn&#=
39;t be forced to create a ReadTask extension for that.<br>
<br>What do you think?<br><br>Shai<br></div>

--0016e68fd0644cf09f0486d8fa4f--