Skip to content

PDO refactoring for code simplification#2522

Merged
Alkarex merged 17 commits intoFreshRSS:devfrom
Alkarex:pdo_refactor
Sep 29, 2019
Merged

PDO refactoring for code simplification#2522
Alkarex merged 17 commits intoFreshRSS:devfrom
Alkarex:pdo_refactor

Conversation

@Alkarex
Copy link
Copy Markdown
Member

@Alkarex Alkarex commented Sep 15, 2019

(As promised in #2496 . It may seem like a lot, but it is mainly some fairly automatic changes)

  • Automatic prefix when using the syntax _tableName (with backticks)
    • This allows writing:
SELECT * FROM `_entry`;

instead of:

SELECT * FROM `' . $this->prefix . 'entry`; 

As well as writing:

return $this->pdo->lastInsertId('`_tag_id_seq`');

instead of:

return $this->bd->lastInsertId('"' . $this->prefix . 'tag_id_seq"');
  • Uniformity: MySQL is now using PDO::ATTR_EMULATE_PREPARES = false just like SQLite and PostgreSQL, with consequences such as only one statement per query. See e.g. https://phpdelusions.net/pdo#emulation
  • Use PDO methods exec(), query(), prepare() + execute() in a more efficient way
  • Remove auto-update SQL code for versions older than FreshRSS 1.5 (3 years old)
  • The name of the default category is set in PHP instead of in the DB (simplies SQL and allows changing the name according to the FreshRSS language)
  • Rename ->bd to ->pdo (less of a frenshism, and more informative)
  • Fix some requests, which were not compatible with MySQL prepared statements

* Automatic prefix when using the syntax `_tableName`
* Uniformity: MySQL is now PDO::ATTR_EMULATE_PREPARES = false just like SQLite and PostgreSQL, with consequences such as only one statement per query
* Use PDO methods exec(), query(), prepare() + execute() in a more efficient way
* Remove auto-update SQL code for versions older than FreshRSS 1.5 (3 years old)
* The name of the default category is set in PHP instead of in the DB (simplies SQL and allows changing the name according to the FreshRSS language)
* Rename `->bd` to `->pdo` (less of a frenshism, and more informative)
* Fix some requests, which were not compatible with MySQL prepared statements
@Alkarex Alkarex added the Work in progress 🚧 Wait before merging label Sep 15, 2019
@Alkarex Alkarex added this to the 1.15.0 milestone Sep 15, 2019
@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented Sep 15, 2019

No ready yet: I lack some work in install.php (to try to reuse the main code instead of duplicating it), a syntax update for PostgreSQL nextval() and lastInsertId(), and a bit for MySQL in the SQL file.

@Alkarex Alkarex mentioned this pull request Sep 16, 2019
Alkarex added a commit to Alkarex/FreshRSS that referenced this pull request Sep 17, 2019
@Alkarex Alkarex mentioned this pull request Sep 17, 2019
@Alkarex Alkarex mentioned this pull request Sep 19, 2019
@Alkarex
Copy link
Copy Markdown
Member Author

Alkarex commented Sep 20, 2019

@marienfressinaud Maybe you could have a look at this PR before you leave for vacation, at least its general principles. install.php is not completely finished (not working fully at the moment) but when deployed over an existing installation, this PR should already work more or less (more testing needed though).

Copy link
Copy Markdown
Member

@marienfressinaud marienfressinaud left a comment

Choose a reason for hiding this comment

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

Most of these changes are very welcomed 👍 I don't have a lot to say except that I'm not totally comfortable with my review since the PR is quite big. Smaller dedicated PRs or even more focused commits would help a lot :)

@Alkarex Alkarex removed the Work in progress 🚧 Wait before merging label Sep 29, 2019
@Alkarex Alkarex changed the title A bit of PDO refactoring PDO refactoring for code simplification Sep 29, 2019
@Alkarex Alkarex merged commit e3e5954 into FreshRSS:dev Sep 29, 2019
@Alkarex Alkarex deleted the pdo_refactor branch September 29, 2019 14:22
Alkarex added a commit to Alkarex/FreshRSS that referenced this pull request Sep 29, 2019
Alkarex added a commit that referenced this pull request Sep 29, 2019
Alkarex added a commit to Alkarex/FreshRSS that referenced this pull request Sep 29, 2019
Alkarex added a commit that referenced this pull request Sep 29, 2019
This was referenced Oct 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants