Re: svn commit: r514254 - in /lucene/solr/trunk/src/java/org/apache/solr/request: JSONResponseWriter.java TextResponseWriter.java XMLWriter.java

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r514254 - in /lucene/solr/trunk/src/java/org/apache/solr/request: JSONResponseWriter.java TextResponseWriter.java XMLWriter.java

Chris Hostetter-3

One thing about this commit threw me for a loop, i'm not understanding
the purpose/affect of this change, off the cuff it seeems like it's
changing an unexpected field (not in the schema) from being an error to
being treated as if it were plain text.   am i correct?  if so was this
intentionally part of the patch?  does it lead to any dangerous cases?



: --- lucene/solr/trunk/src/java/org/apache/solr/request/XMLWriter.java (original)
: +++ lucene/solr/trunk/src/java/org/apache/solr/request/XMLWriter.java Sat Mar  3 14:21:27 2007
: @@ -25,6 +25,7 @@
:  import org.apache.solr.search.DocSet;
:  import org.apache.solr.schema.IndexSchema;
:  import org.apache.solr.schema.SchemaField;
: +import org.apache.solr.schema.TextField;
:
:  import java.io.Writer;
:  import java.io.IOException;
: @@ -290,8 +291,10 @@
:        FieldType ft = schema.getFieldType(fname);
:        ***/
:
: -      SchemaField sf = schema.getField(fname);
: -
: +      SchemaField sf = schema.getFieldOrNull(fname);
: +      if( sf == null ) {
: +        sf = new SchemaField( fname, new TextField() );
: +      }
:        if (fidx1+1 == fidx2) {
:          // single field value
:          if (version>=2100 && sf.multiValued()) {




-Hoss

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r514254 - in /lucene/solr/trunk/src/java/org/apache/solr/request: JSONResponseWriter.java TextResponseWriter.java XMLWriter.java

Ryan McKinley
On 3/3/07, Chris Hostetter <[hidden email]> wrote:
>
> One thing about this commit threw me for a loop, i'm not understanding
> the purpose/affect of this change, off the cuff it seeems like it's
> changing an unexpected field (not in the schema) from being an error to
> being treated as if it were plain text.   am i correct?  if so was this
> intentionally part of the patch?  does it lead to any dangerous cases?
>

oops, sorry about that, this was part of something else.  I should
have been more careful.

For the 'luke' patch, i was looking at indexes that don't match the
schema.  Rather then throw an error, it just treats the field as a
text - while i think we should have errors while putting things into
the index, it is nice to be able to view any index with solr - even
when the contents don't match the schema.

good eye!

ryan
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r514254 - in /lucene/solr/trunk/src/java/org/apache/solr/request: JSONResponseWriter.java TextResponseWriter.java XMLWriter.java

Yonik Seeley-2
On 3/4/07, Ryan McKinley <[hidden email]> wrote:
> For the 'luke' patch, i was looking at indexes that don't match the
> schema.  Rather then throw an error, it just treats the field as a
> text - while i think we should have errors while putting things into
> the index, it is nice to be able to view any index with solr - even
> when the contents don't match the schema.

Yes, I agree.  It will aid in debugging rather than hindering I think.

-Yonik
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r514254 - in /lucene/solr/trunk/src/java/org/apache/solr/request: JSONResponseWriter.java TextResponseWriter.java XMLWriter.java

Chris Hostetter-3

: > schema.  Rather then throw an error, it just treats the field as a
: > text - while i think we should have errors while putting things into
: > the index, it is nice to be able to view any index with solr - even
: > when the contents don't match the schema.
:
: Yes, I agree.  It will aid in debugging rather than hindering I think.

the two use cases i had floating in my had that concerned me were...

1) a field that use to be in the schema nad has since been removed, but
not all docs with thta field have been removed/reindexed ... if that doc
poped up in a search result it use to be an error, now you get the field
back ... which could trigger odd behavior in the client.

but this probably isn't htat bad because clients that are paranoid about
getting fields they don't expect should ask for an explicit field list.

2) unprintable characters...  don't we have FieldTypes that encode their
values using byte sequences that aren't valid unicode?
(TextField.storedToReadable is a NOOP right?)


-Hoss

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r514254 - in /lucene/solr/trunk/src/java/org/apache/solr/request: JSONResponseWriter.java TextResponseWriter.java XMLWriter.java

Chris Hostetter-3

: : > schema.  Rather then throw an error, it just treats the field as a
: : > text - while i think we should have errors while putting things into
: : > the index, it is nice to be able to view any index with solr - even
: : > when the contents don't match the schema.
: :
: : Yes, I agree.  It will aid in debugging rather than hindering I think.

I almost forgot ... if we want to leave it in, we should document it in
CHANGES and make the corrisponding change in TextResponseWriter too right?



-Hoss

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r514254 - in /lucene/solr/trunk/src/java/org/apache/solr/request: JSONResponseWriter.java TextResponseWriter.java XMLWriter.java

Yonik Seeley-2
On 3/4/07, Chris Hostetter <[hidden email]> wrote:
> I almost forgot ... if we want to leave it in, we should document it in
> CHANGES and make the corrisponding change in TextResponseWriter too right?

Change should definitely be made to TextResponseWriter too.
I had planned at some point to move the XMLResponseWriter to use
TextResponseWriter.

As far as CHANGES, do you think people have actually encountered this?
If so, I guess it should go in there.

-Yonik
Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r514254 - in /lucene/solr/trunk/src/java/org/apache/solr/request: JSONResponseWriter.java TextResponseWriter.java XMLWriter.java

Chris Hostetter-3

: As far as CHANGES, do you think people have actually encountered this?
: If so, I guess it should go in there.

i'm pretty sure i've seen people say: "oh shit there are docs in the index
that still have that old field i just removed? ... damn i better go fix
that" ... so describing hte hcange in behavior is probably wise.


-Hoss