"I guess the main take away is that if someone specifies valType and it
does *not* point to a FloatField, we should still probably error"

But that's just it. There's no way for the EFF to point to any underlying type
at all! Still, I can easily be persuaded that it'd be a bad thing to
allow, say, a
"string" type (which I just tested and it works just fine BTW) and later
pull the rug out from under someone if we get more strict.

I think I understand how this came to pass. There are tests (see schema11.xml
and TestFunctionQuery) that should have tripped this trigger. But schema11.xml
has
<fieldType name="float" class="solr.FloatField" omitNorms="true"/>
where more recent schemas have
<fieldType name="float" class="solr.TrieFloatField" omitNorms="true"/>

And since TrieFloatField isn't derived from FloatField, things go boom.

So, the take-away here is that my original hack is actually kind of
OK, but we need to
make valType optional and document it as a no-op currently. We'll still error if
they make it anything except FloatField or TrieFloatField.

I'll open a JIRA real soon now.


On Wed, Dec 14, 2011 at 2:54 PM, Erick Erickson <erickerick...@gmail.com> wrote:
> valType is NOT optional at all, at least in the 3x code line.
> You get errors like this on startup if you leave it out:
>
> Dec 14, 2011 2:07:48 PM org.apache.solr.common.SolrException log
> SEVERE: org.apache.solr.common.SolrException: Missing parameter
> 'valType' for FieldType=eff_float{keyField=id, defVal=1}
>        at org.apache.solr.schema.FieldType.getArg(FieldType.java:121)
>
>
> and Solr goes downhill from there.
>
> And now I see why I prompt with questions before diving in *too* far.
> Your comments made me look at the rest of the class (I mean it's
> another 25 lines down in the file, after all....) and there's this little
> gem of a method:
>
>  @Override
>  public ValueSource getValueSource(SchemaField field, QParser parser) {
>    // default key field to unique key
>    SchemaField keyField = keyFieldName==null ?
> schema.getUniqueKeyField() : schema.getField(keyFieldName);
>    return new FileFloatSource(field, keyField, defVal, parser);
>  }
>
> So the valType is, as you say, completely irrelevant because a FileFloatSource
> is returned no matter what.
>
> So I think I now propose to remove that check altogether. The code should
> still work whether the valType is specified or not  and make note that it
> ignored for now. This is really no change in behavior, just documenting what
> happens currently.
>
> I'm assuming that we should still work with schemas that define it rather
> than break someone's schema if it is there by, for instance, using a 3.5
> schema file with valType in an EFF field with new code and the proposed
> change.
>
>
> On Wed, Dec 14, 2011 at 1:59 PM, Chris Hostetter
> <hossman_luc...@fucit.org> wrote:
>>
>> :       if (ftype==null || !(ftype instanceof FloatField)) {
>> :         throw new SolrException(SolrException.ErrorCode.SERVER_ERROR,
>> : "Only float (FloatField) is currently supported as external field
>> : type.  got '" + ftypeS + "'");
>>        ...
>> : now that Trie fields support sortMissingFirst/Last, does it make sense
>> : to allow the "float" type too? I'm guessing here that
>> : sortMissingFirst/Last is why EFFs are restricted this way, but I
>> : haven't really dug into it.
>>
>> I don't think i've ever looked closely at the internals of this field type
>> before, but I'm not sure that it really matters what field types are
>> allowed there -- it's not used for anything as far as i can tell.  I can't
>> see any reason why the "valType" param exists at all (let alone the
>> insistence that it refer to a FloatField) the ExternalFileField completely
>> ignores the ftype once it has it.
>>
>> (The fact that valType is totally optional suggests that the limitation
>> isn't really that big of a deal anyway)
>>
>> I suspect the intent was that ExternalFileField *should* ultimatley be
>> able to return any "type" of ValueSource, and that the specified valType
>> would then dictate the ValueSource impl returned (so that if you used an
>> "IntField" you would get a ValueSource that "natively" dealt with int
>> values from the file, but if you used "DoubleField" it would natively
>> deal with double values.
>>
>> changing that validation code to also accept TrieFloatField
>> doesn't seem like it would practically make any different w/o also
>> changing a lot about how the actaul value source is returned -- either
>> by adding more File_____Source classes or my generalizing FileFloatSource.
>> barring actual changes to those classes, a better way to deal with the "no
>> one uses FloatField anymore so this limitation is confusing" problem is to
>> just stop promoting/suggestion people use the valType at all (until the
>> day comes when it's actually useful for something) and focus on
>> documenting the fact that ExternalFileFields reads and returns
>> values of type "float" from the specified file.
>>
>>
>>
>> -Hoss
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
>> For additional commands, e-mail: dev-h...@lucene.apache.org
>>

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

Reply via email to