nick-boss-tech commented on code in PR #4965: URL: https://github.com/apache/solr/pull/4965#discussion_r4179250467
########## changelog/unreleased/SOLR-10198.yml: ########## @@ -0,0 +1,7 @@ +title: Normalize Lucene stored fields to SolrJ-native types in JavaBin responses, including EmbeddedSolrServer streaming. Review Comment: 🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)* Applied, thank you. You were right on the scope too. I re-checked the claim in the code before changing it: on the serialized path the codec passes each field value through the Resolver, and the Resolver already converts an `IndexableField` through `DocsStreamer.getValue` ([JavaBinResponseWriter.java](https://github.com/nick-boss-tech/solr/blob/08ba904e4db089af91f23e84be31c23602d32fc4/solr/core/src/java/org/apache/solr/response/JavaBinResponseWriter.java#L107-L116)). The streaming callback was the only path that missed it, because it receives the document itself rather than serialized bytes. So the `JavaBinResponseWriter` change is reverted (that file is back to its base contents), and the conversion now runs only in the EmbeddedSolrServer streaming codec, just before the document reaches the callback ([EmbeddedSolrServer.java](https://github.com/nick-boss-tech/solr/blob/08ba904e4db089af91f23e84be31c23602d32fc4/solr/core/src/java/org/apache/solr/client/solrj/embedded/EmbeddedSolrServer.java#L383-L387)). Pushed in 08ba904e4db. I also updated the PR title and description to match the narrower scope. ########## solr/core/src/java/org/apache/solr/response/DocsStreamer.java: ########## @@ -220,7 +234,12 @@ public static SolrDocument externalizeStoredValues(SolrDocument doc, IndexSchema private static Object externalizeValue(Object val, IndexSchema schema) { if (val instanceof IndexableField f) { - return getValue(schema.getFieldOrNull(f.name()), f); + try { + return getValue(schema.getFieldOrNull(f.name()), f); + } catch (Exception e) { Review Comment: 🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)* The try wraps the conversion itself, `DocsStreamer.getValue`, which runs `FieldType.toObject` on the stored value ([DocsStreamer.java](https://github.com/nick-boss-tech/solr/blob/08ba904e4db089af91f23e84be31c23602d32fc4/solr/core/src/java/org/apache/solr/response/DocsStreamer.java#L238-L242)). It throws when a stored value cannot be converted under the field's current type; the classic case is data indexed before a schema change of that field's type. The Resolver on the serialized path has the same catch and the same log line ([JavaBinResponseWriter.java](https://github.com/nick-boss-tech/solr/blob/08ba904e4db089af91f23e84be31c23602d32fc4/solr/core/src/java/org/apache/solr/response/JavaBinResponseWriter.java#L111-L115)); there it continues with the raw value. Here the value is omitted instead, because the callback would otherwise receive the raw `IndexableField`, which is the type mismatch this change removes. The rest of the document still converts. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
