Deprecate static type methods

[greg0ire]

The type registry should be used directly instead.

[morozov]

IIRC, earlier we discussed that the type registry should become scoped to the wrapper connection at some point. Now looks like the right time to introduce Connection::getTypeRegistry(): TypeRegistry and recommend using it.

Promoting the usage of TypeRegistry::getInstance() feels like back in pre-Zend_Registry times 🤮

[morozov]

Also, speaking of the registries. At this point, it sounds like TypeRegistry and getTypeRegistry() are not the best names for the API.

At the time of the introduction of the type registry (#3354), technically, it was a set: each type could be (registered) in or not (registered) in the set/registry. Now it's a map of userland type names to the objects implementing the types.

I believe we should name the new method getTypeMapping() or getTypeMap() and later rename the registry class accordingly.

[greg0ire]

If I introduce such a property in the wrapper Connection, and then I start using it in that class, the test suite breaks because calls to Type::addType() won't have an effect on the instance stored in the connection object. I see an ugly solution to that issue: expose the existence of the singleton to TypeRegistry with a public internal method, and, then, in TypeRegistry, fallback to that singleton if it is defined. That way, there can be support for cases where part of your application has migrated to scope typed registry, while another part continues using the singleton. Do you see simpler solution?

[greg0ire]

Also, I don't know with what this should be replaced:

dbal/src/Schema/Table.php

Lines 349 to 356 in 8532d49

	public function addColumn($name, $typeName, array $options = [])
	{
	$column = new Column($name, Type::getType($typeName), $options);
	$this->_addColumn($column);
	return $column;
	}

[morozov]

I don't think this workaround is desired because it masks an API design issue: we want the type mapping to be scoped to a connection but we want schema objects to be unaware of the connections but aware of the type mapping.

We could do some prototyping and see what it will take to make the connection-scoped type mapping accessible to schema objects, e.g. add an argument of TypeRegistry to all schema object methods that depend on it (e.g. to Table::addColumn()). Alternatively, instead of accepting a string, those methods could accept the type instance. Actually, the latter looks even better to me since an API expressed in terms of objects would be more robust.

We can try prototyping without taking BC into account just to see what the resulting API changes will look like. We can thing of the BC and the upgrade path later.

[morozov]

Sorry, that would have been a type instance method, not a static method. We're trying to get rid of static type methods.

[greg0ire]

Yes, but let's say the type is 1… if bind() is to encapsulate using this as a driver-level binding type (which is an int, right?), then how do I pass that 1 to that method? Through which argument?

[morozov]

Yes, but let's say the type is 1…

How can a type be 1? It's an object. That's what I mean:

class IntegerType {
    public function bind(Statement $statement, int|string $param, mixed $value): void {
        $statement->bind($param, $value, ParameterType::INTEGER);
    }
}

The logic here is the same as in Type::getBindingType() which triggered this discussion with the only difference that the consumer of the Type API no longer asks: "what parameter type should I use to bind values of this type?". Instead, it tells: "bind this for me".

[greg0ire]

OK, I won't be able to try that until next week, but thanks, it's clear now

[morozov]

FWIW, I believe there will be a ton of work before this API turns into something reasonable. I'm not even sure it's doable in the 4.0 release timeframe unless we want to release a half-baked solution.