Skip to content

Feat 5376 improve the select query#287

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.

Sign up for GitHub

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

Closed
faisalill wants to merge 20 commits into from
Closed

Feat 5376 improve the select query #287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
0777736
fixed query and attribute types
faisalill Jun 28, 2023
1ca3476
Merge pull request #3 from faisalill/work-feat-5376
faisalill Jun 28, 2023
fedafbf
cleaned composer.json
faisalill Jun 28, 2023
0278adf
clean code
faisalill Jun 28, 2023
cde877c
changed few assertions for testFindSelect
faisalill Jun 28, 2023
b93544f
return by default
faisalill Jul 1, 2023
3a3b51f
permissions can be queried now
faisalill Jul 8, 2023
e39efe7
added assertions to test that other default keys are not returned
faisalill Jul 9, 2023
60bae57
fixed errors from testFindSelect
faisalill Jul 9, 2023
c4c6e2a
removed mytest from composer.json
faisalill Jul 9, 2023
f901305
added more assertions
faisalill Jul 9, 2023
e61eb13
fixed mongodb and postgres issue
faisalill Jul 12, 2023
0313f8c
fixed whitespace related issue
faisalill Jul 12, 2023
5fd6373
fixed lint issue
faisalill Jul 12, 2023
27364ea
Merge branch 'utopia-php:main' into feat-5376-improve-the-select-query
faisalill Jul 18, 2023
85afec2
Made changes as per review
faisalill Jul 20, 2023
4d20bc3
Merge branch 'utopia-php:main' into feat-5376-improve-the-select-query
faisalill Jul 22, 2023
a2d25c4
Fix code as per review
faisalill Jul 24, 2023
e480ad8
Make changes as per review
faisalill Jul 26, 2023
56dedb3
Merge branch 'utopia-php:main' into feat-5376-improve-the-select-query
faisalill Jul 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions composer.json
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -24,6 +24,10 @@
"Composer\\Config::disableProcessTimeout",
"docker compose exec tests vendor/bin/phpunit --configuration phpunit.xml"
],
"mytest": [
"Composer\\Config::disableProcessTimeout",
"docker compose exec tests vendor/bin/phpunit ./tests/Database/Adapter/MongoDBTest.php --stop-on-failure"
],
"lint": "./vendor/bin/pint --test",
"format": "./vendor/bin/pint",
"check": "./vendor/bin/phpstan analyse --level 7 src tests --memory-limit 512M",
Expand Down
59 changes: 45 additions & 14 deletions src/Database/Adapter/MariaDB.php
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1046,17 +1046,26 @@ public function find(string $collection, array $queries = [], ?int $limit = 25,
$results = $stmt->fetchAll();

foreach ($results as $key => $value) {
$results[$key]['$id'] = $value['_uid'];
$results[$key]['$internalId'] = $value['_id'];
$results[$key]['$createdAt'] = $value['_createdAt'];
$results[$key]['$updatedAt'] = $value['_updatedAt'];
$results[$key]['$permissions'] = json_decode($value['_permissions'] ?? '[]', true);

unset($results[$key]['_uid']);
unset($results[$key]['_id']);
unset($results[$key]['_createdAt']);
unset($results[$key]['_updatedAt']);
unset($results[$key]['_permissions']);
if (array_key_exists('_uid', $value)) {
$results[$key]['$id'] = $value['_uid'];
unset($results[$key]['_uid']);
}
if (array_key_exists('_id', $value)) {
$results[$key]['$internalId'] = $value['_id'];
unset($results[$key]['_id']);
}
if (array_key_exists('_createdAt', $value)) {
$results[$key]['$createdAt'] = $value['_createdAt'];
unset($results[$key]['_createdAt']);
}
if (array_key_exists('_updatedAt', $value)) {
$results[$key]['$updatedAt'] = $value['_updatedAt'];
unset($results[$key]['_updatedAt']);
}
if (array_key_exists('_permissions', $value)) {
$results[$key]['$permissions'] = json_decode($value['_permissions'] ?? '[]', true);
unset($results[$key]['_permissions']);
}

$results[$key] = new Document($results[$key]);
}
Expand DownExpand Up@@ -1189,10 +1198,32 @@ protected function getAttributeProjection(array $selections, string $prefix = ''
return '*';
}

if (in_array('$id', $selections)) {
$index = array_search('$id', $selections);
unset($selections[$index]);
}
if (in_array('$internalId', $selections)) {
$index = array_search('$internalId', $selections);
$selections[$index] = '_id';
}
if (in_array('$createdAt', $selections)) {
$index = array_search('$createdAt', $selections);
$selections[$index] = '_createdAt';
}
if (in_array('$updatedAt', $selections)) {
$index = array_search('$updatedAt', $selections);
$selections[$index] = '_updatedAt';
}
if (in_array('$permissions', $selections)) {
Comment thread
faisalill marked this conversation as resolved.
$index = array_search('$permissions', $selections);
unset($selections[$index]);
}
if (in_array('$collection', $selections)) {
$index = array_search('$collection', $selections);
unset($selections[$index]);
}

$selections[] = '_uid';
$selections[] = '_id';
$selections[] = '_createdAt';
$selections[] = '_updatedAt';
$selections[] = '_permissions';

if (!empty($prefix)) {
Expand Down
22 changes: 20 additions & 2 deletions src/Database/Adapter/Mongo.php
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1331,10 +1331,28 @@ protected function getAttributeProjection(array $selections, string $prefix = ''
}

$projection['_uid'] = 1;
$projection['_id'] = 1;
$projection['_permissions'] = 1;
$projection['_createdAt'] = 1;
$projection['_updatedAt'] = 1;
Comment on lines 1335 to 1336

@abnegateabnegateJul 25, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We need to make sure these are not set by default the same as maraidb

@faisalillfaisalillJul 26, 2023•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Except _collection and _id rest have to be set because.

@abnegateabnegateJul 26, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

@faisalillfaisalillJul 27, 2023•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

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:

@faisalillfaisalillJul 27, 2023•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

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.

$projection['_permissions'] = 1;

if (isset($projection['$id'])) {
unset($projection['$id']);
}
if (isset($projection['$internalId'])) {
unset($projection['$internalId']);
}
if (isset($projection['$permissions'])) {
unset($projection['$permissions']);
}
if (isset($projection['$createdAt'])) {
unset($projection['$createdAt']);
}
if (isset($projection['$updatedAt'])) {
unset($projection['$updatedAt']);
}
if (isset($projection['$collection'])) {
unset($projection['$collection']);
}

return $projection;
}
Expand Down
39 changes: 28 additions & 11 deletions src/Database/Adapter/Postgres.php
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1056,17 +1056,26 @@ public function find(string $collection, array $queries = [], ?int $limit = 25,
$results = $stmt->fetchAll();

foreach ($results as $key => $value) {
$results[$key]['$id'] = $value['_uid'];
$results[$key]['$internalId'] = $value['_id'];
$results[$key]['$createdAt'] = $value['_createdAt'];
$results[$key]['$updatedAt'] = $value['_updatedAt'];
$results[$key]['$permissions'] = json_decode($value['_permissions'] ?? '[]', true);

unset($results[$key]['_uid']);
unset($results[$key]['_id']);
unset($results[$key]['_createdAt']);
unset($results[$key]['_updatedAt']);
unset($results[$key]['_permissions']);
if (array_key_exists('_uid', $value)) {
$results[$key]['$id'] = $value['_uid'];
unset($results[$key]['_uid']);
}
if (array_key_exists('_id', $value)) {
$results[$key]['$internalId'] = $value['_id'];
unset($results[$key]['_id']);
}
if (array_key_exists('_createdAt', $value)) {
$results[$key]['$createdAt'] = $value['_createdAt'];
unset($results[$key]['_createdAt']);
}
if (array_key_exists('_updatedAt', $value)) {
$results[$key]['$updatedAt'] = $value['_updatedAt'];
unset($results[$key]['_updatedAt']);
}
if (array_key_exists('_permissions', $value)) {
$results[$key]['$permissions'] = json_decode($value['_permissions'] ?? '[]', true);
unset($results[$key]['_permissions']);
}

$results[$key] = new Document($results[$key]);
}
Expand DownExpand Up@@ -1215,6 +1224,14 @@ protected function getAttributeProjection(array $selections, string $prefix = ''
}
}

foreach ($selections as $key => $value) {
$valueUnquoted = \trim(\stripslashes($value), "\"");
$value = "$" . $valueUnquoted;
if (\in_array($value, Database::INTERNAL_ATTRIBUTES)) {
unset($selections[$key]);
}
}

return \implode(', ', $selections);
}

