Doctrine DBAL prepared statement troubles

General discussion of development ideas and the approaches taken in the 4.x branch of phpBB. The next feature release of phpBB 4 will be 4.0/Triton.
Forum rules
Please do not post support questions regarding installing, updating, or upgrading phpBB 4.x. If you need support for phpBB 3.3.x please visit the 3.3.x Support Forum on phpbb.com.

If you have questions regarding writing extensions please post in Extension Writers Discussion to receive proper guidance from our staff and community.
CHItA
Development Team
Development Team
Posts: 169
Joined: Thu Mar 12, 2015 1:43 pm
Location: Budapest, Hungary

Doctrine DBAL prepared statement troubles

Post by CHItA »

My previous plans were to pull in Doctrine DBAL and suggest using it directly instead of our DBAL. This has quite a few advanteges: getting new features such as prepared statements, not having to maintain our own DBAL, etc. We would also keep our own interface, and implement it with Doctrine, so BC is kept for extensions and parts of the code we will have not yet updated.

The problem is that Doctrine drivers are not transparent, as in you cannot use the same SQL on all the drivers. This is a big problem for us, as no one should be expected to write different SQL based on which driver is running underneath. One example where this occurs is when using parameters in prepared statements. Only positional parameters are supported by all drivers - this is just my assumption, actually I could not find any guarantee for this either. Thus we have a couple of choices:
  • Require using positional parameters
  • Hack our abstractions into the DBAL drivers, that fix their "broken" drivers
  • Annoy them with a feature request until they comply - note: our use case might be quite different then most of their user base (as in we have no idea what driver is used)
  • keep our good old DBAL
Doctrine has the code to convert between all parameter representation, however it is only implemented in the Connection object, so if you call executeUpdate() or executeSelect() you should be able to use any parametrization on any driver. However, if one calls prepare on the connection object all bets are off, you get what you get, which is kind of not good enough for our purposes.
User avatar
3Di
Registered User
Posts: 885
Joined: Tue Nov 01, 2005 9:50 pm
Location: Milano 🇮🇹 Frankfurt 🇩🇪
Contact:

Re: Doctrine DBAL prepared statement troubles

Post by 3Di »

I'd stay with our good old DBAL.
🆓 Free support for our extensions also provided here: phpBB Studio
🚀 Looking for a specific feature or alternative option? We will rock you!
Please PM me only to request paid works. Thx. Want to compensate me for my interest? Donate
My development's activity º PhpStorm's proud user º Extensions, Scripts, MOD porting, Update/Upgrades
User avatar
david63
Registered User
Posts: 326
Joined: Mon Feb 07, 2005 7:23 am
Location: Lancashire, UK

Re: Doctrine DBAL prepared statement troubles

Post by david63 »

Let's play a bit of Devil's Advocate here.
  • What advantages will Doctrine give to phpBB other than not having to maintain it?
  • Are there features/facilities in Doctrine that phpBB needs?
  • Are there currently any issues with phpBB's DBAL?
  • How much time/effort is spent maintaining phpBB's DBAL?
  • How much effort would be required to convert all the current sql queries to Doctrine, and for what gain?
  • What would happen if a future change to Doctrine "broke" phpBB?
I would not worry about BC for extensions as long as Doctine goes into phpBB 4, as if all the planned features for phpBB4 go ahead then virtually all extensions will have to be as a minimum modified, if not re-written.

If Doctrine would work "out of the box" then it would be a no brainer to go with Doctrine but as that is not the case then I would agree to stay with the current DBAL.
David
Remember: You only know what you know -
and you do not know what you do not know!
CHItA
Development Team
Development Team
Posts: 169
Joined: Thu Mar 12, 2015 1:43 pm
Location: Budapest, Hungary

Re: Doctrine DBAL prepared statement troubles

Post by CHItA »

david63 wrote: Wed Jan 13, 2021 8:36 am
  • What advantages will Doctrine give to phpBB other than not having to maintain it?
  • Are there features/facilities in Doctrine that phpBB needs?
  • Are there currently any issues with phpBB's DBAL?
Prepared statements, more powerful query builder just to name a few. But not having to maintain our own DBAL is good enough for me.
david63 wrote: Wed Jan 13, 2021 8:36 am
  • Are there currently any issues with phpBB's DBAL?
  • How much time/effort is spent maintaining phpBB's DBAL?
IIRC there are some bugs with Oracle, we just haven't tested it historically so it wasn't all that apparent. Maintainance time is roughly zero, which kind of is the problem.
david63 wrote: Wed Jan 13, 2021 8:36 am How much effort would be required to convert all the current sql queries to Doctrine, and for what gain?
No effort required whatsoever.
david63 wrote: Wed Jan 13, 2021 8:36 am What would happen if a future change to Doctrine "broke" phpBB?
Broke it how? I don't really see that happening, more than Symfony breaking phpBB.
User avatar
3Di
Registered User
Posts: 885
Joined: Tue Nov 01, 2005 9:50 pm
Location: Milano 🇮🇹 Frankfurt 🇩🇪
Contact:

