[ticket/14463] Add Doctrine DBAL as a driver

[CHItA]

This probably breaks all db tools etc, so it is mostly opened to gather feedback.

Checklist:

• Correct branch: master for new features; 3.3.x & 3.2.x for fixes
• Tests pass
• Code follows coding guidelines: master, 3.3.x and 3.2.x
• Commit follows commit message format

Tracker ticket (set the ticket ID to your ticket ID):

https://tracker.phpbb.com/browse/PHPBB3-14463

[ghost]

I have played with this a few months back aswell.
In case you’re interested: https://github.com/mrgoldy/phpbb/tree/ticket/14463

[CHItA]

As of now, this method is just horse shit mixed with snake oil, as fetch_all() doesn't fetch all but all remaining rows from the current cursor position. The problem here is saving all rows (which is one option to actually support going backwards) would add some overhead on all queries. I'm not sure if breaking BC here would make more sense, as I guess this might be some functionality that is not used at all by anyone (certainly not by the core).

@marc1706 @Derky @Nicofuma thoughts?

[marc1706]

IMHO if there is no good way to achieve this then don't. Especially if it's not even used in the core. We should however make sure to properly document this BC break, e.g. in the documentation. I can prepare a page for this (similar to how symfony is documenting those changes: https://github.com/symfony/symfony/blob/master/UPGRADE-5.0.md).

[CHItA]

The only solution I see is to store all fetched rows in the iterator. I doubt that there would be any other solution to this.

[CHItA]

Although we could reexecute the query as well when backward iteration is called for, thus only having any overhead when it is absolutely required.

[ghost]

Is it not possible to save queries that are made per page in a protected property? This will also allow extensions to access SQL queries that are already made by the core, without having to rerun them if they're not cached.
For example saving the SQL queries in $queries and all the SQL results in $results.

protected $queries = [
    0 => 'SELECT * FROM `phpbb_forums`',
];

protected $results = [
    0 => [
        0 => ['forum_id => 1', '...' => '...'],
    ],
];

So you can iterate over them and check before querying to see if it already exists.

[CHItA]

I don't think we would want to add any new features to this, as the goal here is to keep BC as much as possible. Also, if you re-execute a query, chances are it comes from the cache which should be much faster. Thirdly, extensions should not really execute the same queries as that data probably is accessible to them from somewhere.

[ghost]

Well, that's not completely true as a lot of queries in the core are not cached and not accessible for extensions, if not in tedious ways. But it was just an idea.

[CHItA]

Extension authors can add events if they want to.

[ghost]

What I not fully understand, is that the idea was to 'nuke dbal' and to eventually get rid of the phpBB part all together. Wouldn't it then make more sense to nuke it completely. Namely, replacing phpBB DBAL with Doctrine DBAL and implementing a phpBB wrapper class (for BC) that can eventually be deprecated and removed, rather than keeping the phpBB DBAL and adding a Doctrine driver?

[CHItA]

What I not fully understand, is that the idea was to 'nuke dbal' and to eventually get rid of the phpBB part all together. Wouldn't it then make more sense to nuke it completely. Namely, replacing phpBB DBAL with Doctrine DBAL and implementing a phpBB wrapper class (for BC) that can eventually be deprecated and removed, rather than keeping the phpBB DBAL and adding a Doctrine driver?

That is exactly what this PR does. See the @depreceted annotation on doctrine. I'm not really sure if this was your question.

[ghost]

Ah, I kind of misinterpreted this PR. As you are indeed doing it.
However, in a different fashion that I had imagined. I would of thought of adding a wrapperClass rather than using a factory. Both work I suppose and with this you keep the class \phpbb\db\driver\driver_interface alive. Okay I get it I think.

[rubencm]

https://github.com/doctrine/dbal/search?p=1&q=getTruncateTableSQL&type=code
https://www.thetopsites.net/article/51786076.shtml

In case you want to implement #5329 here