jpountz commented on PR #14213: URL: https://github.com/apache/lucene/pull/14213#issuecomment-2649287798
> I went with this initial approach as it aligns with the fact that StoredFieldsWriter already supports DataInput (seemingly for merges). Indeed, the change that introduced this capability had a similar motivation to yours: #12581. > I wrapped the DataInput in a record style class StoredFieldDataInput to associated length with it. I'm curious if we should make it an actual record? > I was uncertain whether Lucene would want DataInput to be fully supported in Field similar to stringValue, readerValue, doubleValue, etc, I like what you did better. I don't want to have to deal with the case when a value is provided through a `DataInput` and needs to be analyzed / indexed. > Finally, would we want to modify StoredFieldsWriter#writeField(FieldInfo info, DataInput value, int length) to use this StoredFieldDataInput abstraction It's slightly awkward to have an argument called "value" that doesn't fully encapsulate all the information that is necessary to know what the actual value is, so I like your proposal to modify it. > DataInput is only one potential approach. I took it because there was already some work around DataInput with stored fields. This looks like the best approach to me, I like the symmetry with the read API in `StoredFieldVisitor`. In general the change makes sense to me, I'm curious to get @iverase 's opinion since he's worked on the same change on the read side (`StoredFieldsVisitor`). It is also consistent with the fact that analyzed fields support passing a `java.io.Reader` instead of a fully materialized `String`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org