Expand Down
31 changes: 20 additions & 11 deletions src/Database/Adapter/SQL.php
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -127,17 +127,26 @@ public function getDocument(string $collection, string $id, array $queries = [])
return new Document([]);
}

$document['$id'] = $document['_uid'];
$document['$internalId'] = $document['_id'];
$document['$createdAt'] = $document['_createdAt'];
$document['$updatedAt'] = $document['_updatedAt'];
$document['$permissions'] = json_decode($document['_permissions'] ?? '[]', true);

unset($document['_id']);
unset($document['_uid']);
unset($document['_createdAt']);
unset($document['_updatedAt']);
unset($document['_permissions']);
if (isset($document['_uid'])) {
$document['$id'] = $document['_uid'];
unset($document['_uid']);
}
if (isset($document['_id'])) {
$document['$internalId'] = $document['_id'];
unset($document['_id']);
}
if (isset($document['_createdAt'])) {
$document['$createdAt'] = $document['_createdAt'];
unset($document['_createdAt']);
}
if (isset($document['_updatedAt'])) {
$document['$updatedAt'] = $document['_updatedAt'];
unset($document['_updatedAt']);
}
if (isset($document['_permissions'])) {
$document['$permissions'] = json_decode($document['_permissions'] ?? '[]', true);
unset($document['_permissions']);
}

