Decouple identifier management from database Connection, introduce an IdentifierHandler class

A first patch, splitting the identifiers' resolution in a class of its own. I'd like to move table prefix management here too.

Progress with SQLIte, MySql and PgSql will fail. Mainly trying to move prefixes to the new class, deprecating Connection::tablePrefix. Ideally we could get to deprecating also ::setPrefix.

Will come back on this later.

I think this is D10 material, actually.

Needs to do #3124382: Remove per-table prefixing first, anyway.

Refreshed a bit.

So, here is the idea.

Developers should not be worried by exceeding identifiers length or using wrong identifiers; the database API should be self-sufficient in receiving input and cleansing/transforming it in machine usable information; failing as a last resort.

Therefore:

1. Table names, database name, column names, index names in a db are 'identifiers'.
2. 'Identifiers' are subject to db-specific limitations (length, chars allowed, etc)
3. In Drupal, 'Identifiers' are value objects. Each identifier has:
   • a type (e.g. table, column, alias, etc)
   • a raw value (e.g. 'config', 'key_value', 'mydb."key_value"', 'with$$invalidchar', etc)
   • a canonical value, i.e. the raw value cleansed of unallowed characters (e.g. from the above 'config', 'key_value', 'mydb.key_value', 'withinvalidchar', etc)
   • a machine value, i.e. the canonical value transformed to be used on the db-specific SQL query - including applying quote chars, prefixing, and shortening if necessary (e.g. from the above, assuming a prefix 'abba': '"abbaconfig"', '"abbakey_value"', '"mydb"."key_value"', '"abbawithinvalidchar"', etc)
4. each db-driver implements an 'IdentifierHandling' class that parses/transforms a raw value into the relevant value object, including a local cache to prevent processing again and again each raw value.

This works now for MySql and SQLite; the MySql implementation shortens table names that are longer than 64 characters using hashing techniques.

PgSql will follow with its own 63 chars max limitation.

In follow ups, this approach could be extended from tables to columns, aliases, indexes etc.

Added pgsql conversion. Tests are now green on all 3 core drivers. The concept is in and implemented; now to cleaning, BC and documenting.

Ready for review now.

The machineName in the value objects should be stored unquoted. On that.

Done #22, back to NR.

Filed follow-up #3514117: [PP-2] Use Table identifier value objects instead of table name strings in database Schema classes

#24 thanks Daffie for your review!

1) for sure we can undeprecate those, or postpone removal to a later major (D13+). But why would that be desirable? Besides that those 'wrappers' would be redundant, what would someone call those for, given that the value objects encapsulate the result of those wrappers? Besides, once we move to value objects then we could pass them as arguments for methods instead of strings and the reason for such wrappers would simply not hold anymore - see for example #3514117: [PP-2] Use Table identifier value objects instead of table name strings in database Schema classes and how sqlite's Schema class would benefit.

Supposing a prefix 'foo' is required, given that the Table identifier implements \Stringable and it returns the machine name identifier:

$table = $this->connection->identifiers->table('try_me');
echo "SELECT * FROM $table";

would return, without any further processing:

for SQLite:
SELECT * FROM "foo"."try_me"

for MySQL
SELECT * FROM "footry_me"

for PostgreSql (supposing a 'bar' schema is in use)
SELECT * FROM "bar"."footry_me"

2) that's exactly what the MR is doing no? Identifier/Database, Identifier/Schema, Identifier/Table are value objects, defined final - they only carry the values. The values themselves are determined by each driver's IdentifierHandler class, that parses the 'raw value' string identifier and instantiates the value objects with the needed values. Each driver's IdentifierHandler extends from the abstract IdentifierHandlerBase which has the 'regular' implementation - and all drivers need diversions one way or another, say for the different max lenght, or for how tables are prefixed, or for adding schemas etc.

Updated fork.

Filed follow up #3514261: [PP-1] Allow using Table identifier value objects in database Query classes.

#35: this is a totally alternative approach, that removes the need for the other issue.

Too long names are automatically shortened here to match the db limitations.

Thanks @mradcliffe for your review! The bigger picture is better understandable IMHO if this is reviewed together with #3514261: [PP-1] Allow using Table identifier value objects in database Query classes and #3514117: [PP-2] Use Table identifier value objects instead of table name strings in database Schema classes that will show how this will improve readibility of code once we start using the identifiers as parameters in methods.

Re the BC concern:

If a site provided a prefix and used a non-core database driver would that site stop working because $prefix isn't set anywhere by that database driver (if it relied on the protected property)?

I added here a more robust handling of the deprecated $prefix variable through the magic methods.

Definitely review by maintainers of non-core drivers would help; however I am not sure how many D11 non-core db drivers are out there; MSSQL and Oracle seem stuck at earlier Drupal versions.

@daffie thanks for the detailed review, it will take some time before I can get to it. In the meantime, please consider this is not the endgame, and consider reviewing this in combination with #3514261: [PP-1] Allow using Table identifier value objects in database Query classes and #3514117: [PP-2] Use Table identifier value objects instead of table name strings in database Schema classes that would further implement the changes occurring here.

For instance, when you talk about DX, the endgame would be using the new identifier object with its automatic conversion to string: for example,

$query = $comments . 'DELETE FROM {' . $this->connection->escapeTable($this->table) . '} ';

would become

$query = $comments . "DELETE FROM $this->tableIdentifier";

That's quite a DX improvement IMO... just we cannot do all of it toghether in a single MR otherwise it will become too big.

#43 you mean manually? An idea could be to create via createTable a PostgreSQL table with a very long name. Then inspecting the database you would find a table with an abbreviated name 63 chars long. Any Database API select/insert/update/query etc with the long name in Drupal would return data from the real table with abbreviated name.

fork update

rebased

Sorry @daffie for some reason I cannot see your remarks on the GitLab UI, they show as collapsed and when I try to open them nothing shows up - I see what you wrote in the issue here, but the problem is I cannot see 'where' they are positioned. Can you try again or give some more details in comments here?

I think we should postpone this for the moment, and let #3411490: Replace array-based DB Schema API with a value object structure land first. That one may have implication/connections with this one, so let's pause for a moment.