Conversation
|
/cc @NinoFloris |
NinoFloris
left a comment
There was a problem hiding this comment.
Thanks for the PR.
I agree we should fix it, though it's not critical as it can easily be worked around.
| public bool TypeEquals(Type type) => TypeMatchPredicate?.Invoke(type) ?? Type == type; | ||
| public bool DataTypeNameEquals(string dataTypeName) | ||
| { | ||
| var norm = Postgres.DataTypeName.NormalizeName(dataTypeName); |
There was a problem hiding this comment.
| var norm = Postgres.DataTypeName.NormalizeName(dataTypeName); | |
| var normalized = Postgres.DataTypeName.NormalizeName(dataTypeName); |
There was a problem hiding this comment.
We can't do the normalization here, as it would very negatively affect startup time and in general mapping resolution time. But it does make sense for the collection lookup to do this.
We should define 2 new methods: private DataTypeNameEqualsCore essentially a rename of the original implementation. Then the current public DataTypeNameEquals(string) can do a normalize and call into core for the collection lookups. Finally we'd also add internal DataTypeNameEquals(DataTypeName) to be used in Find, also calling into core by taking the value of the DataTypeName.
There was a problem hiding this comment.
Thanks for the guidance! I initially added some tracing to check normalization results and indeed there were a lot of calls to it (coming from data source builders' default ConfigureDefaultFactories implementations, IIUC).
NinoFloris
left a comment
There was a problem hiding this comment.
Apologies for missing this @0MG-DEN, it all looks good. Thanks!
|
Backported to 9.0.4 via 8aa2ce8 |
Fixes #6010