return new Document($document);
}
Expand Down
56 changes: 54 additions & 2 deletions src/Database/Database.php
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -121,6 +121,16 @@ class Database
public const EVENT_INDEX_CREATE = 'index_create';
public const EVENT_INDEX_DELETE = 'index_delete';

// Internal Parameters
public const INTERNAL_ATTRIBUTES = [
'$id',
'$internalId',
'$createdAt',
'$updatedAt',
'$permissions',
'$collection'
];

protected Adapter $adapter;

protected Cache $cache;
Expand DownExpand Up@@ -2284,6 +2294,26 @@ public function getDocument(string $collection, string $id, array $queries = [])
$this->cache->save($cacheKey, $document->getArrayCopy());
}

foreach ($queries as $query) {
if ($query->getMethod() == Query::TYPE_SELECT) {
$queriedValues = $query->getValues();
$defaultKeys = Database::INTERNAL_ATTRIBUTES;

foreach ($queriedValues as $queriedValue) {
if (in_array($queriedValue, $defaultKeys)) {
$index = array_search($queriedValue, $defaultKeys);
unset($defaultKeys[$index]);
}
}

foreach ($defaultKeys as $defaultKey) {
if ($document->isSet($defaultKey)) {
$document->removeAttribute($defaultKey);
}
}
}
}

$this->trigger(self::EVENT_DOCUMENT_READ, $document);

return $document;
Expand DownExpand Up@@ -4015,6 +4045,17 @@ public function find(string $collection, array $queries = [], ?int $timeout = nu

$results = $this->applyNestedQueries($results, $nestedQueries, $relationships);

// remove default keys $id and $permissions from the results
foreach ($queries as $query) {
if ($query->getMethod() === Query::TYPE_SELECT) {
foreach ($results as $result) {
Comment thread
faisalill marked this conversation as resolved.
foreach (Database::INTERNAL_ATTRIBUTES as $parameter) {
$result->removeAttribute($parameter);
}
}
}
}

$this->trigger(self::EVENT_DOCUMENT_FIND, $results);

return $results;
Expand DownExpand Up@@ -4351,8 +4392,17 @@ public function decode(Document $collection, Document $document, array $selectio
}
}

if (empty($selections) || \in_array($key, $selections) || \in_array('*', $selections)) {
$document->setAttribute($key, ($array) ? $value : $value[0]);
if (
empty($selections)
|| \in_array($key, $selections)
|| \in_array('*', $selections)
|| \in_array($key, ['$createdAt', '$updatedAt'])
) {
if (\in_array($key, ['$createdAt', '$updatedAt']) && $value[0] === null) {
continue;
} else {
$document->setAttribute($key, ($array) ? $value : $value[0]);
}
}
}

Expand DownExpand Up@@ -4510,6 +4560,8 @@ private function validateSelections(Document $collection, array $queries): array
}
}

$keys = \array_merge($keys, Database::INTERNAL_ATTRIBUTES);

$invalid = \array_diff($selections, $keys);
if (!empty($invalid) && !\in_array('*', $invalid)) {
throw new DatabaseException('Cannot select attributes: ' . \implode(', ', $invalid));
Expand Down
Loading