Feat 5376 improve the select query #287
Conversation
fixed query and attribute types
|
@fogelito can you review this PR please. |
Combine $createdAt and $updatedAt to in_array Use static internal parameteres array rather than $stringToRemove
| $projection['_createdAt'] = 1; | ||
| $projection['_updatedAt'] = 1; |
There was a problem hiding this comment.
We need to make sure these are not set by default the same as maraidb
There was a problem hiding this comment.
Except _collection and _id rest have to be set because.
- no _permissions returns a null document (https://github.com/utopia-php/database/blob/main/src/Database/Database.php#L2238)
- no _uid will not convert the data types of the document
- no _createdAt and _updatedAt are required here (https://github.com/utopia-php/database/blob/main/src/Database/Adapter/Mongo.php#L971)
There was a problem hiding this comment.
Can we make it so that they are not required at this point? I think we should be able to if we can do it for MariaDB
There was a problem hiding this comment.
For all adapters _uid and _permissions are a must.
MariaDB/MySQL/SQLite: https://github.com/faisalill/database/blob/feat-5376-improve-the-select-query/src/Database/Adapter/MariaDB.php#L1226
Postgres: https://github.com/faisalill/database/blob/feat-5376-improve-the-select-query/src/Database/Adapter/Postgres.php#L1211
Because of this:
- no _permissions returns a null document (https://github.com/utopia-php/database/blob/main/src/Database/Database.php#L2238)
- no _uid will not convert the data types of the document
There was a problem hiding this comment.
For Postgres I am querying all the internal attributes and if any internal attribute is in the query I am removing it from selection then before the document is returned I check which internal attributes were in the query and only keep those internal attributes in the document.
But for all the adapters internal attributes won't be returned. Unless queried specificly.
https://github.com/faisalill/database/blob/feat-5376-improve-the-select-query/tests/Database/Base.php#L1042
All these tests work fine.
It's a mess ik.
I will close this PR start from scratch and update u on the same.
src/Database/Database.php
Outdated
| foreach ($queries as $query) { | ||
| if ($query->getMethod() === Query::TYPE_SELECT) { | ||
| foreach ($results as $result) { | ||
| foreach (Database::INTERNAL_PARAMETERS as $parameter) { |
There was a problem hiding this comment.
This will remove internal attributes even if they were selected, we need to make sure they are only removed when not selected
Let's add a test to cover this case as well
There was a problem hiding this comment.
Tests here check whether only the queried internal attribute is returned.
| $this->assertArrayNotHasKey('$collection', $document); | ||
| $this->assertArrayNotHasKey('$createdAt', $document); | ||
| $this->assertArrayNotHasKey('$updatedAt', $document); | ||
| $this->assertArrayNotHasKey('$permissions', $document); | ||
| } | ||
| } |
There was a problem hiding this comment.
Let's add a test for the find method when selecting internal attributes
|
I will close this PR start from scratch and update u guys on the same. |
No description provided.