parse-server icon indicating copy to clipboard operation
parse-server copied to clipboard

Count Objects not working in some classes

Open MrMartinR opened this issue 3 years ago • 25 comments
trafficstars

New Issue Checklist

Issue Description

Dashboard doesn't count object in most of the classes

Steps to reproduce

Since the first installation I always had this issue...

Actual Outcome

Only showing '2' in the _Session class, the other classes show '0'

Expected Outcome

To show the total of objects in each class

Environment

Parse dashboard hosted on AWS Ubuntu EC2

Dashboard

  • Parse Dashboard version: 4.1.1
  • Browser: Safari, Safari Technology Preview Release, Firefox, Chrome
  • Browser version: Safari 15.5, Safari Technology Preview Release 146, Firefox 100.0.2, Chrome 102.0.5005.61.

Server

  • Parse Server version: 5.2.1
  • Operating system: macOS Monterrey
  • Local or remote host (AWS, Azure, Google Cloud, Heroku, Digital Ocean, etc): AWS Ubuntu EC2

Database

  • System (MongoDB or Postgres): Postgres
  • Database version: 14.2
  • Local or remote host (MongoDB Atlas, mLab, AWS, Azure, Google Cloud, etc): AWS

Logs

Screen Shot 2022-06-04 at 19 15 17

MrMartinR avatar Jun 04 '22 18:06 MrMartinR

Thanks for opening this issue!

  • 🚀 You can help us to fix this issue faster by opening a pull request with a failing test. See our Contribution Guide for how to make a pull request, or read our New Contributor's Guide if this is your first time contributing.

Thanks for opening this issue!

  • 🚀 You can help us to fix this issue faster by opening a pull request with a failing test. See our Contribution Guide for how to make a pull request, or read our New Contributor's Guide if this is your first time contributing.

Probably related: https://github.com/parse-community/parse-dashboard/issues/1637

The issue above was reported to only happen on Postgres, not on MongoDB; same as this new issue. May be DB specific, and therefore possibly a Parse Server issue.

mtrezza avatar Jun 04 '22 19:06 mtrezza

it indeed is parse server issue as the count is coming from parse server only.

patelmilanun avatar Apr 05 '23 07:04 patelmilanun

i found the solution should i raise MR

