Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ancestor query with filter on children not returning anything #1237

Closed
susanlinsfu opened this issue Apr 15, 2016 · 24 comments
Closed

Ancestor query with filter on children not returning anything #1237

susanlinsfu opened this issue Apr 15, 2016 · 24 comments
Assignees
Labels
api: datastore Issues related to the Datastore API.

Comments

@susanlinsfu
Copy link

susanlinsfu commented Apr 15, 2016

I am using gcloud-node v.0.30.3 and since it doesn't support gRPC I am using gcd-rpc for my local datastore.

Say I run this query to get all the children:

        var query = datastore.createQuery('Contacts').hasAncestor(datastore.key(['ContactsParent', 'my_username']));
            datastore.runQuery(query, function (err, entities) {
            if (err) {
                console.log('error ' + err);
                return;
            }
            console.log(JSON.stringify(entities));
                return;
        });

This works and returns all the entities. If I go:

        var query = datastore.createQuery('Contacts').hasAncestor(datastore.key(['ContactsParent', 'my_username'])).filter('status', 'some status');
            datastore.runQuery(query, function (err, entities) {
            if (err) {
                console.log('error ' + err);
                return;
            }
            console.log(JSON.stringify(entities));
                return;
        });

Nothing is return even though it should return something. Nothing is printed in the node console.

I have noticed some strange behavior. If I add a limit clause to this query it will return an empty array ( [] is printed in the console), but without it nothing.

My index.yaml file looks like this:

indexes: - kind: Contacts ancestor: yes properties: - name: status

I am expecting with this query that all the children get returned, and then from those children only the ones with a status of 'some status' return. This is not happening.

The records do exist in the datastore, but when I filter they do not. Example of a record:

datastore.save(
            {
              key: datastore.key(['ContactsParent','my_username', 'Contacts','my_contact']),
              method: 'insert',
              data: {
                status: 'some status'
              }
            }
}
@stephenplusplus
Copy link
Contributor

cc @pcostell

@pcostell
Copy link
Contributor

@stephenplusplus Could you verify what proto gets sent in this case?

@pcostell
Copy link
Contributor

Note I tested this manually using direct gRPC to the emulator and it seems to work fine, so I think it may be limited to gcloud-node.

@stephenplusplus
Copy link
Contributor

stephenplusplus commented Apr 15, 2016

I'm trying this now and get stuck in an infinite loop. The response from the query always has batch.moreResults as MORE_RESULTS_AFTER_LIMIT, so our code creates a nextQuery, and automatically runs it. The endCursor value is always the same as well, so it never gets anywhere.

@stephenplusplus stephenplusplus added the api: datastore Issues related to the Datastore API. label Apr 15, 2016
@pcostell
Copy link
Contributor

Didn't we address this in 30.3?

@stephenplusplus
Copy link
Contributor

We handle NOT_FINISHED by running the query again automatically, regardless of "autoPaginate" being true or false. And we added some perpetual limit logic to make sure we only ask for the right amount of results on those "nextQuery"s. But when we get the MORE_RESULTS_AFTER_LIMIT response, we still build and run the nextQuery.

Here's where our response logic starts-- anything look incorrect?

@pcostell
Copy link
Contributor

Doesn't the logic say to create the nextQuery if more_results is NOT_FINISHED or MORE_RESULTS_AFTER_LIMIT, but only run it if it is NOT_FINISHED?

@stephenplusplus
Copy link
Contributor

stephenplusplus commented Apr 15, 2016

It looks that way because of the code in that function, but when runQuery is called, it's actually wrapped by another function which automatically runs any nextQuery. This wrapper function is used throughout our API with any method that returns a "nextQuery" (storage.bucket.getFiles(), pubsub.getTopics(), etc). So we had to add in the manual override to continue running if it's "NOT_FINISHED", because the wrapper function wouldn't have continued running the query if the user had "autoPaginate" set to false.

Is it possible the emulator should have returned another value in place of "MORE_RESULTS_AFTER_LIMIT"? The code is only running over and over again because the response is telling us there are more results, but they're not coming.

@pcostell
Copy link
Contributor

Yes this is definitely the case: the emulator always returns
MORE_RESULTS_AFTER_LIMIT or NOT_FINISHED, but never NO_MORE_RESULTS.

I am working on making the emulator match production Datastore now.
However, I don't think the client should ever be doing this automatically.
This is the point of having NOT_FINISHED. MORE_RESULTS_AFTER_LIMIT should
only be used as a signal to the user of the client library that there might
be another page. For resource reasons the API explicitly only guarantees
that the value of this field is more conservative from a correctness point
of view (it won't say that is is finished if it actually isn't -- except of
course for eventual consistency for eventually consistent queries).

On Fri, Apr 15, 2016, 3:31 PM Stephen Sawchuk [email protected]
wrote:

