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

count() is incorrect #4755

Closed
adam-lynch opened this issue Jun 2, 2023 · 20 comments
Closed

count() is incorrect #4755

adam-lynch opened this issue Jun 2, 2023 · 20 comments

Comments

@adam-lynch
Copy link
Contributor

adam-lynch commented Jun 2, 2023

In our web app, we do this:

const rxQuery = collection.count({
    selector: {
      id: {
        $eq: 'persistentStateBlob',
      },
    },
  });
const result = await.exec();

Expected: result is 0.
Actual: result is 1.

In fact, if I use the following code instead...

const rxQuery = collection.find({
    selector: {
      id: {
        $eq: 'persistentStateBlob',
      },
    },
  });
const result = (await.exec()).length;

... result is 0. (That's the exact same query)

Data

The collection does contain one item with an ID of persistentStateBlob2.

Schema

{
    primaryKey: 'id',
    properties: {
        id: { type: 'string', maxLength: 100 },
        value: { type: 'string' },
    },
    required: ['value'],
    type: 'object',
    version: 0,
}

Versions and storage

  • rxdb: 14.11.1
  • rxdb-premium: 14.11.1
  • storage: premium IndexedDB storage via shared storage worker

What I've tried

### With our app

  • Chromium and Firefox browsers.
  • Disabling eventReduce.
  • Disabling multiInstance.
  • Removing the key compression plugin (this collection doesn't use it anyway).
  • Disabling the shared storage worker wrapper.
  • Using the memory storage (without the shared storage worker).
  • Deleting the IndexedDB databases in browser devtools.

All behave the same.

RxDB tests

  • I created a test in test/unit/bug-report.test.ts and ran it with node+dexie; it passed.
  • I changed the count matching only test in test/unit/rx-collection.test.ts and ran it with node+dexie; it passed.

Other notes

  • count(...) (with a selector) correctly returns 0 if the collection is empty.
  • count() without any argument seems correct.
@pubkey
Copy link
Owner

pubkey commented Jun 7, 2023

We need a PR with a test for that. This would make it easy to check if this is a dexie.js problem or if other storages are also affected.

@adam-lynch
Copy link
Contributor Author

#4775

@adam-lynch
Copy link
Contributor Author

adam-lynch commented Jun 12, 2023

Oh... it has failed; https://github.com/pubkey/rxdb/actions/runs/5245370737/jobs/9472723197 (I'm guessing this job is the closest to my usage but I'm not 100% sure)

@stale
Copy link

stale bot commented Jun 26, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed soon. If you still have a problem, make a PR with a test case or to prove that you have tried to fix the problem.

@stale stale bot added the stale label Jun 26, 2023
@adam-lynch
Copy link
Contributor Author

keep open

@stale stale bot removed the stale label Jun 26, 2023
@stale
Copy link

stale bot commented Jul 10, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed soon. If you still have a problem, make a PR with a test case or to prove that you have tried to fix the problem. Notice that only bugs in the rxdb premium plugins are ensured to be fixed by the maintainer. Everything else is expected to be fixed by the community, likely you must fix it by yourself.

@stale stale bot added the stale label Jul 10, 2023
@adam-lynch
Copy link
Contributor Author

This is still an issue as far as I know. I can't today but I'll test again with the latest version when I get a chance

@stale stale bot removed the stale label Jul 10, 2023
@pubkey
Copy link
Owner

pubkey commented Jul 12, 2023

Hi @adam-lynch
I have reproduced and analyzed the problem.
I am working on a fix, but it will take some time and very likely it will require a major RxDB release.

@adam-lynch
Copy link
Contributor Author

Ok great

@pubkey
Copy link
Owner

pubkey commented Jul 18, 2023

@adam-lynch
Fix is released, please check the newest RxDB version.

@adam-lynch
Copy link
Contributor Author

I checked again using my tests and the example I described (using $eq) works correctly. However, $regex doesn't work as expected.

Example: I have 4 items total. 2 of which have IDs that start with hello2 (one is hello2, the other is hello22). The query is { id: { $regex: '^hello2' } }. The result is 4, not 2.

@pubkey
Copy link
Owner

pubkey commented Jul 19, 2023

@adam-lynch I tried to reproduce this but couldnt. https://github.com/pubkey/rxdb/pull/4829/files

@adam-lynch
Copy link
Contributor Author

I'll try to make a simpler test on my end. The test I used today uses rxQuery.$.subscribe

@adam-lynch
Copy link
Contributor Author

.count(...).exec() works fine (returns 2). .count().$.subscribe does not (returns 3). It works if I use the memory RxStorage rather than IndexedDB.

I tried a few other things too including disabling event reduce, disabling key compression, adding a cache replacement policy which always uncaches all, disabled validation, etc.

I don't know how I can debug this any further. I tried stepping through but I don't understand it all. I don't see anywhere that the regex I give is converted into a query/plan. It seems like it just sets the bounds and counts.

@pubkey
Copy link
Owner

pubkey commented Jul 25, 2023

@adam-lynch
Can you provide a test case for that?
My previous test uses .find(), not .count() because using count query on a plain regex is not allowed, it will throw with

Running a count() query in slow mode is now allowed. Either run a count() query with a selector that fully matches an index or set allowSlowCount=true when calling the createRxDatabase

Did you set allowSlowCount=true ?
Did you insert the documents with bulkInsert or with multiple insert calls?

It works if I use the memory RxStorage rather than IndexedDB.

This is interesting and helps a bit in debugging.

@adam-lynch
Copy link
Contributor Author

Can you provide a test case for that?

I'll try now.

Did you set allowSlowCount=true ?

Yes.

Did you insert the documents with bulkInsert or with multiple insert calls?

bulkUpsert

@adam-lynch
Copy link
Contributor Author

Test: #4843. Interestingly when I run this test locally, the .exec call fails and the .$.subscribe call works. The opposite of what I saw last in our Cypress tests.

pubkey added a commit that referenced this issue Jul 27, 2023
pubkey added a commit that referenced this issue Jul 27, 2023
* Slow count is broken #4755

* ADD changelog
@pubkey
Copy link
Owner

pubkey commented Jul 27, 2023

@adam-lynch Thank you, the test helped.
I could reproduce and fix this, please try out the latest RxDB version. This was broken in multiple storages.

@adam-lynch
Copy link
Contributor Author

Thanks, I'll test on Monday

@adam-lynch
Copy link
Contributor Author

Looks good, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants