MongoDB findAndModify() with aggregation

classic Classic list List threaded Threaded
6 messages Options
| Threaded
Open this post in threaded view
|

MongoDB findAndModify() with aggregation

Benjamin Thompson
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
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
| Threaded
Open this post in threaded view
|

Re: MongoDB findAndModify() with aggregation

Alan DeKok-2
On Mar 20, 2020, at 8:05 AM, Benjamin Thompson <[hidden email]> 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.


-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
| Threaded
Open this post in threaded view
|

Re: MongoDB findAndModify() with aggregation

Benjamin Thompson
>> 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
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
| Threaded
Open this post in threaded view
|

Re: MongoDB findAndModify() with aggregation

Geaaru
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).
>
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
| Threaded
Open this post in threaded view
|

Re: MongoDB findAndModify() with aggregation

Benjamin Thompson
> 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
-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html
| Threaded
Open this post in threaded view
|

Re: MongoDB findAndModify() with aggregation

Alan DeKok-2
In reply to this post by Geaaru
On Mar 21, 2020, at 6:23 AM, Geaaru <[hidden email]> 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.


-
List info/subscribe/unsubscribe? See http://www.freeradius.org/list/users.html