Skip to content

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

Closed

Description

@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].

Activity

  1. added
    type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.
    api: datastoreIssues related to the Datastore API.
    on Aug 17, 2015
  2. stephenplusplus commented on Aug 17, 2015

    @stephenplusplus
    Contributor

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

  3. stephenplusplus commented on Aug 17, 2015

    @stephenplusplus
    Contributor

    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.

  4. pcostell commented on Aug 17, 2015

    @pcostell
    Contributor

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

  5. pcostell commented on Aug 17, 2015

    @pcostell
    Contributor

    @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.

  6. jiminikiz commented on Aug 17, 2015

    @jiminikiz
    Author

    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.

  7. jiminikiz commented on Aug 17, 2015

    @jiminikiz
    Author

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

  8. stephenplusplus commented on Aug 17, 2015

    @stephenplusplus
    Contributor

    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.

  9. callmehiphop commented on Aug 17, 2015

    @callmehiphop
    Contributor

    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.

  10. stephenplusplus commented on Aug 17, 2015

    @stephenplusplus
    Contributor

    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.

  11. callmehiphop commented on Aug 17, 2015

    @callmehiphop
    Contributor

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

  12. jiminikiz commented on Aug 18, 2015

    @jiminikiz
    Author

    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.

  13. stephenplusplus commented on Aug 18, 2015

    @stephenplusplus
    Contributor

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

    (Fixed in #808)

  14. added a commit that references this issue on Jan 17, 2023
  15. added a commit that references this issue on Feb 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

api: datastoreIssues related to the Datastore API.type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions