[SERVER-76542] Geo 2d index update validation differs between 6.0 and 6.3 Created: 26/Apr/23  Updated: 09/May/23  Resolved: 09/May/23

Status: Closed
Project: Core Server
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major - P3
Reporter: Romans Kasperovics Assignee: Alberto Massari
Resolution: Duplicate Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: File bf-28454.js    
Issue Links:
Depends
Problem/Incident
is caused by SERVER-76875 Exclude fields containing dots from i... Closed
Assigned Teams:
Query Execution
Operating System: ALL
Sprint: QE 2023-05-15
Participants:
Linked BF Score: 120

 Description   

This is a bit complex issue, as it depends on a combination of 2 indexes and 2 updates.

I was able to reproduce the issue using the original fuzzer-generated test and then extracted and minimized the number of indexes and updates.

The attached script runs without an error on v6.3 and the current master (v7.0), but fails on v6.0 with "geo values must be 'legacy coordinate pairs' for 2d indexes".

The resmoke command to run the attached script is straightforward: python3 buildscripts/resmoke.py run bf-28454.js



 Comments   
Comment by Alberto Massari [ 09/May/23 ]

It is correct that the "is index affected" flag doesn't trigger the update of the 2D index, what is wrong is that trying to update that index ends up reading the "obj.geoLegacy" field, as it contains a dot in the name and should not be picked up. Fixing (and backporting) SERVER-76875 would fix this fuzzer error too

Comment by Alberto Massari [ 04/May/23 ]

Shorter repro case:

db.geo.drop()
db.geo.createIndex({"obj.inner.geoLegacy": "2d"})
db.geo.createIndex({"num": 1 })
db.geo.remove({_id: 17922})
db.geo.insert([ {_id: 17922, "obj": {_id: 17923, "str": "monetize Auto Loan Account"} } ])
db.geo.runCommand( {"update": "geo", "updates": [{"q": {}, "u": [{$set: {"num": 90, "obj": {$setField: {field: "inner.geoLegacy", input: "$obj", value: "$obj"}},}},  ], "upsert": false, "multi": false, }], "ordered": true, })

On 6.0 the last command fails because the presence of the index on "num" is enough to trigger the full indexing of the document, that will try to build a geo index key on the invalid value of obj.inner.geoLegacy. On 6.3 and 7.0, only the index on "num" is computed, so the error is not detected and the update operation succeeds. The reason why the index on "obj.inner.geoLegacy" is not marked as affected by the update operation is because "inner.geoLegacy" is a field in the "obj" subdocument, and not a "geoLegacy" field in an "inner" subdocument of the "obj" subdocument. If the index on "num" is removed, also 6.0 fails to update the geo index, and all version would detect the bad data only by running 

db.geo.validate({full: true})

Comment by Romans Kasperovics [ 03/May/23 ]

One more observation: the issued query differs from the logged query. I would guess the logged version is an equivalent rewrite without the $setField operator.

Comment by Romans Kasperovics [ 03/May/23 ]

Thanks alberto.massari@mongodb.com! Yes, I could reproduce the difference between 9ac3279 and your change 5977e70, so I guess we have located the change. Could you fix it or take over this ticket?

BTW, I failed to further minimize the example by removing one of the indexes, the first update statement, or some fields being updated. If one of the indexes is missing, or the first update is missing, or some of the update fields are missing, the update runs without hitting the uassert in question. I assume the fuzzer has discovered some weird and unexpected corner-case.

Comment by Kyle Suarez [ 02/May/23 ]

romans.kasperovics@mongodb.com could you please try the experiment as alberto.massari@mongodb.com mentioned?

Comment by Alberto Massari [ 02/May/23 ]

If my change (SERVER-65364) is involved, it would cause 6.3 and 7.0 to skip the index update because there is no change affecting the geoLegacy entry. The fix was not backported to 6.0, so if it thinks that at least one index is affected it would update all of them, including the geo one that would fail to parse the value that is created using an object that is not a geo value.

Can you run the repro case on a commit before 5977e70 (e.g. 9ac3279) and see if 5977e70 is the cause of the different behaviour?

Comment by Romans Kasperovics [ 02/May/23 ]

Thanks, jordi.olivares-provencio@mongodb.com! Additional info, the explain output for both versions looks identical, with an exception for v6.0 having an extra server parameter "internalQueryFrameworkControl": "tryBonsai", but I assume it does not matter in this case.

Generated at Thu Feb 08 06:32:57 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.