Storage calculations - #930
Conversation
📝 WalkthroughWalkthroughThe MariaDB adapter now retrieves collection and permissions table sizes with one prepared ChangesMariaDB collection size
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThe PR optimizes MariaDB/MySQL collection-size calculation by combining two metadata queries into one round trip.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (2): Last reviewed commit: "Merge branch 'main' of github.com:utopia..." | Re-trigger Greptile |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Database/Adapter/MariaDB.php`:
- Around line 293-304: Update the SQL in the MariaDB adapter’s size query to use
distinct PDO placeholders for each table_schema predicate, then bind $database
to both markers in the adjacent bindParam calls. Keep the existing $collection
and $permissions bindings unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 96672097-f8c0-434a-a2bb-c707f6b26271
📒 Files selected for processing (1)
src/Database/Adapter/MariaDB.php
| table_schema = :database | ||
| UNION ALL | ||
| SELECT data_length + index_length AS size | ||
| FROM INFORMATION_SCHEMA.TABLES | ||
| WHERE table_name = :permissions AND | ||
| table_schema = :database | ||
| ) AS sizes | ||
| "); | ||
|
|
||
| $collectionSize->bindParam(':name', $collection); | ||
| $collectionSize->bindParam(':database', $database); | ||
| $permissionsSize->bindParam(':permissions', $permissions); | ||
| $permissionsSize->bindParam(':database', $database); | ||
| $statement->bindParam(':name', $collection); | ||
| $statement->bindParam(':permissions', $permissions); | ||
| $statement->bindParam(':database', $database); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 5 'PDO::ATTR_EMULATE_PREPARES|ATTR_EMULATE_PREPARES|new PDO\s*\(' . || true
rg -n -C 4 'table_schema = :database|bindParam.*:database' \
src/Database/Adapter/MariaDB.phpRepository: utopia-php/database
Length of output: 19931
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- MariaDB adapter structure and relevant methods ---'
ast-grep outline src/Database/Adapter/MariaDB.php
rg -n -C 12 'function __construct|function getPDOAttributes|getMaxIndexLength|table_schema = :database|bindParam' \
src/Database/Adapter/MariaDB.php src/Database/Adapter/SQL.php
printf '%s\n' '--- PDO construction and attribute overrides ---'
rg -n -C 8 'new PDO|ATTR_EMULATE_PREPARES|setAttribute|prepare\s*\(' \
src tests bin README.md -g '*.php' -g '*.md' | head -n 500
printf '%s\n' '--- Standalone PHP environment and marker probe ---'
if command -v php >/dev/null 2>&1; then
php -r 'echo "PHP ", PHP_VERSION, PHP_EOL; echo "PDO drivers: ", implode(",", PDO::getAvailableDrivers()), PHP_EOL;'
php <<'PHP'
<?php
$sql = <<<'SQL'
SELECT SUM(size) FROM (
SELECT data_length + index_length AS size
FROM INFORMATION_SCHEMA.TABLES
WHERE table_name = :name AND table_schema = :database
UNION ALL
SELECT data_length + index_length AS size
FROM INFORMATION_SCHEMA.TABLES
WHERE table_name = :permissions AND table_schema = :database
) AS sizes
SQL;
foreach (PDO::getAvailableDrivers() as $driver) {
try {
$dsn = $driver === 'sqlite' ? 'sqlite::memory:' : null;
if ($dsn === null) {
continue;
}
foreach ([true, false] as $emulate) {
$pdo = new PDO($dsn, null, null, [
PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION,
PDO::ATTR_EMULATE_PREPARES => $emulate,
]);
try {
$stmt = $pdo->prepare($sql);
echo "$driver emulate=" . ($emulate ? 'true' : 'false')
. " prepare=ok parameter_count=" . $stmt->parameterCount() . PHP_EOL;
} catch (Throwable $e) {
echo "$driver emulate=" . ($emulate ? 'true' : 'false')
. " prepare=" . get_class($e) . ': ' . $e->getMessage() . PHP_EOL;
}
}
} catch (Throwable $e) {
echo "$driver setup=" . get_class($e) . ': ' . $e->getMessage() . PHP_EOL;
}
}
PHP
else
echo 'php unavailable'
fiRepository: utopia-php/database
Length of output: 42315
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- PDO wrapper and adapter connection access ---'
ast-grep outline src/Database/PDO.php src/Database/PDOStatement.php src/Database/Adapter/SQL.php
rg -n -C 15 'class PDO|class PDOStatement|function __construct|function getPDO|function reconnect|ATTR_EMULATE_PREPARES|prepareNative|__call' \
src/Database/PDO.php src/Database/PDOStatement.php src/Database/Adapter/SQL.php
printf '%s\n' '--- Constructor and documented connection contract ---'
rg -n -C 10 'new MariaDB|MariaDB\(|Utopia\\Database\\PDO|Database\\PDO|setAttribute|ATTR_EMULATE_PREPARES' \
README.md docs src tests bin composer.json composer.lock 2>/dev/null || true
printf '%s\n' '--- PDO attribute configuration definitions ---'
rg -n -C 8 'getPDOAttributes|pdoAttr|attrs' bin src tests -g '*.php'Repository: utopia-php/database
Length of output: 50376
🌐 Web query:
PHP PDO::prepare named parameter marker cannot be used more than once in a prepared statement emulated prepares
💡 Result:
By default, PHP PDO prepared statements do not allow a named parameter marker to be used more than once in a single SQL statement [1][2]. When native prepared statements are used, the database driver expects each parameter marker to correspond to a unique binding, and repeating the same name will typically cause an error or unexpected behavior because the underlying database API cannot map multiple occurrences of the same name to the same value [3]. However, you can use the same named parameter multiple times if you enable emulated prepared statements [1][4]. This is done by setting the PDO::ATTR_EMULATE_PREPARES attribute to true [5][4]: $pdo->setAttribute(PDO::ATTR_EMULATE_PREPARES, true); When emulation mode is enabled, PDO does not send the query to the database server during the prepare phase [6]. Instead, it parses the SQL string in PHP, manually replaces the named placeholders with the provided values (using quoting where necessary), and sends the resulting fully-formed query to the database for execution [3][6]. Because this substitution happens entirely within PHP before the query is sent to the database, the restriction on repeating placeholders is bypassed [3]. While enabling emulation mode allows for repetitive named parameters, it is important to be aware of the trade-offs: 1. Security: While emulated prepares still protect against SQL injection by properly quoting values, native prepared statements are generally preferred as they provide a more robust separation of query logic and data at the database engine level [1][5]. 2. Behavior: Emulated prepares do not perform server-side validation of the SQL syntax during the prepare phase, meaning you may only discover syntax errors when the query is actually executed [6]. If you prefer to keep emulation mode disabled (the default in most modern PDO configurations), the standard solution is to use unique names for each placeholder (e.g.,:name1,:name2) and bind the same value to each one [2][4].
Citations:
- 1: https://www.php.net/pdo.prepare
- 2: https://stackoverflow.com/questions/27461763/phps-pdo-prepared-statement-am-i-able-to-use-one-placeholder-multiple-times
- 3: https://stackoverflow.com/questions/74863922/why-does-pdo-allow-to-use-indexed-array-with-named-placeholders-but-only-when-e
- 4: https://stackoverflow.com/questions/2432084/pdo-parameterized-query-reuse-named-placeholders
- 5: https://stackoverflow.com/questions/18511645/use-bound-parameter-multiple-times
- 6: https://www.php.net/manual/en/pdo.prepare.php
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- SQL connection storage and access ---'
sed -n '1,115p' src/Database/Adapter/SQL.php
rg -n -C 8 'function getPDO|function __construct|protected .*pdo|public .*pdo' src/Database/Adapter/SQL.php
printf '%s\n' '--- PDO wrapper API ---'
sed -n '1,150p' src/Database/PDO.php
sed -n '150,330p' src/Database/PDO.php
printf '%s\n' '--- PDO statement binding API ---'
rg -n -C 6 'class PDOStatement|function bindParam|function bindValue|function execute|function __construct|prepareNative' \
src/Database/PDOStatement.php src/Database/PDO.php
printf '%s\n' '--- MariaDB construction and PDO usage ---'
sed -n '1,45p' src/Database/Adapter/MariaDB.php
rg -n -C 5 'getPDO\(\)->prepare|new MariaDB|new \\?PDO|new PDO|ATTR_EMULATE_PREPARES' \
src/Database/Adapter/MariaDB.php src/Database/PDO.php README.md tests/e2e/Adapter/MariaDBTest.phpRepository: utopia-php/database
Length of output: 38641
Use distinct PDO markers for the two schema predicates.
When native prepares are enabled, PDO does not support reusing :database. MariaDB accepts caller-supplied PDO connections, so the emulated-prepare default is not enforced. Bind $database to two distinct markers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Database/Adapter/MariaDB.php` around lines 293 - 304, Update the SQL in
the MariaDB adapter’s size query to use distinct PDO placeholders for each
table_schema predicate, then bind $database to both markers in the adjacent
bindParam calls. Keep the existing $collection and $permissions bindings
unchanged.
Source: MCP tools
1 participant
Summary by CodeRabbit