[
https://issues.apache.org/jira/browse/SOLR-2671?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13069996#comment-13069996
]
Hoss Man commented on SOLR-2671:
--------------------------------
bq. I don't think we should pass null to the lucene Field (its not a valid
value for the enum).
It's not a valid value on the enum, but it is seems like it should be a valid
value indicating that *no* IndexOptions should be used
(AbstractField.setIndexOptions doesn't seem to reject null as valid input)
Either that or there should be an explicit IndexOptions.NONE value (which kind
of makes me wonder why we have both FieldInfos.IndexOptions and Field.Index ...
seems like maybe they should be one Class with setters for the optional parts)
Ugh... now my head hurts.
Bottom line: if null isn't a valid value to pass to setIndexOptions, then
setIndexOptions should be doced as such and throw IllegalArguementException ...
right? because otherwise it seems totally normal to say "I'm not indexing this
field, therefor i'm specifying null IndexOptions"
bq. Speaking of omitNorms and Boost, I don't think we should set indexOptions
to null, unless we do the same with these consistently.
Hmmm .... i see what you're saying, and i agree they createField shouldn't set
these on the Field object if !indexed, ala..
{code}
protected Fieldable createField(String name, String val, Field.Store storage,
Field.Index index,
Field.TermVector vec, boolean omitNorms,
IndexOptions options, float boost){
Field f = new Field(name,
val,
storage,
index,
vec);
if (index.isIndexed()) {
f.setOmitNorms(omitNorms);
f.setIndexOptions(options);
f.setBoost(boost);
}
return f;
}
{code}
...but i'm not sure it makes sense to make them nullable...
In the boost case, it's not a property of the SchemaField, it comes at runtime
when the Field instances are being created, and the client code (ie: The
Request Handler adding docs) doesn't know (and isn't expected to check) whether
a the SchemaField it's using specifies index=true/false.
So even if we change boost to be a Boolean, your reasoning still holds that the
if the field is !indexed, that boost shouldn't be used (null or otherwise).
I suppose from a backcompat standpoint there's not much harm in changing the
FieldType.createField args to make it Boolean instead of boolean (all the
calling code should still work because of autoboxing) i'm just not sure i see
much advantage.
In the case of SchemaField.omitNorms -- and all the term vector methods on
SchemaField as well -- those are simple accessors for the properties. It
doesn't seem like a good idea to make them "smart" about indexed, they just
report wether the property was or was not set when the SchemaField was created.
The diff between SchemaField.omitNorms() and SchemaField.indexOptions() is that
indexOptions isn't a direct accessor for any one property, it returns the
effective IndexOptions object used based on two properties (which don't have
their own accessors). So the question of whether indexOptions() should return
null seems orthogonal to whether omitNorms() should return null. But it also
makes me realize that for consistency we should really add explicit
termFreqAndPosition() and termPosition() accessors to be consistent with
omitNorms() ... which leaves us back deciding on how indexOptions() (as a
"smart" accessor) should behave.
Now my head really hurts.
It seems like maybe the crux of the issue is really:
* add termFreqAndPosition() and termPosition() methods to SchemaField to be
consistent with the other explicit properties
* add docs in AbstractField, SchemaField, and FieldType.createField on which
things are meaningless if indexed=false -- just putting "(value|behavior) is
(meaningless|undefined) if #isIndexed() is false" in the javadocs on most of
those get/set methods would be a big win for now)
* better docs on IndexOptions / AbstractField about null not being a valid
value for IndexOptions (unless i've convinced you that it should be)
* an IllegalArguementException on any method that takes an IndexOptions arg if
that object is null
* change FieldType.createField as described above to only call those Field
setters "if indexed()"
...if we do that, then we can leave SchemaField.indexOptions() alone and it
will be clear from the docs that the value returned is meaningless for non
indexed fields.
what do you think?
> SchemaField.indexOptions() should return null if SchemaField.indexed()
> returns false
> ------------------------------------------------------------------------------------
>
> Key: SOLR-2671
> URL: https://issues.apache.org/jira/browse/SOLR-2671
> Project: Solr
> Issue Type: Improvement
> Reporter: Hoss Man
> Fix For: 3.4, 4.0
>
>
> It doesn't make sense for SchemaField.indexOptions() to return any of the
> values from the IndexOptions enum if the field is not going to be indexed at
> all.
> we should return null to prevent missleading any code that calls that method
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
