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

Add a function to get the size of a collection/table in bytes. #272

Merged
merged 39 commits into from
Aug 10, 2023

Conversation

faisalill
Copy link
Contributor

What's Changed:

Added getCollectionSize method to MariaDB, MySQL, SQLite and MongoDB adapters.
The function for Postgres is working but it is giving an error during test so I just commented the code out.

Tests:

I have checked manually using the getCollectionSize method it worked.
I did add a test to check whether it returns a string.

@faisalill
Copy link
Contributor Author

@abnegate

@abnegate abnegate self-requested a review May 23, 2023 01:08
Copy link
Member

@abnegate abnegate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great start 😁

src/Database/Adapter.php Outdated Show resolved Hide resolved
src/Database/Adapter/MariaDB.php Outdated Show resolved Hide resolved
src/Database/Adapter/MariaDB.php Outdated Show resolved Hide resolved
src/Database/Adapter/MariaDB.php Outdated Show resolved Hide resolved
src/Database/Adapter/Postgres.php Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@faisalill
Copy link
Contributor Author

faisalill commented May 23, 2023

@abnegate ,

1.Fixed the stuff u mentioned.

2.As for the testing part I don't know why but it keeps returning me 0 (checked both before and after adding data) . That's the reason I used assertIsInt. Postgres is giving me the collection size in bytes but during test it is giving me an error SQLite too but it's related to the function returning null/0 as I said.
getSIzeOfCollectionErrors

  1. Regarding the Documentation, I guess only Document and Relationship methods are pending.

  2. Can I get permission to add try catch blocks which throws Database Exception as there are a lot of methods in the adapters that are missing it.

Thank you for your time.

@abnegate
Copy link
Member

@faisalill

  1. Fixed the stuff u mentioned.

Awesome, nice work!

  1. For the testing part I don't know why but it keeps returning me 0 (checked both before and after adding data) . That's the reason I used assertIsInt. Postgres is giving me the collection size in bytes but during test it is giving me an error SQLite too but it's related to the function returning null/0 as I said.

Please try to dig into this further, we'll need to get all adapters working. How are you running the tests? The Postgres issue is strange for the test suite. For the SQLite issue, try some extra debugging, adding var_dump or other logging to see what the function is doing at each step, that should help figure out why you're getting null.

  1. Regarding the Documentation, I guess only Document and Relationship methods are pending.

Awesome, let's add those methods too

  1. Can I get permission to add try catch blocks which throws Database Exception as there are a lot of methods in the adapters that are missing it.

Let's address that seperately, it's outside the scope of this PR. Make sure you've merge in the upstream main branch too, a lot of the Exception's have been updated to DatabaseException. DM me if you need help with it

@faisalill
Copy link
Contributor Author

faisalill commented May 25, 2023

@abnegate,

  1. Completed the documentation will create a new PR for it.
  2. Got the tests working.
  3. Changed the tests. (I check whether the size before and after data is greater)
    Thank you

src/Database/Adapter/Postgres.php Outdated Show resolved Hide resolved
src/Database/Adapter/MariaDB.php Outdated Show resolved Hide resolved
src/Database/Adapter/MariaDB.php Outdated Show resolved Hide resolved
src/Database/Adapter/MariaDB.php Outdated Show resolved Hide resolved
src/Database/Adapter/Mongo.php Outdated Show resolved Hide resolved
tests/Database/Base.php Outdated Show resolved Hide resolved
tests/Database/Base.php Outdated Show resolved Hide resolved
tests/Database/Base.php Outdated Show resolved Hide resolved
@faisalill
Copy link
Contributor Author

@abnegate,

  1. Formatted using composer format for indentation.
  2. Changed the amount of attributes created for testing.
  3. Regarding test for mysql : I tried to add different attributes and almost 10000 documents but still mysql keeps returning me 65536 as the size. I checked the size using sql terminal and it still returns me 65536. Adding so many documents is just increasing the time of tests. Rest all the adapters are working fine their size changes but mysql.

