Datastrore.transaction.select & Datastore.transaction.filter not working properly in GCloud v0.14+

[jiminikiz]

I have tried the following code snippets in v-0.[14,15,16,17,18]

transaction.filter("email = ", email);

.filter works for v[14,15,16] but not v[17,18] with no changes to my code base. In [17,18] the full amount of rows from the table is returned.

transaction.select(["date","account"]);

.select just hasn't worked for any version [14+]; the full amount of columns is always returned. I have not tested .select with any version before [14].

[stephenplusplus]

Thanks for reporting the bug. We'll look into it asap.

[stephenplusplus]

I'm not able to reproduce either failure. We have a suite of tests that run against a real Datastore backend. Here's one that tests filter and another that tests select. Are we not catching a use case similar to yours?

Pinging @pcostell in case there is a deeper reason a filter or select query might not produce the expected results.

[pcostell]

There are requirements around filters / what can be selected, however the Datastore backend will fail if these requirements are not met.

[pcostell]

@jiminikiz could you provide a test case that puts some data, then shows this failure when queryingn that data? It would be useful since it seems @stephenplusplus is not able to reproduce.

[jiminikiz]

We wrote a function that we use to run queries, it gets exported in node within module, datastore. Here is a snippet of the module we wrote and have been using for months:

var DataSet = require('gcloud').datastore.dataset(<ourConfig>);

module.exports = {
    ...
    query: function ( query, callback ) {
        var parameter, filter = query.filter,
            transaction = DataSet.createQuery( query.namespace, query.path );

        if ( filter !== undefined ) {
            for ( parameter in filter ) {
                transaction.filter( parameter, filter[ parameter ] );
            }
        }
        if ( query.sort !== undefined  ) {
            transaction.order( query.sort );
        }
        if ( query.select !== undefined ) {
            transaction.select( query.select );
        }
        if ( query.limit !== undefined ) {
            transaction.limit( query.limit );
        }

        DataSet.runQuery( transaction, callback );
    }
    ...
}

We usually name the export Datastore. Here is an example running a query:

var Datastore = require('datastore');
...
Datastore.query({
    namespace: 'accounts',
    path: ['account'],
    filter: { "email =": email }
}, function ( error, accounts ) {
    // do stuff
});
...

With the above snippet, the value of accounts within the callback will be a single item array in GCloud [<16]. For [16+], accounts is an array of every account in the namespace > path.

...
Datastore.query({
    namespace: 'accounts',
    path: ['account'],
    filter: { "email =": email },
    select: ['name','email']
}, function ( error, accounts ) {
    // do stuff
});
...

The above query like this will return all of the columns no matter what version of GCloud I am using.

[jiminikiz]

Would you like to see the resulting payloads for each version?

[stephenplusplus]

I think I have spotted the problem. Any modification to a Query object (which is the object returned from createQuery() or what your snippet calls transaction) doesn't alter the state of the original query. Each change returns a new Query object.

Having each query be immutable was always our goal. Even though we did refactor the class and tests recently, we must not have noticed it wasn't working as intended before, and therefore didn't make any notes about breaking changes in the release.

The fix will be overwriting transaction with the result of the modification:

module.exports = {
    ...
    query: function ( query, callback ) {
        var parameter, filter = query.filter,
            transaction = DataSet.createQuery( query.namespace, query.path );

        if ( filter !== undefined ) {
            for ( parameter in filter ) {
                transaction = transaction.filter( parameter, filter[ parameter ] );
            }
        }
        if ( query.sort !== undefined  ) {
            transaction = transaction.order( query.sort );
        }
        if ( query.select !== undefined ) {
           transaction =  transaction.select( query.select );
        }
        if ( query.limit !== undefined ) {
            transaction = transaction.limit( query.limit );
        }

        DataSet.runQuery( transaction, callback );
    }
    ...
}

I'm really sorry that we missed this.

[callmehiphop]

Having each query be immutable was always our goal.

Do you think we should provide an option for creating mutable queries? It seems like @jiminikiz's case, it being immutable is pretty inconvenient, not to mention I don't know if that's a pattern we enforce throughout gcloud-node.

[stephenplusplus]

I think we should pick one or the other, and I would prefer mutable. It has historically been immutable... not sure I remember why exactly, but maybe having to do with when we call start while running a query, it alters the user's provided query. Definitely open to going mutable like any other object.

[callmehiphop]

Cool, I think I'd prefer it mutable as well. I'll try and put together a PR for that today or tomorrow.

[jiminikiz]

Thanks guys, the work around @stephenplusplus posted (overwriting transaction) works great for now. I'll be on the look out for when query objects do become mutable in a future version.

[stephenplusplus]

Thanks for coming by and reporting, I'm glad we figured this out. We'll have a release out today.

(Fixed in #808)