nick-boss-tech opened a new pull request, #5077: URL: https://github.com/apache/solr/pull/5077
🤖 *AI text below* 🤖 *(posted on behalf of Nick Shanin)* https://issues.apache.org/jira/browse/SOLR-6045 ## What happens today **An atomic update that adds one field twice does not apply its operations. The maps are indexed as text.** With the javabin writer, two `addField` calls with operation maps for one field leave `{set=new_value1}` and `{set=new_value2}` in the index. The factory wraps the whole list in one operation ([factory](https://github.com/nick-boss-tech/solr/blob/bcae04d77bdbb33c33e077c3945a5deca0ce7dc1/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateProcessorFactory.java#L139)). On the base, the merger checks only the whole field value, a list here, so it finds no operation map ([base check](https://github.com/nick-boss-tech/solr/blob/97d973814336101e12475558d7419321c743de79/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java#L88-L97)). ## What this change does **Repeated operation maps are applied in the order the fields were added. A field that mixes plain values with operation maps is rejected, whichever value comes first.** - The factory leaves a field alone when all its values are operation maps ([helper](https://github.com/nick-boss-tech/solr/blob/bcae04d77bdbb33c33e077c3945a5deca0ce7dc1/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateProcessorFactory.java#L161-L177)). - The merger treats a field as atomic when any of its values is an operation map ([atomic check](https://github.com/nick-boss-tech/solr/blob/bcae04d77bdbb33c33e077c3945a5deca0ce7dc1/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java#L88-L96)). It collects the maps in value order ([collect](https://github.com/nick-boss-tech/solr/blob/bcae04d77bdbb33c33e077c3945a5deca0ce7dc1/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java#L122-L141)) and applies each one ([apply](https://github.com/nick-boss-tech/solr/blob/bcae04d77bdbb33c33e077c3945a5deca0ce7dc1/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java#L201-L202)). - The scan reads every value. A field with a null value has no values, so the scan skips it ([null guard](https://github.com/nick-boss-tech/solr/blob/bcae04d77bdbb33c33e077c3945a5deca0ce7dc1/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java#L109-L113)). - The mixed-field check raises a bad request error that lists the field's values ([check](https://github.com/nick-boss-tech/solr/blob/bcae04d77bdbb33c33e077c3945a5deca0ce7dc1/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java#L130-L137)). - A child document is a map too, but the merger does not read it as an operation map ([map check](https://github.com/nick-boss-tech/solr/blob/bcae04d77bdbb33c33e077c3945a5deca0ce7dc1/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java#L100)). In a field that also has operation maps, it is a plain value and fails the mixed-field check. On the base, a single child document in an update with operations failed with "Unknown operation" ([base](https://github.com/nick-boss-tech/solr/blob/97d973814336101e12475558d7419321c743de79/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java#L157-L158), [base](https://github.com/nick-boss-tech/solr/blob/97d973814336101e12475558d7419321c743de79/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java#L186)). Where its field has no operation maps, it is now stored as a plain value, as a list of child documents already was. - A JSON array of operation objects is now applied as atomic operations. On base the objects were stored as values ([array parser](https://github.com/nick-boss-tech/solr/blob/bcae04d77bdbb33c33e077c3945a5deca0ce7dc1/solr/core/src/java/org/apache/solr/handler/loader/JsonLoader.java#L637-L648)). This is wider than the ticket, and the changelog does not say so. ## Proof **The two mixed-order tests fail on the base code and pass at this head.** - `AtomicUpdatesTest`: 32 tests, 0 failures, 1 pre-existing skip. Verified 2026-10-09 at `bcae04d77bdb`. - Run against the merge-base `97d973814336`, the same test class has 6 failures in 32 tests. Both mixed-order tests are among them: [`testMixedFieldPlainValueFirstIsRejected`](https://github.com/nick-boss-tech/solr/blob/bcae04d77bdbb33c33e077c3945a5deca0ce7dc1/solr/core/src/test/org/apache/solr/update/processor/AtomicUpdatesTest.java#L1343-L1369) (plain value first) and [`testMixedFieldOperationMapFirstIsRejected`](https://github.com/nick-boss-tech/solr/blob/bcae04d77bdbb33c33e077c3945a5deca0ce7dc1/solr/core/src/test/org/apache/solr/update/processor/AtomicUpdatesTest.java#L1372-L1396) (operation map first). ## Limits **JSON arrays, the XML writer and in-place updates are not covered by tests.** - JSON arrays. No test covers the array behavior described above. - Order. Operations apply in the order the `addField` calls ran. The apply-order tests check one order only, set and then add. - XML writer. The test comments describe how the XML writer shapes repeated fields. That path was not traced. - In-place updates. The in-place check still reads the whole field value ([line 295](https://github.com/nick-boss-tech/solr/blob/bcae04d77bdbb33c33e077c3945a5deca0ce7dc1/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java#L295)), and it returns when the value is not a map ([line 309](https://github.com/nick-boss-tech/solr/blob/bcae04d77bdbb33c33e077c3945a5deca0ce7dc1/solr/core/src/java/org/apache/solr/update/processor/AtomicUpdateDocumentMerger.java#L309)). A repeated-operation field takes the full atomic path. - Sequencing. [SOLR-12703](https://issues.apache.org/jira/browse/SOLR-12703) also changes `mergeDocHavingSameId`. The two merge cleanly, but their combined behavior is not tested here. Apply them in sequence. Changelog: `changelog/unreleased/SOLR-6045-atomic-update-repeated-field.yml` ### AI assistance AI agents assisted with research, implementation, review, and drafting. Nick Shanin directed the work and takes responsibility for this contribution. -- 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]
