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

long size varchar with long index size mysql #210

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from 11 commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ loader.php
/bin/view/results/
.vscode
.vscode/*
database.sql

## - Oh Wess!
Makefile
Expand Down
20 changes: 7 additions & 13 deletions composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion phpunit.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
convertNoticesToExceptions="true"
convertWarningsToExceptions="true"
processIsolation="false"
stopOnFailure="false">
stopOnFailure="true">
<testsuites>
<testsuite name="Application Test Suite">
<directory>./tests/</directory>
Expand Down
28 changes: 27 additions & 1 deletion src/Database/Adapter.php
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ abstract public function getKeywords(): array;

/**
* Filter Keys
*
*
* @throws Exception
* @return string
*/
Expand All @@ -490,4 +490,30 @@ public function filter(string $value): string

return $value;
}


/**
* Returns attribute from List of Document $attributes[]
* @param string $attributeKey
* @param array $attributes Document[]
* returns Document
* @return Document|null
* @throws Exception
*/
public function findAttributeInList(string $attributeKey, array $attributes): ?Document
Copy link
Member

Choose a reason for hiding this comment

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

This add unnecessary pollution to the API. This method should not be exposed to consumers of this library as it add unnecessary complexity, and noise.

We can move it to the MariaDB adapter as no other adapter uses it. This location can be reconsidered when we will need it in other places.

The method name doesn't seem to apply to any existing name pattern of the current API. I would vote to call it findAttribute and mark it protected.

