WIP - Database refactoring

[nupplaphil]

• For Addon pre-loaded capability
• Making Datbase results "object oriented"
• Either a IDatabaseResult or a IDatabaseError returns
• encapsulates all "statement" based calls into the corresponding Result-object (fetch, affected rows, ...)
• Splits PDO and MysqlI driver specific namespaces
• Preparation for new Driver specific classes (postgresql?)

[MrPetovan]

Postgre hell yeah!

[annando]

Please keep in mind that other databases like PostgreSQL should be connected via PDO. So a MySQL PDO and a PostgreSQL PDO will have most code in common. The differences are at the places where we create the SQL command (since PostgreSQL uses a different way to escape table names and field names.

So possibly it would be an idea, to complete drop MySQLi and to work solely via PDO.

[nupplaphil]

Ok, so we would have just one driver (pdo) and stick to it.

As far as I read about PDO, it could enable support for other DBMS as well, but, as you said, there are language specific differences

Shall I proceed with PDO only? The origin intention was to proper separate the core from our select, insert abstraction.

Thus the execution flow would be:

1. ReadOnlyFileConfig for the DB settings
2. IDatabaseConnection only with this config, nothing else
3. DatabaseConfig with just IDatabaseConnection, so the config uses the core!
4. FullDatabase class with logging, profiling settings from the Config table and other convenience features :-)
5. The Addon class uses just the DatabaseConfig, so we can retrieve the add-ons before loading the FullDatabase which enables to include all add-on specific dbstructures, logger implementations, ..

The last one was the orignal goal of this PR and lead to this change g

[annando]

I would like to know @MrPetovan's opinion about this as well. Also we should ask in the forum if admins are using PDO or MySQLi.

[MrPetovan]

I'm not familiar enough with PostgreSQL to know the caveats of using PDO to interface with it.

Otherwise, I'm not happy with the naming, I feel like FullDatabase calls for a HalfDatabase or something to that effect. I'd rather have it the other way, having the regular Database handle the day-to-day calls, and a BootstrapDatabase with the read-only file config that loads addons, etc...

[nupplaphil]

The naming and the whole PR is highly WIP so we can discuss about it :-)

And fullDatabase was just a working title.. I'm with you about the naming suggestions and will follow it when adapting it

[nupplaphil]

a complete different question ..

Why don't we use a tiny, lightweight but fast library like https://github.com/ezSQL/ezsql which supports all kind of databases .. This would reduce this work massively ..

Did we already talk about such thing? Shall I start a discussion in our board?

At least the syntax of this library seems like understandable and almost identically to how we use it.
We could use all out insert and update implementation and "just" replace p and e with the library implementation

[MrPetovan]

The simple answer is that any significant change in the way we perform queries will incur a massive migration cost which has so far stymied any discovery efforts in this domain.

[nupplaphil]

I see 60 DBA::p() calls, the rest is covered with our insert, update, ... abstraction layer

• so 60 places where we would have to replace the SQL string with a nearly analog syntax
• the rest would be replacing the p and e calls inside the Database class with a library call

It seems like a more direct way than checking 60 places if they are PostgreSQL compatible and fixing them step by step :)

[MrPetovan]

Wow, I didn’t expect so few instances left of the freeform queries, I guess we have been effective refactoring the modules. We should finish this sometimes.

[annando]

In the last years I replaced most of the direct calls step by step with more high level calls. Much of this was done by now working a lot with views.

Concerning that library: I had a short glance at it. I haven't found anything that could help with our system of having a file like dbstructure.config.php and dbview.config.php where changing something there will automatically execute the appropriate database calls and the pre and post update calls. Also I saw that although there seems to be some call for inserting rows, you cannot define what should happen on a duplicate, like with our calls. Due to timing issues this cannot be solved with some code around it.

I had a look at the functions to alter the table structure. But it seems like these calls simply are some wrapper around the calls. There is no checking for the existing database structure for example. Means we will need the whole functionality in DBStructure.php anyway - and that seems to be most of the work for the being able to work with PostgreSQL. The other stuff will be comparable easy. When we are using PDO solely, most calls should work fine, we only will have to work at the escaping of field and table names.

Also I just saw that that other library doesn't seem to support the creation of views - which we really use a lot.

Also comments like this doesn't sound promising:

I'm really in maintenance mode with this library at the moment. There are many todo's/additional features that needs implementing.

Kind of expecting other contributors to fill in. I have this library doing what I need it for at the moment, and fixing any bug issues others raise.

[nupplaphil]

Ok I understand: stick to the plan :-)

Thanks for your explanation and insights about the necessary functionality

[annando]

Concerning PDO or MySQLi: I asked the admins in post https://forum.friendi.ca/display/ec054ce7-3264-0227-d9e1-be3580203355.

it seems as if everyone uses PDO. So possibly we could stick to it and should only vary in nuances between MySQL and PostgreSQL.

[MrPetovan]

Suggested change

	$result = $this->readInternal($sql, $parameters, false);
	$result = $this->writeInternal($sql, $parameters, false);

[HankG]

I'd be up for creating a test instance running Postgres :)

[tobiasd]

In case this PR shall be part of the 2023.09 (10) release, please redirect it towards the 2023.09-rc branch.