lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael McCandless <luc...@mikemccandless.com>
Subject Re: FieldType refactoring?
Date Tue, 18 Oct 2011 18:47:29 GMT
Hi Adnan,

I'm glad to hear you are using trunk; this is a good feedback!

I agree about creating both StoredTextField and TextField (at the edge
of the API): that's much easier to consume than what you now must do
for the stored case (new Field("name", "value",
TextField.STORED_TYPE).  I also really don't like how you don't know
if StringField / TextField is stored by default.  Having XField and
StoredXField would make that clear.

So, regardless of how we create FTs/IFTs behind the scenes, I agree we
should have separate sugar classes for StoredXField and XField.

The expectation is the vast majority of users ("normal" users) will
use the sugar field classes (Stored/StringField, Stored/TextField,
NumericField, BinaryField, etc.) and never have to work with FieldType
classes.

Then, expert users are able to create arbitrary FieldType instances or
way-expert can just make their own impls of IndexableFieldType (and
IndexableField), ie bypassing Document/Field entirely.

I agree it'd be nice to have immutable IFT instances, as you suggest,
and then have no FieldType class at all.  The challenge is the API for
creating such IFT instances (see the [long] discussion towards the end
of LUCENE-2308)... basically the committers have strong disagreements
on how to instantiate them (builder APIs vs
ctor-taking-tons-of-boolean args APIs vs mutable + freeze).

That said, I think the int bit flags approach (suggested on
LUCENE-2308 but nobody has followed up on / worked out patch yet) is a
good compromise, since nearly all of these settings are booleans.  So
we could have something like:

  new FieldType(INDEXED | STORED)

So then FieldType would no longer be mutable, and it'd be a concrete
IFT impl that uses bit flags under the hood to record the details for
the type.

Mike McCandless

http://blog.mikemccandless.com

On Tue, Oct 18, 2011 at 11:43 AM, Adnan Duric <aduric@gmail.com> wrote:
> Hi guys,
> First of all, this is a fantastic project. I'm having a great time playing
> around with it.
> I downloaded and compiled the trunk and I initially started trying to get
> the examples working but quickly realized that the trunk version was
> different than the previous versions. Anyway, long story short, I managed to
> get it working.
> However, I'd just like to comment on the functionality where the user adds
> Field instances to a Document. Each Field has a name, value and a FieldType.
> I got a little confused when I began to create a FieldType.
> The FieldType class implements an IndexableFieldType, so it's effectively an
> IndexableFieldType type but it also uses the same type to construct the
> actual FieldType. This confused me because when I created my own FieldType,
> I had to create a concrete implementation of IndexableFieldType and then
> pass it to FieldType. This also means that there are two ways to create
> IndexableFieldType types:
> IndexableFieldType ift = new FieldType(new MyFieldType());
> IndexableFieldType ift2 = new MyFieldType();
> Then I noticed that FieldType just contains mutators to set the various
> members of IndexableFieldType plus a toString() method. After seeing the
> freeze() method, I began to realize that when a concrete FieldType is
> created, some dependent objects have a chance to mutate the FieldType, and
> then it should be immutable. These objects seem to be the various instances
> of type IndexableField such as TextField, StringField, NumericField, etc...
> Upon examination, I realized that when these classes are instantiated, they
> create a FieldType and mutate some of its fields before freeze()'ing it. I
> tried to see if I could somehow mutate after freezing but I couldn't find a
> way to do it.
> I instantiated a TextField and added to a Document:
> IndexableField txtfield = new TextField(name, value);
> doc.add(txtfield);
> However, my test failed since the IndexableFieldType in txtfield was set to
> TYPE_UNSTORED by default, and I guess I needed a TYPE_STORED. The only way
> to have a "stored" version of TextField was to create a Field and set it to
> a "stored" TextField like this:
> doc.add(new Field("fieldname", text, TextField.TYPE_STORED));
> The STORED versions for each class have to be created explicitly by going to
> the Field directly. This got me thinking about how to consolidate the two
> FieldTypes. Then I thought about getting rid of FieldType altogether since
> it's just a mutator for IndexableFieldTypes, and having each concrete
> IndexableField type (TextField, NumericField) constructed by directly
> calling an IndexableFieldType. Since there seems to be "two" main
> FieldTypes, maybe we could have a StoredFieldType and UnstoredFieldType that
> implement IndexableFieldType:
> final class StoredFieldType implements IndexableFieldType {
>     public boolean indexed() { return true; }
>     public boolean stored() { return true; }
>     public boolean tokenized() { return true; }
>
> .
> .
> .
>
> }
> final class UnstoredFieldType implements IndexableFieldType { ... }
> And then have each IndexableField that is used, itself instantiate one of
> the above IndexableFieldType. For example the TextField(String, String)
> constructot:
> /** Creates a new un-stored TextField */
> public TextField(String name, String value) {
>
> super(name, value, new UnstoredFieldType());
>
> }
> This way, the FieldType class is no longer needed, the IndexableField type
> classes don't need to mutate anything, and, in the end, are instantiated the
> same way. Of course, for the "stored" TextField, there needs to be a
> StoredTextField class that instantiates a StoredFieldType, but I'm willing
> to argue that it is better to have this duality appear closer to the 'edge'
> of the API. Another advantage is that an entire hierarchy can be built with
> IndexableFieldTypes by using a factory that creates them based on their
> parameters.
> Anyway, I'm probably missing some details and I'd appreciate some feedback.
>
> Thanks for your time and keep up the great work!
>
> Adnan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org


Mime
View raw message