Visitar URL original
Add mysqli exception mode by kamil-tekiela · Pull Request #349 · joomla-framework/database · GitHub
Skip to content

Add mysqli exception mode - #349

Merged
richard67 merged 4 commits into
joomla-framework:3.x-devfrom
kamil-tekiela:Add-mysqli-exception-mode
Nov 9, 2025
Merged

richard67 merged 4 commits into
joomla-framework:3.x-devfrom
kamil-tekiela:Add-mysqli-exception-mode

Conversation

@kamil-tekiela

Copy link
Copy Markdown
Contributor

This PR changes the MysqliDriver to use exception mode just like PDO does. I tried not to make too many changes and stick to the previous style. There was some dead code in executeUnpreparedQuery which I removed to make the change easier.

@HLeithner

Copy link
Copy Markdown
Contributor

thanks, maybe we can remove the warning suppression (@) now too? other question is can we get the state before we set it and set it back after connecting?

It also seems that MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT is the default since php 8.1, that would mean we already get exceptions in this driver but doesn't catch them?

@kamil-tekiela

Copy link
Copy Markdown
Contributor Author

thanks, maybe we can remove the warning suppression (@) now too? other question is can we get the state before we set it and set it back after connecting?

Thanks for pointing this out. I removed the error suppression.
We can get the state from mysqli_driver->report_mode but I don't see why we would need to do that.

It also seems that MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT is the default since php 8.1, that would mean we already get exceptions in this driver but doesn't catch them?

Yes, that would be true. Hence, why the current code needs to be changed because it doesn't work as intended. I still like to set the error mode explicitly though.

@rdeutz rdeutz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

code review

@richard67 richard67 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code review ok, and I've also done some tests on 5.4-dev of the CMS.

@richard67
richard67 merged commit a567e9b into joomla-framework:3.x-dev Nov 9, 2025
@richard67

Copy link
Copy Markdown
Contributor

Thanks @kamil-tekiela for this fix, and @HLeithner and @rdeutz for reviews.

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.

4 participants