src/Database/Adapter/MariaDB.php Outdated Show resolved Hide resolved
src/Database/Adapter/MariaDB.php Outdated Show resolved Hide resolved
src/Database/Adapter/MariaDB.php Outdated Show resolved Hide resolved
src/Database/Adapter/MariaDB.php Outdated Show resolved Hide resolved
src/Database/Adapter.php Outdated Show resolved Hide resolved
Comment on lines 173 to 174
SELECT SUM(\"pgsize\") FROM \"dbstat\" WHERE name='{$namespace}_{$name}';
");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's fix the indentation here

src/Database/Adapter/Mongo.php Outdated Show resolved Hide resolved
src/Database/Adapter/Postgres.php Outdated Show resolved Hide resolved
src/Database/Adapter/SQLite.php Outdated Show resolved Hide resolved
tests/Database/Base.php Outdated Show resolved Hide resolved
@faisalill
Copy link
Contributor Author

@abnegate,
1, Fixed the indentation, and other stuff you mentioned.
2. I tried to add different documents in huge quantity (20000 docs), tried the sql terminal to insert and check the size and still it is returning 65536.

@faisalill
Copy link
Contributor Author

@abnegate ,
Fixed the indentation as you mentioned.

@faisalill
Copy link
Contributor Author

@abnegate ,
The tests are working fine now.
Can you re-review please

@abnegate abnegate requested a review from fogelito June 8, 2023 11:14
tests/Database/Base.php Outdated Show resolved Hide resolved
@faisalill faisalill requested a review from abnegate July 19, 2023 02:30
src/Database/Adapter/MariaDB.php Outdated Show resolved Hide resolved
src/Database/Adapter/Mongo.php Outdated Show resolved Hide resolved
src/Database/Adapter/MySQL.php Outdated Show resolved Hide resolved
@faisalill faisalill requested a review from abnegate July 26, 2023 02:22
src/Database/Adapter/Mongo.php Outdated Show resolved Hide resolved
src/Database/Adapter/MySQL.php Outdated Show resolved Hide resolved
src/Database/Adapter/MySQL.php Outdated Show resolved Hide resolved
src/Database/Adapter/MySQL.php Outdated Show resolved Hide resolved
src/Database/Adapter/MariaDB.php Outdated Show resolved Hide resolved
src/Database/Adapter/MariaDB.php Outdated Show resolved Hide resolved
@faisalill faisalill requested a review from abnegate July 27, 2023 01:21
@faisalill
Copy link
Contributor Author

@abnegate Made the changes

@fogelito
Copy link
Contributor

I think we missed the full-text index calculations table size? since there are additional tables.
cc @abnegate

@fogelito
Copy link
Contributor

looks like the first FT will create 6 table:
6 table:
appwrite/fts_00000000000004a9_00000000000001ca_index_1
appwrite/fts_00000000000004a9_00000000000001ca_index_2
appwrite/fts_00000000000004a9_00000000000001ca_index_3
appwrite/fts_00000000000004a9_00000000000001ca_index_4
appwrite/fts_00000000000004a9_00000000000001ca_index_5
appwrite/fts_00000000000004a9_00000000000001ca_index_6

with 5 additional table:

appwrite/fts_00000000000004a9_deleted_cache
appwrite/fts_00000000000004a9_deleted
appwrite/fts_00000000000004a9_config
appwrite/fts_00000000000004a9_being_deleted_cache
appwrite/fts_00000000000004a9_being_deleted

The second fulltext index for the same table will create only 6 tables

@abnegate
Copy link
Member

abnegate commented Aug 2, 2023

@fogelito Any ideas how we can count those too?

@abnegate
Copy link
Member

abnegate commented Aug 7, 2023

@faisalill Please add another test that:

  1. Creates a collection
  2. Creates some documents
  3. Gets the size
  4. Adds a fulltext index to the collection
  5. Checks that the size is larger than before

@faisalill
Copy link
Contributor Author

faisalill commented Aug 8, 2023

@abnegate
Added test for full text indexing it worked for all adapters except for mongoDB.
MongoDB is returning the same size before and after creating an index.
Will try to work on it and update u on the same.

@faisalill
Copy link
Contributor Author

@abnegate @fogelito
Size increases when full text index is added.
Added tests for it too.

src/Database/Adapter/MariaDB.php Outdated Show resolved Hide resolved
src/Database/Adapter/MariaDB.php Outdated Show resolved Hide resolved
src/Database/Adapter/Mongo.php Outdated Show resolved Hide resolved
tests/Database/Base.php Outdated Show resolved Hide resolved
@abnegate abnegate merged commit 318ee12 into utopia-php:main Aug 10, 2023
3 checks passed
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

Successfully merging this pull request may close these issues.

4 participants