[ 
https://issues.apache.org/jira/browse/LUCENE-5215?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13777189#comment-13777189
 ] 

Robert Muir commented on LUCENE-5215:
-------------------------------------

initial thoughts after one pass:
much simpler than I imagined...

the changes to FieldInfosReader/Writer seem intuitive and obvious! this looks 
great.

thanks for taking care of the new 46Codec, i know how much of a pain this is. 
admittedly i didnt do a review "e.g. search for 4.5, Lucene45, such strings in 
eclipse", but thats normally the stuff i do here. I dont see problems, just be 
paranoid :)
I'm glad to see the feature listed under Lucene46Codec as a major structural 
change to the index.

In perFieldDocValuesFormat where we have suffixAtt = 
Integer.valueOf(suffixAtt);, do we have any concerns? since its serialized as a 
string att, maybe we should upgrade
to long now and have no worries? Then again we can always do this later...

The changes to R&LDocs i wont attempt here, thats more Mike's forte anyway! 
Unfortunately this is the key to the whole patch and the most important thing 
to think about, ill try to apply it and take a second swipe unless mike beats 
me to it...

One thing i noticed from API perspective: the SegmentReader.readFieldInfos 
seems an awkward place to me for this: must it really be public or can it be 
package-private? I dont want to block the change on this (especially as I have 
no obvious clear suggestion to give), but I'm just worried its the right place.

wherever you see @Deprecated (eg Lucene45Codec) ensure @deprecated <reason> in 
javadocs too.

                
> Add support for FieldInfos generation
> -------------------------------------
>
>                 Key: LUCENE-5215
>                 URL: https://issues.apache.org/jira/browse/LUCENE-5215
>             Project: Lucene - Core
>          Issue Type: New Feature
>          Components: core/index
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>         Attachments: LUCENE-5215.patch, LUCENE-5215.patch, LUCENE-5215.patch, 
> LUCENE-5215.patch, LUCENE-5215.patch, LUCENE-5215.patch
>
>
> In LUCENE-5189 we've identified few reasons to do that:
> # If you want to update docs' values of field 'foo', where 'foo' exists in 
> the index, but not in a specific segment (sparse DV), we cannot allow that 
> and have to throw a late UOE. If we could rewrite FieldInfos (with 
> generation), this would be possible since we'd also write a new generation of 
> FIS.
> # When we apply NDV updates, we call DVF.fieldsConsumer. Currently the 
> consumer isn't allowed to change FI.attributes because we cannot modify the 
> existing FIS. This is implicit however, and we silently ignore any modified 
> attributes. FieldInfos.gen will allow that too.
> The idea is to add to SIPC fieldInfosGen, add to each FieldInfo a dvGen and 
> add support for FIS generation in FieldInfosFormat, SegReader etc., like we 
> now do for DocValues. I'll work on a patch.
> Also on LUCENE-5189, Rob raised a concern about SegmentInfo.attributes that 
> have same limitation -- if a Codec modifies them, they are silently being 
> ignored, since we don't gen the .si files. I think we can easily solve that 
> by recording SI.attributes in SegmentInfos, so they are recorded per-commit. 
> But I think it should be handled in a separate issue.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

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

Reply via email to