{
$attributeKey = $this->filter($attributeKey);

/**
* @var Document $attribute
*/

foreach ($attributes as $attribute){
if($attributeKey === $this->filter($attribute->getId())){
return $attribute;
}
}

return null;
}
}
67 changes: 53 additions & 14 deletions src/Database/Adapter/MariaDB.php
Original file line number Diff line number Diff line change
Expand Up @@ -148,37 +148,45 @@ public function createCollection(string $name, array $attributes = [], array $in
$database = $this->getDefaultDatabase();
$namespace = $this->getNamespace();
$id = $this->filter($name);
$schemaAttributes = [];

foreach ($attributes as $key => $attribute) {
foreach ($attributes as $attribute) {
Copy link
Member

Choose a reason for hiding this comment

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

We have to explain this logic, not clear what is being achieved here. Please add a docblock.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

$attrId = $this->filter($attribute->getId());
$attrType = $this->getSQLType($attribute->getAttribute('type'), $attribute->getAttribute('size', 0), $attribute->getAttribute('signed', true));
$size = $attribute->getAttribute('size', 0);
$type = $attribute->getAttribute('type');
$signed = $attribute->getAttribute('signed', false);
$attrType = $this->getSQLType($type, $size, $signed);

if ($attribute->getAttribute('array')) {
$attrType = 'LONGTEXT';
}

$attributes[$key] = "`{$attrId}` {$attrType}, ";
$schemaAttributes[] = "`{$attrId}` {$attrType}, ";
}

$schemaIndexes = [];
foreach ($indexes as $key => $index) {
$indexId = $this->filter($index->getId());
$indexType = $index->getAttribute('type');
$indexAttributes = [];

$indexAttributes = $index->getAttribute('attributes');
foreach ($indexAttributes as $nested => $attribute) {
$indexLength = $index->getAttribute('lengths')[$key] ?? '';
$indexLength = (empty($indexLength)) ? '' : '(' . (int)$indexLength . ')';
$indexOrder = $index->getAttribute('orders')[$key] ?? '';
foreach ($index->getAttribute('attributes') as $attribute) {
$indexAttribute = $this->filter($attribute);
$attr = $this->findAttributeInList($indexAttribute, $attributes);

$size = $index['lengths'][$key] ?? 0;
$size = $this->getDefaultIndexSize($size, $attr);
$length = $size === 0 ? '' : '(' . $size . ')';

$indexOrder = $index->getAttribute('orders')[$key] ?? '';
if ($indexType === Database::INDEX_FULLTEXT) {
$indexOrder = '';
}

$indexAttributes[$nested] = "`{$indexAttribute}`{$indexLength} {$indexOrder}";
$indexAttributes[] = "`{$indexAttribute}`{$length} {$indexOrder}";
}

$indexes[$key] = "{$indexType} `{$indexId}` (" . \implode(", ", $indexAttributes) . " ),";
$schemaIndexes[] = "{$indexType} `{$indexId}` (" . \implode(", ", $indexAttributes) . " ),";
}

try {
Expand All @@ -189,9 +197,9 @@ public function createCollection(string $name, array $attributes = [], array $in
`_createdAt` datetime(3) DEFAULT NULL,
`_updatedAt` datetime(3) DEFAULT NULL,
`_permissions` MEDIUMTEXT DEFAULT NULL,
" . \implode(' ', $attributes) . "
" . \implode(' ', $schemaAttributes) . "
PRIMARY KEY (`_id`),
" . \implode(' ', $indexes) . "
" . \implode(' ', $schemaIndexes) . "
UNIQUE KEY `_uid` (`_uid`),
KEY `_created_at` (`_createdAt`),
KEY `_updated_at` (`_updatedAt`)
Expand Down Expand Up @@ -368,7 +376,6 @@ public function deleteAttribute(string $collection, string $id, bool $array = fa
* @param array $orders
* @return bool
* @throws Exception
* @throws PDOException
*/
public function createIndex(string $collection, string $id, string $type, array $attributes, array $lengths, array $orders): bool
{
Expand All @@ -383,6 +390,18 @@ public function createIndex(string $collection, string $id, string $type, array
}, $attributes);

foreach ($attributes as $key => $attribute) {

// $size = (int)($lengths[$key] ?? 0);
Copy link
Member

Choose a reason for hiding this comment

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

Don't we need this part?
Should we throw an exception when index size is not set properly?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the Index size, not the attribute size, it is not mandatory.
I commend out this part since I have questions. This is another place we have to fix regardless of the current bug.

// foreach ($collectionAttributes as $ca){
// if($attribute === $ca['key']){
// if($ca['type'] == Database::VAR_STRING){
// if($ca['size'] > 700){
// $size = $size === 0 || $size > 700 ? 700 : $size;
// }
// }
// }
// }

$length = $lengths[$key] ?? '';
$length = (empty($length)) ? '' : '(' . (int)$length . ')';
$order = $orders[$key] ?? '';
Expand Down Expand Up @@ -1790,7 +1809,6 @@ protected function getSQLIndex(string $collection, string $id, string $type, ar

default:
throw new Exception('Unknown Index Type:' . $type);
break;
}

return "CREATE {$type} `{$id}` ON {$this->getSQLTable($collection)} ( " . implode(', ', $attributes) . " )";
Expand Down Expand Up @@ -1891,4 +1909,25 @@ public static function getPDOAttributes(): array
PDO::ATTR_STRINGIFY_FETCHES => true // Returns all fetched data as Strings
];
}

/**
* Return the maximum index length per attribute type
* @param int $size
* @param Document $attribute
* @return int
*/
public function getDefaultIndexSize(int $size, Document $attribute): int
{
$maxIndexLength = 760;

if($attribute['type'] === Database::VAR_STRING){
if($attribute['size'] > $maxIndexLength){
$size = $size === 0 || $size > $maxIndexLength ? $maxIndexLength : $size;
}
}

return $size;
}


}
1 change: 0 additions & 1 deletion src/Database/Adapter/SQLite.php
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@ public function renameIndex(string $collection, string $old, string $new): bool
* @param array $orders
* @return bool
* @throws Exception
* @throws PDOException
*/
public function createIndex(string $collection, string $id, string $type, array $attributes, array $lengths, array $orders): bool
{
Expand Down
Loading