googleapis/python-ndb

NDB: Queries with != filter are broken in the last version.

Closed this issue · 19 comments

Environment:
OS: macOs Sonoma 14.2.1
Python 3.11
google-cloud-ndb==2.3.0

@sorced-jim Looks like queries with != filter were broken in #950 PR (Use server side != for queries).

The issue is that in model.py function
def ne(self, value):
"""FilterNode: Represents the != comparison."""
return self._comparison("!=", value)

parameter server_op=True was not passed to the query.FilterNode function threw the _comparison(self, op, value) function.

As a result queries with != filter are failing.

Steps to reproduce:

class TestModel(ndb.Model):
    int_prop = ndb.IntegerProperty()
    str_prop = ndb.StringProperty()

query = TestModel.query(TestModel.int_prop != 2)
results = query.fetch()

...

When running the test query.fetch() is failing with the error
google.api_core.exceptions.RetryError: Maximum number of 3 retries exceeded while calling <function make_call..rpc_call at 0x1032d99e0>, last exception: None

[datastore] Mar 07, 2024 10:25:43 AM io.grpc.internal.SerializingExecutor run
[datastore] SEVERE: Exception while executing runnable io.grpc.internal.ServerImpl$JumpToApplicationThreadServerStreamListener$1HalfClosed@33a9178e
[datastore] java.lang.IllegalArgumentException: Unable to perform filter using operator 9
[datastore] 	at com.google.appengine.api.datastore.FilterMatcher.addFilter(FilterMatcher.java:216)
[datastore] 	at com.google.appengine.api.datastore.BaseEntityComparator.makeFilterMatchers(BaseEntityComparator.java:117)
[datastore] 	at com.google.appengine.api.datastore.BaseEntityComparator.<init>(BaseEntityComparator.java:55)
[datastore] 	at com.google.appengine.api.datastore.EntityProtoComparators$EntityProtoComparator.<init>(EntityProtoComparators.java:52)
[datastore] 	at com.google.cloud.datastore.emulator.impl.LocalDatastoreFileStub.runQuery(LocalDatastoreFileStub.java:341)
[datastore] 	at com.google.cloud.datastore.emulator.impl.CloudDatastoreV1.runQuery(CloudDatastoreV1.java:627)
[datastore] 	at com.google.cloud.datastore.emulator.v1.DatastoreV1Emulator.runQuery(DatastoreV1Emulator.java:64)
[datastore] 	at com.google.cloud.datastore.emulator.v1.DatastoreEmulatorGrpcAdapter$1.runQuery(DatastoreEmulatorGrpcAdapter.java:87)
[datastore] 	at com.google.datastore.v1.DatastoreGrpc$MethodHandlers.invoke(DatastoreGrpc.java:695)
[datastore] 	at io.grpc.stub.ServerCalls$UnaryServerCallHandler$UnaryServerCallListener.onHalfClose(ServerCalls.java:182)
[datastore] 	at io.grpc.internal.ServerCallImpl$ServerStreamListenerImpl.halfClosed(ServerCallImpl.java:346)
[datastore] 	at io.grpc.internal.ServerImpl$JumpToApplicationThreadServerStreamListener$1HalfClosed.runInContext(ServerImpl.java:860)
[datastore] 	at io.grpc.internal.ContextRunnable.run(ContextRunnable.java:37)
[datastore] 	at io.grpc.internal.SerializingExecutor.run(SerializingExecutor.java:133)
[datastore] 	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
[datastore] 	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
[datastore] 	at java.base/java.lang.Thread.run(Thread.java:834)

Weird, there is a system test for this already:

@pytest.mark.usefixtures("client_context")
def test_filter_not_equal(ds_entity):
    for i in range(5):
        entity_id = test_utils.system.unique_resource_id()
        ds_entity(KIND, entity_id, foo=i)

    class SomeKind(ndb.Model):
        foo = ndb.IntegerProperty()

    eventually(SomeKind.query().fetch, length_equals(5))

    query = SomeKind.query(SomeKind.foo != 2)
    results = query.fetch()
    results = sorted(results, key=operator.attrgetter("foo"))
    assert [entity.foo for entity in results] == [0, 1, 3, 4]

