Consider making RelationalTypeMappingInfo concrete

[roji]

RelationalTypeMappingInfo is currently abstract. In Npgsql it's necessary to internally create RelationalTypeMappingInfo when finding an array mapping, which involves constructing and finding the element mapping first. There doesn't seem to be any particular reason to stop RelationalTypeMappingInfo from being instantiated directly, rather than subclassing it without adding any value. I might be missing the real reason for making it abstract, though.

This isn't urgent as it's easy to work around it by subclassing (note that a private internal ConcreteRelationalTypeMappingInfo exists inside .RelationalTypeMappingSource).

[roji]

Note that there also doesn't seem to be a constructor on RelationalTypeMappingInfo which allows it to be constructed with both a ClrType and a StoreType.

If you really intend this class to be constructed only within the core, I'll try to work around it some other way.

[ajcvickers]

@roji The reason TypeMappingInfo is not insatiable is protection against code creating a core mapping info when a relational one should have been created instead. More generally, the construction of TypeMappingInfo objects is constrained so that there is less chance that the wrong kind of mapping info is created. The intention was that providers could maintain this by creating new subclasses as needed if the provider needed to flow additional info through the type mapper.

Can you explain a bit more why the provider needs to create a RelationalTypeMappingInfo? And related to this, where the ClrType and StoreType are coming from? (I'm not against adding a PR to add a new constructor, but I'd like to understand a bit more about how it is being used.)

[ajcvickers]

@roji Did you have any further thoughts on this?

[roji]

Sorry for disappearing on this... Take a look at https://github.com/npgsql/Npgsql.EntityFrameworkCore.PostgreSQL/blob/dev/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs#L318, which is where I get the mapping for an array.

As you can see, the method takes the array mapping it is given and attempts to look up the corresponding element mapping for that array. If it's given a store type, it parses integer[] into integer, and if given a CLR type, it gets the element type via Type.GetElementType().

This works well, except that it can only look up element mappings either by store type, or by CLR type. To do things properly I need to provide both when looking up the element mapping - this is what #11359 was about. In other words, if a store type is specified on an array property, I still need the CLR type to select the correct mapping out of the list (you can see this happening here). I currently have two data types which require this: PostgreSQL smallint (which is used to store both CLR short and byte), and PostgreSQL timestamp with time zone (which is used to store both DateTime and DateTimeOffset).

However, I can't simply instantiate a RelationalTypeMappingInfo - which after all isn't much more than a holder for the store type, CLR type, etc. - to recursively call FindMapping().

This definitely isn't super-urgent (can be punted for post-2.1) - it will only affect users trying to use arrays of timestamp without time zone, which really is a rare specific case.