it can be done using qs = `SELECT n_live_tup AS approximate_row_count FROM pg_stat_all_tables WHERE relname = $1; instead of qs = 'SELECT reltuples AS approximate_row_count FROM pg_class WHERE relname = $1'; when doing count inside parse-server\src\Adapters\Storage\Postgres\PostgresStorageAdapter.js

patelmilanun avatar Apr 05 '23 10:04 patelmilanun

Would this be a Parse Server or Parse Dashboard PR?

mtrezza avatar Apr 09 '23 21:04 mtrezza

Parse server it is

patelmilanun avatar Apr 10 '23 03:04 patelmilanun

Thanks for opening this issue!

  • 🚀 You can help us to fix this issue faster by opening a pull request with a failing test. See our Contribution Guide for how to make a pull request, or read our New Contributor's Guide if this is your first time contributing.

Moving the issue to Parse Server.

mtrezza avatar Apr 10 '23 23:04 mtrezza

🎉 This change has been released in version 6.1.0-alpha.15

parseplatformorg avatar May 28 '23 12:05 parseplatformorg

Reopening, the fix caused other issues, see failing CI: https://github.com/parse-community/parse-server/actions/runs/5105446713/jobs/9177489496?pr=8586. Fix has been reverted with https://github.com/parse-community/parse-server/pull/8588. @patelmilanun could you try again?

mtrezza avatar May 28 '23 18:05 mtrezza

Can you please help me on this. Based on what I see, I can conclude that my changes are not working for other versions of PostgreSQL like 11 to 14 with different PostGIS and only work with PostgreSQL version 15 with PostGIS 3.3.

So is that mean my code isn't backward compatible and I need a workaround for older versions.

patelmilanun avatar May 29 '23 06:05 patelmilanun

We have all the postres tests in the CI so you can see for which versions it's failing in a PR. You could start the PR with adding a tests that demonstrates that the count is incorrect, to see which versions need a fix. Then you could apply a fix depending on the version.

mtrezza avatar May 29 '23 10:05 mtrezza

@patelmilanun I made the request in https://github.com/parse-community/parse-server/pull/8586#issuecomment-1566205298 to revert your PR due to it not passing the test suite. I recommend reading https://github.com/parse-community/parse-server/pull/8586#issuecomment-1566221232 as it has more insight about why the count is only an estimate and is not intended to be exact. The comment also points to the broken test case if you are attempting to look at it in the future.

So is that mean my code isn't backward compatible and I need a workaround for older versions.

It's possible your changes are not backwards compatible. If your version of an estimate count is not available in older versions of Postgres, IMO, it may not be worth the workaround since both are only estimates. Of course, you can look into ways to detect the postgres version and use your code over the original way if you believe your estimate is always more accurate in newer versions of postgres.

cbaker6 avatar May 29 '23 13:05 cbaker6

Only showing '2' in the _Session class, the other classes show '0'

The original issue was that classes show a count of zero. If that is due to being an estimate we can change this from "bug" to "feature request". If there is any improvement possible (such as exact count if table doesn't have many rows, and only estimate if it has many rows), then we can keep this open, otherwise we can close as designed. In any case, a count of zero while there are 4 rows looks rather unexpected.

mtrezza avatar May 29 '23 13:05 mtrezza

@cbaker6 @mtrezza My changes are related to the count being wrong so how it's affecting the test case line expect(response.results.length).toEqual(2);. I mean we are counting length of result and not checking the count. I mean if it's failing at line expect(response.count).toEqual(0); than I get it as it's the actual line checking for count right?

image

Apart from this I followed this guide to test it https://github.com/parse-community/parse-server/blob/alpha/CONTRIBUTING.md#postgres-with-docker which starts a docker with PostgreSQL 13.8 and PostGIS 3.2.2 and it's not failing for me.

patelmilanun avatar May 29 '23 14:05 patelmilanun

The original issue was that classes show a count of zero. If that is due to being an estimate we can change this from "bug" to "feature request".

A feature request makes sense. The count method defaults to estimating most likely due to a count being expensive, so a good design decision IMO. The dashboard uses the estimate. When a direct query asks for count it uses the expensive count. https://github.com/parse-community/parse-server/blob/505dd6bcfe2ce787a85d380b60b0d4dc5656fea1/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js#L2020-L2044

In any case, a count of zero while there are 4 rows looks rather unexpected.

Agreed... I do believe an estimate is okay as most deployed apps will have multiple classes/tables with multiple rows. The dashboard shouldn't cause an unnecessary burden on the DB. I've seen some posts about count being expensive in MongoDB as well, but haven't looked into how the parse mongo adapter is handling this. IMO the display of the amount of rows is merely a convenience and shouldn't be depended on for an exact number. It should probably be noted somewhere in the docs so postgres users who also use the Parse Dashboard don't think it's a bug.

cbaker6 avatar May 29 '23 14:05 cbaker6

I mean if it's failing at line expect(response.count).toEqual(0); than I get it as it's the actual line checking for count right?

@patelmilanun there are multiple tests in the same function. You should verify which one is failing, my guess is it's line 203, not the one you pointed to, but I'm guessing. If you feel your adjustment improved the results, you still have to update the test cases and justify your change of the test (this would seem odd to me because you mentioned it passes for Postgres 15, but not earlier versions): https://github.com/parse-community/parse-server/blob/6c5f89a56bf21609818b8aac7c55137101f8c62f/spec/InstallationsRouter.spec.js#L194-L203

One thing to remember is the CI is there to test all supported versions as developers use many different versions in their active deployments. Just because it passes one (or locally on your system), doesn't mean it will pass all others in the CI.

cbaker6 avatar May 29 '23 14:05 cbaker6

@patelmilanun You may want to look online for possible solutions, row counting being a common issue, see for example https://stackoverflow.com/questions/7943233/fast-way-to-discover-the-row-count-of-a-table-in-postgresql.

Off the top of my head, a possible solution could be to get the estimate first, and if the estimate is a low number (like <1k), then do an exact count. That would require 2 DB requests, but if this is the only solution due to the design limitations of the DB then it could be an option in Parse Server. I could imagine that estimated vs. exact counting is something that is also an interesting feature for normal queries.

mtrezza avatar May 29 '23 15:05 mtrezza

But I had the same PostgreSQL as well as PostGIS version as the pipeline which is failing does have. But for me its running perfectly. I don't know how to reproduce it and as such I can't even propose a solution.

patelmilanun avatar May 30 '23 05:05 patelmilanun

Did you run the full tests (/spec) locally?

mtrezza avatar May 30 '23 06:05 mtrezza

No I didn't but after ur reply I did and i'm attaching the result of PARSE_SERVER_TEST_DB=postgres PARSE_SERVER_TEST_DATABASE_URI=postgres://postgres:password@localhost:5432/parse_server_postgres_adapter_test_database npm run coverage while there was a docker container running with same version as failed versions which was started using docker run -d --name parse-postgres -p 5432:5432 -e POSTGRES_PASSWORD=password --rm postgis/postgis:11-3.0-alpine && sleep 20 && docker exec -it parse-postgres psql -U postgres -c 'CREATE DATABASE parse_server_postgres_adapter_test_database;' && docker exec -it parse-postgres psql -U postgres -c 'CREATE EXTENSION pgcrypto; CREATE EXTENSION postgis;' -d parse_server_postgres_adapter_test_database && docker exec -it parse-postgres psql -U postgres -c 'CREATE EXTENSION postgis_topology;' -d parse_server_postgres_adapter_test_database.

U can see that there wasn't any case which are failing for me which are failing for CI with the same exact version.

test coverage log.txt

patelmilanun avatar May 30 '23 07:05 patelmilanun

🎉 This change has been released in version 6.3.0-beta.1

parseplatformorg avatar Jun 10 '23 23:06 parseplatformorg

🎉 This change has been released in version 6.3.0-alpha.1

parseplatformorg avatar Jun 18 '23 01:06 parseplatformorg

🎉 This change has been released in version 6.3.0

parseplatformorg avatar Sep 16 '23 01:09 parseplatformorg