What version of the emulator is being used? Try using the Datastore mode emulator gcloud emulators firestore start --database-mode=datastore-mode

@sorced-jim
Current versions:
Google Cloud SDK 467.0.0
cloud-datastore-emulator 2.3.1

We used:
gcloud beta emulators datastore start --no-store-on-disk --host-port=127.0.0.1:9090 --consistency=1.0 -q

Getting the same errors also with
gcloud emulators firestore start --database-mode=datastore-mode --host-port=127.0.0.1:9090 -q

What's the Firestore emulator you are using? I tested with cloud-firestore-emulator 1.19.2 and the not equal tests pass.

The current Cloud Firestore Emulator version is 1.19.2
Was installed as a part of Google Cloud SDK 467.0.0

What's the stack trace for the Firestore in Datasatore mode emulator? It should be different from the Cloud Datastore emulator.

Waiting for emulator to launch on port 9090...
Connection to 127.0.0.1 port 9090 [tcp/websm] succeeded!
Executing: /Users/alexr/google-cloud-sdk/platform/cloud-firestore-emulator/cloud_firestore_emulator start --host=127.0.0.1 --port=9090 --database-mode=datastore-mode

So there is no stack trace from the emulator with the != query?

That's what we have on the failed test

======================================================================
ERROR: test_query_filter_not_equal (wix_privatemedia_service_tests.ndb.test_ndb_query.TestNDBQuery.test_query_filter_not_equal)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/alexr/dev/wix/wix-privatemedia-service/tests/wix_privatemedia_service_tests/utils/ndb.py", line 12, in wrapper
    return func(self, *args, **kw)
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/alexr/dev/wix/wix-privatemedia-service/tests/wix_privatemedia_service_tests/ndb/test_ndb_query.py", line 123, in test_query_filter_not_equal
    equal_results = query.fetch()
                    ^^^^^^^^^^^^^
  File "/Users/alexr/dev/wix/wix-privatemedia-service/run/lib/python3.11/site-packages/google/cloud/ndb/query.py", line 1202, in wrapper
    return wrapped(self, *dummy_args, _options=query_options)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/alexr/dev/wix/wix-privatemedia-service/run/lib/python3.11/site-packages/google/cloud/ndb/utils.py", line 118, in wrapper
    return wrapped(*args, **new_kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/alexr/dev/wix/wix-privatemedia-service/run/lib/python3.11/site-packages/google/cloud/ndb/utils.py", line 150, in positional_wrapper
    return wrapped(*args, **kwds)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/alexr/dev/wix/wix-privatemedia-service/run/lib/python3.11/site-packages/google/cloud/ndb/query.py", line 1744, in fetch
    return self.fetch_async(_options=kwargs["_options"]).result()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/alexr/dev/wix/wix-privatemedia-service/run/lib/python3.11/site-packages/google/cloud/ndb/tasklets.py", line 210, in result
    self.check_success()
  File "/Users/alexr/dev/wix/wix-privatemedia-service/run/lib/python3.11/site-packages/google/cloud/ndb/tasklets.py", line 157, in check_success
    raise self._exception
  File "/Users/alexr/dev/wix/wix-privatemedia-service/run/lib/python3.11/site-packages/google/cloud/ndb/tasklets.py", line 319, in _advance_tasklet
    yielded = self.generator.throw(type(error), error, traceback)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/alexr/dev/wix/wix-privatemedia-service/run/lib/python3.11/site-packages/google/cloud/ndb/_datastore_query.py", line 116, in fetch
    while (yield results.has_next_async()):
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/alexr/dev/wix/wix-privatemedia-service/run/lib/python3.11/site-packages/google/cloud/ndb/tasklets.py", line 319, in _advance_tasklet
    yielded = self.generator.throw(type(error), error, traceback)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/alexr/dev/wix/wix-privatemedia-service/run/lib/python3.11/site-packages/google/cloud/ndb/_datastore_query.py", line 343, in has_next_async
    yield self._next_batch()  # First time
    ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/alexr/dev/wix/wix-privatemedia-service/run/lib/python3.11/site-packages/google/cloud/ndb/tasklets.py", line 319, in _advance_tasklet
    yielded = self.generator.throw(type(error), error, traceback)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/alexr/dev/wix/wix-privatemedia-service/run/lib/python3.11/site-packages/google/cloud/ndb/_datastore_query.py", line 373, in _next_batch
    response = yield _datastore_run_query(query)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/alexr/dev/wix/wix-privatemedia-service/run/lib/python3.11/site-packages/google/cloud/ndb/tasklets.py", line 319, in _advance_tasklet
    yielded = self.generator.throw(type(error), error, traceback)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/alexr/dev/wix/wix-privatemedia-service/run/lib/python3.11/site-packages/google/cloud/ndb/_datastore_query.py", line 1030, in _datastore_run_query
    response = yield _datastore_api.make_call(
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/alexr/dev/wix/wix-privatemedia-service/run/lib/python3.11/site-packages/google/cloud/ndb/tasklets.py", line 323, in _advance_tasklet
    yielded = self.generator.send(send_value)
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/alexr/dev/wix/wix-privatemedia-service/run/lib/python3.11/site-packages/google/cloud/ndb/_retry.py", line 107, in retry_wrapper
    raise core_exceptions.RetryError(
google.api_core.exceptions.RetryError: Maximum number of 3 retries exceeded while calling <function make_call.<locals>.rpc_call at 0x14235e7a0>, last exception: None 


I can reproduce this error with the Datastore emulator (gcloud beta emulators datastore start), but not with the Firestore emulator. Please recheck your environment variables aren't being override in your test set up.

I am having the same problem with the new release 2.3.0

So will it no longer be possible to use the datastore emulator locally if we use '!=' operators? Only the firestore emulator in datastore-mode will work?

The firestore emulator in datastore-mode doesn't currently support resetting the database.
firebase/firebase-tools#6902

@sorced-jim Is there a path to continue to support the datastore emulator?

Using the firestore emulator is not a replacement for the datastore emulator for us as well during testing. It also doesn't support any way to validate/test composite indexes, not even in firestore mode: firebase/firebase-tools#2027

I've run into this problem as well.
As have been mentioned, it's happened because the Datastore emulator does not support != queries which #950 introduced server-side.

Using the server-side != has actually also unintentionally changed the behaviour of the queries on repeated fields. Previously, x != y would become x < y OR x > y, which would de-duplicate the results since it uses an OR query (according to these legacy docs (see note above linked heading)). The server-side != doesn't perform de-duplication (which we were implicitly relying on).

For now, since we don't use != in many places, my solution is to replace the x != y with a ndb.OR(x < y, x > y), which restores the old behaviour.

The firestore emulator in datastore-mode doesn't currently support resetting the database. firebase/firebase-tools#6902

@tlatin why don't you use the --data-dir cli parameter to control the emulator data file path and just delete that file for resetting the database?

@tlatin why don't you use the --data-dir cli parameter to control the emulator data file path and just delete that file for resetting the database?

The feature was added to the firestore emulator in data-store mode per the linked issue in your comment (firebase/firebase-tools#6902).

In general I wouldn't want to put the filesystem on the critical path of the hundred of tests. Also the previous datastore emulator supported the feature, so it's reasonable to request parity when being asked to migrate to a new solution.

In general I wouldn't want to put the filesystem on the critical path of the hundred of tests. Also the previous datastore emulator supported the feature, so it's reasonable to request parity when being asked to migrate to a new solution.

You are right that the feature should be there. But I am just suggesting a work around. And --data-dir only gives you the control over the path of the database file but it must be using a location on the filesystem by default.

The bug has two work around and those are documented on https://github.com/googleapis/python-ndb/wiki/Upgrade-Notes .