Use Doctrine DBAL for multi-database support - #591
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc02917e63
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return $data; | ||
| $result = $this->db->executeQuery($sql, $conditions); | ||
| $rows = iterator_to_array($result->iterateAssociative(), false); | ||
| $result->closeCursor(); |
There was a problem hiding this comment.
On supported Nextcloud versions, IDBConnection::executeQuery() and query-builder reads return a Doctrine DBAL Result, which has free() rather than the PDO closeCursor() method. Consequently, every legacy read routed through this helper fetches its rows and then throws an undefined-method error before returning; fetchAll() repeats the same invalid call. Use free() or omit the explicit release after exhausting the iterator.
AGENTS.md reference: AGENTS.md:L11-L11
Useful? React with 👍 / 👎.
| ->where($query->expr()->eq('f.id_configuration', $query->createNamedParameter($id_configuration))); | ||
|
|
||
| $totals = []; | ||
| foreach ($this->fetchAll($query) as $row) { |
There was a problem hiding this comment.
When a company has a large invoice history, this now fetches and retains every invoice line before grouping it in PHP, whereas the previous query performed SUM/GROUP BY in the database and returned only one row per month. The statistics request therefore scales in application memory and database transfer with the complete ledger and can time out or exhaust PHP's memory limit; use a portable database-side aggregation or at least stream the result instead of calling fetchAll().
Useful? React with 👍 / 👎.
1 participant
What changed
Bddwith NextcloudIDBConnectionDoctrine DBAL executionordercolumnoc_table prefixNOW()with a PHP-generated ISO dateNo translation catalogs or Transifex files are changed.
Why
The previous implementation obtained low-level prepared statements and embedded MySQL-specific SQL details. That could fail with PostgreSQL, SQLite, or a custom Nextcloud table prefix. Database access now goes through Nextcloud's Doctrine DBAL abstraction, with QueryBuilder used where identifier quoting is required.
Validation
lib/NOW(),EXTRACT(...), or hard-codedoc_gestionidentifiers in the database layergit diff --checkThe PHPUnit executable is not installed in this checkout (
vendor/bin/phpunitis absent), so the PHPUnit suite could not be run locally.