Re: Doctrine DBAL prepared statement troubles

Post by 3Di »

There are currently no problems with our DBAL (DataBase Abstraction Layer) so I don't see why we need to change it for something new to us, especially if we are talking about phpBB version 4.0, at most I would move the topic to version 4.1. Where hopefully we will also see a new style. Discussing topics like this which certainly don't bring great improvements, is right but it's proven to slow down release times and we are already late for one reason or another. Here we are discussing one thing while bigger issues still need to be completed, such as bringing *cps to controllers. Incorporating PRs that were part of GSOC 2018 etc. like mentions and blah blah.

About Oracle DB, let's remove it, please.

Edit: typos
Last edited by 3Di on Wed Jan 13, 2021 4:16 pm, edited 1 time in total.
🆓 Free support for our extensions also provided here: phpBB Studio
🚀 Looking for a specific feature or alternative option? We will rock you!
Please PM me only to request paid works. Thx. Want to compensate me for my interest? Donate
My development's activity º PhpStorm's proud user º Extensions, Scripts, MOD porting, Update/Upgrades
User avatar
rxu
Registered User
Posts: 155
Joined: Tue Apr 04, 2006 4:28 pm
Contact:

Re: Doctrine DBAL prepared statement troubles

Post by rxu »

As for the Oracle issue, I tried to fix all errors arised during tests on Github Actions and found out that phpBB's Oracle driver was actually never compatible with Oracle, at least it's not compatible with 11g (which is 13 years old), so apparently phpBB simply was never really Oracle-compatible, despite the driver was included and compatibility was declared as such. So, as far as phpBB afaik almost didn't get Oracle-related support requests, it seems that phpBB userbase when it comes to Oracle is about 0.
The only viable perspective would be to require 12c as a minimum with refactoring the driver, but currently there's no option to run tests on 12c with Github Actions.
So Oracle-compatibility looks as pretty weak argument for Doctrine DBAL for me.
Having different SQL queries for different SQL drivers is a blocker as for me, having some compatibility layer to workaround this makes migration to Doctrine kinda useless as we get the same point of requiring maintenance for this layer.

As a bottom line, having such a lot of issues and possible restrictions with Doctrine makes me feel like keeping "our old good DBAL" is the choice until some better option pops up.
Image
CHItA
Development Team
Development Team
Posts: 169
Joined: Thu Mar 12, 2015 1:43 pm
Location: Budapest, Hungary

Re: Doctrine DBAL prepared statement troubles

Post by CHItA »

rxu wrote: Wed Jan 13, 2021 3:35 pm Having different SQL queries for different SQL drivers is a blocker as for me, having some compatibility layer to workaround this makes migration to Doctrine kinda useless as we get the same point of requiring maintenance for this layer.
I'm not really sure if having different queries for different drivers was ever an option. You don't have to change your SQL. In that regard there is no difference.
User avatar
rxu
Registered User
Posts: 155
Joined: Tue Apr 04, 2006 4:28 pm
Contact:

Re: Doctrine DBAL prepared statement troubles

Post by rxu »

CHItA wrote: Wed Jan 13, 2021 4:28 pm You don't have to change your SQL.
CHItA wrote: Tue Jan 12, 2021 8:19 pm The problem is that Doctrine drivers are not transparent, as in you cannot use the same SQL on all the drivers.
Then I just didn't manage to understand this statement.
Image
CHItA
Development Team
Development Team
Posts: 169
Joined: Thu Mar 12, 2015 1:43 pm
Location: Budapest, Hungary

Re: Doctrine DBAL prepared statement troubles

Post by CHItA »

That is regarding prepared statements. So you cannot use the same parameters. Not all drivers support named parameters. The rest of SQL differences are handled similarly as the current DBAL, namely, you need to call a function to retreive the correct syntax, see sql_lower_text() etc for examples. But I admit, I wasn't as clear as I should have been.
User avatar
3Di
Registered User
Posts: 885
Joined: Tue Nov 01, 2005 9:50 pm
Location: Milano 🇮🇹 Frankfurt 🇩🇪
Contact:

Re: Doctrine DBAL prepared statement troubles

Post by 3Di »

@ CHItA
let's face it, what do you think of all this? 🧐
🆓 Free support for our extensions also provided here: phpBB Studio
🚀 Looking for a specific feature or alternative option? We will rock you!
Please PM me only to request paid works. Thx. Want to compensate me for my interest? Donate
My development's activity º PhpStorm's proud user º Extensions, Scripts, MOD porting, Update/Upgrades
Post Reply