MongoDB findAndModify() with aggregation
Hi The findAndModify() method was recently updated to support aggregation. This means that the "update" parameter now accepts two different sytaxes: Old syntax (no aggregation): db.collection.findAndModify(query: { ... }, update: { ... } ) New syntax (with aggregation): db.collection.findAndModify(query: { ... }, update: [ ... ] ) The difference being that square bracket are used instead of curly bracktes for the aggregation pipeline. The first syntax works with FreeRADIUS but the second syntax gives the error: rlm_sql_mongo: 'update' does not hold a document. If there is any possiblitly to support the aggregation option then I think this would be very useful as it allows much more complex data processing inside the request. -- Ben Thompson
On Mar 20, 2020, at 8:05 AM, Benjamin Thompson <b.thompson@latera.ru> wrote:
The findAndModify() method was recently updated to support aggregation. This means that the "update" parameter now accepts two different sytaxes:
Old syntax (no aggregation): db.collection.findAndModify(query: { ... }, update: { ... } )
New syntax (with aggregation): db.collection.findAndModify(query: { ... }, update: [ ... ] )
The difference being that square bracket are used instead of curly bracktes for the aggregation pipeline.
The first syntax works with FreeRADIUS but the second syntax gives the error:
rlm_sql_mongo: 'update' does not hold a document.
It's not clear why... I'm not a Mongo expert. I just read the MongoC API, and poked things until it worked. The API is pretty good, which helped.
If there is any possiblitly to support the aggregation option then I think this would be very useful as it allows much more complex data processing inside the request.
Sure. What API does the module have to use? Alan DeKok.
If there is any possiblitly to support the aggregation option then I think this would be very useful as it allows much more complex data processing inside the request.
Sure. What API does the module have to use?
Hi Alan I think the API is the right one. The problem seems to be that the validity check on the "update" parameter is expecting only a document (which starts and ends with a curly bracket), when actually it should accept either a document or array (which starts and ends with square brackets). The code which does the checking is on line 414 of rlm_sql_mongo.c:
SNIP>>>> if (bson_iter_init_find(&iter, bson, "update")) { if (!BSON_ITER_HOLDS_DOCUMENT(&iter)) { DEBUG("rlm_sql_mongo: 'update' does not hold a document."); goto error; }
bson_iter_document(&iter, &document_len, &document); bson_update = bson_new_from_data(document, document_len); if (!bson_update) { DEBUG("rlm_sql_mongo: Failed parsing 'update'"); goto error; } update = true; }
>>>>>>
As a quick test I changed the code as follows:
SNIP>>>> if (bson_iter_init_find(&iter, bson, "update")) { if (!(BSON_ITER_HOLDS_DOCUMENT(&iter) || BSON_ITER_HOLDS_ARRAY(&iter))) { DEBUG("rlm_sql_mongo: 'update' does not hold a document or array."); goto error; }
if (BSON_ITER_HOLDS_DOCUMENT(&iter)) { bson_iter_document(&iter, &document_len, &document); bson_update = bson_new_from_data(document, document_len); } else { bson_iter_array(&iter, &document_len, &document); bson_update = bson_new_from_data(document, document_len); } if (!bson_update) { DEBUG("rlm_sql_mongo: Failed parsing 'update'"); goto error; } update = true; }
>>>>>>
Based the minimal testing which I have done this seems to work. However I am not an expert in C and I was not sure about where the paramter "document" comes from as this in theory might want to be renamed to someting else (as it can either be a document or an array). -- Ben Thompson
Hi guys, I think that the fix must be a little more complex. The change that you share is happening for >= MongoDB 4.2 so probably it is needed to add a different configuration based on the version of mongoc library. The only issue here is I dunno if it's correct base the fix through the mongoc library because here probably it's possible handle the update with both mongoc library 1.15.x and 1.16.x. The best solution probably is this: - try to identify the target MongoDB Server version on first connection and store this inside mongo db instance struct - to change the code in the way that if mongodb is >= 4.2 update is handled as an array instead of a single document. Obviously, this introduces complexity in the code. My cent. -- geaaru On Sat, 2020-03-21 at 13:01 +0300, Benjamin Thompson wrote:
If there is any possiblitly to support the aggregation option then I think this would be very useful as it allows much more complex data processing inside the request.
Sure. What API does the module have to use?
Hi Alan
I think the API is the right one. The problem seems to be that the validity check on the "update" parameter is expecting only a document (which starts and ends with a curly bracket), when actually it should accept either a document or array (which starts and ends with square brackets).
The code which does the checking is on line 414 of rlm_sql_mongo.c:
SNIP>>>> if (bson_iter_init_find(&iter, bson, "update")) { if (!BSON_ITER_HOLDS_DOCUMENT(&iter)) { DEBUG("rlm_sql_mongo: 'update' does not hold a document."); goto error; }
bson_iter_document(&iter, &document_len, &document); bson_update = bson_new_from_data(document, document_len);
if (!bson_update) { DEBUG("rlm_sql_mongo: Failed parsing 'update'"); goto error; }
update = true; }
> > > > > > >
As a quick test I changed the code as follows:
SNIP>>>> if (bson_iter_init_find(&iter, bson, "update")) { if (!(BSON_ITER_HOLDS_DOCUMENT(&iter) || BSON_ITER_HOLDS_ARRAY(&iter))) { DEBUG("rlm_sql_mongo: 'update' does not hold a document or array."); goto error; }
if (BSON_ITER_HOLDS_DOCUMENT(&iter)) { bson_iter_document(&iter, &document_len, &document); bson_update = bson_new_from_data(document, document_len); } else { bson_iter_array(&iter, &document_len, &document); bson_update = bson_new_from_data(document, document_len); }
if (!bson_update) { DEBUG("rlm_sql_mongo: Failed parsing 'update'"); goto error; }
update = true; }
> > > > > > >
Based the minimal testing which I have done this seems to work. However I am not an expert in C and I was not sure about where the paramter "document" comes from as this in theory might want to be renamed to someting else (as it can either be a document or an array).
The best solution probably is this: - try to identify the target MongoDB Server version on first connection and store this inside mongo db instance struct - to change the code in the way that if mongodb is >= 4.2 update is handled as an array instead of a single document.
Don't forget that 4.2 supports to old style too (so a document or array is valid). -- Ben Thompson
On Mar 21, 2020, at 6:23 AM, Geaaru <geaaru@gmail.com> wrote:
I think that the fix must be a little more complex.
Possibly. It's also possible to just document this limitation as "don't do that".
The best solution probably is this: - try to identify the target MongoDB Server version on first connection and store this inside mongo db instance struct - to change the code in the way that if mongodb is >= 4.2 update is handled as an array instead of a single document.
Obviously, this introduces complexity in the code.
Or just documentation is "don't do that" :) Alan DeKok.
participants (3)
-
Alan DeKok -
Benjamin Thompson -
Geaaru