Graphback: Requiring _id field for mongo instead of `id`

[wtrocki]

Graphback was unable to create the specified indexes: E11000 duplicate key error collection: showcase.comment index: id_1 dup key: { : null }. If all else fails, try recreating the index manually.

Schema:

"""
@model
@datasync
"""
type Task {
  """@id"""
  id: ObjectID!
}

Can be replicated in datasync starter.

[wtrocki]

CC @ssd71

It is not possible to issue any query on this (when collection was created before it will receive actual index)

 "message": "E11000 duplicate key error collection: showcase.task index: id_1 dup key: { : null }",

[ssd71]

Looked around a bit, maybe it couldn't create index because there were multiple id: null documents in the collection?

I will have to take closer look if that's not it

[ssd71]

Either way, I think graphback should handle this error a bit better

[wtrocki]

That is what I have in db. Basically it fails after creating single document

[ssd71]

So I replicated this and I think I am onto something here:

• The error in insert occurs because the document that is inserted has no id field, effectively treated as id: null, As an existing document also does not have an id field (hence id: null) the insertion fails as @id creates a unique index
• I am sure you already know this, but the id field in the GraphQL SDL is being mapped to _id for everything from inserts to deletes, but the index is being created on the id field, so the thing is even if we have a custom primary key, in the current state of Graphback Mongo support, the name of custom primary key has to be other than id
• At the heart of the problem it is a limitation of MongoDB Data Provider, and the way we are handling custom primary keys for MongoDB, I am thinking of some ways to go around this, a quick and dirty way (haven't read up enough to recommend) is to map the custom primary key field to _id in the graphback data layer for mongo and stop creating indexes on @id as there is already an index on _id, This is entirely possible because _id can support quite literally any BSON type with the exception of arrays ¹, I need to look a bit into how this is handled in Postgres to ensure consistency though, I would love some feedback on this from @craicoverflow and @machi1990 as well, if possibe, internally(I think) this would function exactly like how id is mapped to _id in MongoDBDataProvider

I hope you don't mind me saying this but, the _id field always has an index in MongoDB, so I don't think we need to put @id on id in the schema as schema's id maps to _id in the database fields, unless you have another point to put forward in which case, I will retract this hehe

[craicoverflow]

At the heart of the problem it is a limitation of MongoDB Data Provider, and the way we are handling custom primary keys for MongoDB

To add to this, we are causing problems for ourselves by allowing any field to be assigned an ID and then mapping that to _id in the data provider. We have discussed doing this before but I think we should really require _id: ID/ObjectID instead of id: ID when using Mongo.

and stop creating indexes on @id as there is already an index on _id

Yes, I don't see a need for @id in Mongo either. How will it work when someone assigns email: String as ID - will this be converted to an ObjectID in the document and its original content lost?

Should @id annotations be ignored in Mongo? We could print an info/warning message to the console.

[ssd71]

At the heart of the problem it is a limitation
Yes, I don't see a need for @id in Mongo either. How will it work when someone assigns email: String as ID - will this be converted to an ObjectID in the document and its original content lost?

This is an actual option, we can indeed use strings in the _id field instead of ObjectID, please see here: https://docs.mongodb.com/manual/core/document/#the-id-field
_id can take any BSON type except arrays

But of course current implementation doesn't allow us to do that

[wtrocki]

To add to this, we are causing problems for ourselves by allowing any field to be assigned an ID and then mapping that to _id in the data provider. We have discussed doing this before but I think we should really require _id: ID/ObjectID instead of id: ID when using Mongo.

This!

[wtrocki]

Yes, I don't see a need for @id in Mongo either. How will it work when someone assigns email: String as ID - will this be converted to an ObjectID in the document and its original content lost?

Worth to mention that in Mongo _id is sacred and very very rarely you do use any other ID - you cannot simply set any primary key - _id is your primary key and you live with it. Obviously you can use indexes etc. to filter by field but primary key is always the same. Thus entire id feature for Mongo seems corner case. If we force _id which will actually simplify a lot then we can also go further and try to recommend id for Postgres. Obviously, we will allow some deviation from it but core use case will be there.

[wtrocki]

I will advise against using anything else than ObjectID. Those objects are crafted for replication so when having multiple replica sets using something different might require more work.

[machi1990]

Hi @ssd71 @wtrocki I feel like #1565 right?

[ssd71]

This issue has become more about requiring _id field for mongo instead of id but maybe let's create separate issue for this? wdyt @wtrocki

[wtrocki]

hmm.. I think lets keep thus issue. There is good discussion and we need this stuff

[machi1990]

This issue has become more about requiring _id field for mongo instead of id but maybe let's create separate issue for this? wdyt @wtrocki

hmm.. I think lets keep thus issue. There is good discussion and we need this stuff

I've renamed the issue to Requiring _id field for Mongo instead of id following the above two comments.

Is this related to https://github.com/aerogear/graphback/issues/1626?

[wtrocki]

Yes it is related

[machi1990]

This bug kind of relied on https://github.com/aerogear/graphback/pull/1749 which has now been merged.

I feel like we can revive this "id" issue for MongoDB so that we close other related issues too.

[craicoverflow]

Do you see this working for both ObjectID and GraphbackObjectID or should it be restricted to GraphbackObjectID?

I believe there is different behaviour between both of these now.

[machi1990]

Do you see this working for both ObjectID and GraphbackObjectID or should it be restricted to GraphbackObjectID?

I believe there is different behaviour between both of these now.

I see it being restricted to _id: GraphbackObjectID

[wtrocki]

Yes.. That will be the best and it is really going to help us to simplify implementation in the datastore.