[GODRIVER-1565] Consolidate IndexModel spec generation helpers and add docs to require order-preserving types Created: 09/Apr/20 Updated: 28/Oct/23 Resolved: 08/May/20 |
|
| Status: | Closed |
| Project: | Go Driver |
| Component/s: | Core API |
| Affects Version/s: | 1.3.2 |
| Fix Version/s: | 1.4.0 |
| Type: | Improvement | Priority: | Major - P3 |
| Reporter: | Tamir Aviv | Assignee: | Divjot Arora (Inactive) |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Description |
|
EDIT: We've had a number of users try to use maps for the IndexModel.Keys value. This is incorrect because index key specs are ordered, but Go maps do not guarantee that items will be returned in insertion order when reading. We should improve the docs for the Keys field to mention that an order-preserving type is required, similar to the docs we have for Database.RunCommand. While we're here, we can also consolidate some of the logic in the index name generation code by first calling transformBsoncoreDocument to create the keys spec and passing that to getOrGenerateIndexName to loop over it. This will likely be a small perf improvement, but it'll still be incorrect to use an unordered type for Keys.
ORIGINAL DESCRIPTION: If you want to create a compound index you should fill the following struct:
Here the "keys" should be of type map. Let's assume we want to create the following compound index (without any options):
The code could create all of the following indexes: 1. index { "item": 1, "stock": -1 } with name item_1_stock_-1 2. index { "item": 1, "stock": -1 } with name stock_-1_item_1 3. index { "stock": -1, "item": 1 } with name item_1_stock_-1 4. index { "stock": -1, "item": 1 } with name stock_-1_item_1
Explanation: First of all, it's important to understand why do we have this bug. in Go when you want to go over all the keys in map you use 'range' clause in a for statement. The problem is that according to the spec: "The iteration order over maps is not specified and is not guaranteed to be the same from one iteration to the next." So if we insert a map to this IndexModel struct the order is not guaranteed ! and as we know it's important in the case of compound index.
Ok, but why do we have 4 options in the example above and not two?
The reason is that if you don't specify a name in the 'Index Option' it will generate one for you. The function that generates the name and the function that generates the keys are two different functions! they both go over the same map but do different things. And this is the two functions that I'm referring to:
func getOrGenerateIndexName(registry *bsoncodec.Registry, model IndexModel) (string, error) func transformBsoncoreDocument(registry *bsoncodec.Registry, val interface{}) (bsoncore.Document, error)
|
| Comments |
| Comment by Githook User [ 08/May/20 ] |
|
Author: {'name': 'Divjot Arora', 'email': 'divjot.arora@10gen.com', 'username': 'divjotarora'}Message: |
| Comment by Divjot Arora (Inactive) [ 06/May/20 ] |
| Comment by Divjot Arora (Inactive) [ 09/Apr/20 ] |
|
tamirav@checkpoint.com I've changed the ticket title and description to match the work involved. We do have an existing example for creating indices at https://pkg.go.dev/go.mongodb.org/mongo-driver/mongo?tab=doc#example-IndexView.CreateMany . Hopefully that's helpful for your use case. |
| Comment by Tamir Aviv [ 09/Apr/20 ] |
|
Hi divjot.arora, After reading about bson.D I agree, we should repurpose this ticket to improve the documentation of the 'Keys' field to contains only order type (and give the bson.D as an example) About the two functions, it seems that if you use bson.D the order will be the same for each iteration in both functions, so we will get the same request each time. but I agree that it will be nicer to pass the transformed keys to getOrGenerateIndexName to avoid double transformation (the function getOrGenerateIndexNamealso transformed the keys the same way transformBsoncoreDocument does)
Thanks for your fast reply! Tamir. |
| Comment by Divjot Arora (Inactive) [ 09/Apr/20 ] |
|
This is an issue we've seen other users run into as well. This isn't really a bug in the driver because, as you mention, maps in Go do not guarantee that keys will be returned in insertion order when the map is iterated. Even if there were not two functions that iterated the map, this would still be an issue because a you could specify the map {x: 1, y: 1} but the created index could be {y: 1, x: 1}, which is incorrect. The resolution here is to use an ordered type like bson.D. Given that others have run into this as well, I think we should repurpose this ticket as an improvement to do two things:
Thoughts? |