It looks that way because of the code in that function, but when runQuery
is called, it's actually wrapped by another function which automatically
runs any nextQuery automatically. This wrapper function is used
throughout our API with any method that returns a "nextQuery"
(storage.bucket.getFiles(), pubsub.getTopics(), etc). So we had to add in
the manual override to continue running if it's "NOT_FINISHED", because the
wrapper function wouldn't have continued running the query if the user had
"autoPaginate" set to false.

Is it possible the emulator should have returned another value in place of
"MORE_RESULTS_AFTER_LIMIT"? The code is only running over and over again
because the response is telling us there are more results, but they're not
coming.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1237 (comment)

@stephenplusplus
Copy link
Contributor

This is just the default behavior of our API. If there is a "nextQuery" to run, we run it automatically. All methods that act this way can be instructed not to. In the case of datastore.runQuery(), query.autoPaginate(false) must be set.

We've done it many ways in the past, but this is why it was defaulted.

Do we need to rethink that approach for running Datastore queries, or is allowing auto-pagination to be disabled sufficient?

// @jgeewax

@susanlinsfu
Copy link
Author

susanlinsfu commented Apr 15, 2016

Sorry, but from what I understand adding autoPaginate(false) to my query should solve it for now? I have just tried adding it to my query and it still returns an empty array.

@pcostell
Copy link
Contributor

pcostell commented Apr 16, 2016

@stephenplusplus that makes sense, I think in that case you should only return nextQuery if more_results = NOT_FINISHED (and not do the request inside runQuery, instead depend on the generic pagination).

On Fri, Apr 15, 2016, 4:37 PM susanlinsfu [email protected] wrote:

Sorry, but from what I understand adding autoPaginate(false) to my query
should solve it? I have just tried adding it to my query and it still
returns an empty array.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#1237 (comment)

@stephenplusplus
Copy link
Contributor

@susanlinsfu regarding the empty result set, I'm not sure why they aren't being returned. Here's a script that I've tried: https://gist.github.com/stephenplusplus/8302ecb5892072f80db17f717a84a898. The query seems to return the results. Does that same script not work for you, or am I maybe not properly recreating your scenario?

@susanlinsfu
Copy link
Author

@stephenplusplus ahh yes it works for me now, I had a spelling mistake in the word "some stuff". And yes you are right if autopaginate is disabled it works, but if it is not there (or if limit) it will not return any data. Thanks!

@stephenplusplus
Copy link
Contributor

Phew, I'm glad you found the fix! I'm not sure which will be first: the gcloud emulator supporting gRPC or a new release of the gcd.sh tool matching the production Datastore API. Until then, as long as you're using either one, autoPaginate(false) is a safe bet. Thanks for posting this issue!

@pcostell
Copy link
Contributor

@stephenplusplus Do we have an issue tracking improper pagination? Even once the emulator has better handling of more_results, gcloud-node still needs to stop paging on MORE_RESULTS_AFTER_LIMIT.

@stephenplusplus
Copy link
Contributor

I thought we should still do that when autoPaginate is true (which is default)?

@pcostell
Copy link
Contributor

I don't think so -- If a user actually wanted all the results, they shouldn't set a limit. As it is now, anyone who wants a specific number of results has to set both a limit and autoPaginate = false. Anyone who wants all the results can either not set a limit or set a limit and leave autoPaginate = true.

@stephenplusplus
Copy link
Contributor

stephenplusplus commented Apr 18, 2016

autoPaginate respects the requested limit and stops running. This is built in to the wrapper function.

Where limitVal is detected and normalized as maxResults: https://github.com/GoogleCloudPlatform/gcloud-node/blob/5199cadb557bcbd1c5d34a5debccf97ec982561d/lib/common/stream-router.js#L97

Where maxResults is respected: https://github.com/GoogleCloudPlatform/gcloud-node/blob/5199cadb557bcbd1c5d34a5debccf97ec982561d/lib/common/stream-router.js#L177

@pcostell
Copy link
Contributor

Ah ok, you can probably ignore me then.

@stephenplusplus
Copy link
Contributor

Sorry that the code made this more confusing than it needed to be.

@aloisDeLaComble
Copy link

autoPaginate(false) does not exist on datastore 0.6.0. Am I doing things wrong or was this method removed ?
If it has been removed, then how to fix the issue that still occurs when mixing hasAncestor with filter ?

@dazraf
Copy link

dazraf commented Jul 7, 2017

I just installed the latest release of gcloud beta tools - this problem is still an issue. Why is it closed?

@ziong
Copy link

ziong commented Aug 15, 2017

+1
problem not yet solved!
any update?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: datastore Issues related to the Datastore API.
Projects
None yet
Development

No branches or pull requests

6 participants