schotime / NPoco

Simple microORM that maps the results of a query onto a POCO object. Project based on Schotime's branch of PetaPoco
Apache License 2.0
848 stars 302 forks source link

ValueTupleRowMapper caching mappers too much #663

Open asztal opened 2 years ago

asztal commented 2 years ago

Hi,

I contributed the ValueTupleRowMapper a while ago and I've noticed an issue with it. It caches mappers based on the tuple type, but there is an issue - what if there are multiple queries that generate the same tuple type, but different column types coming from the database? E.g.

await db.FetchAsync<(string, int)>("select uuid, id from SomeTable"); // Columns: uniqueidentifier, int
await db.FetchAsync<(string, int)>("select name, id from SomeTable"); // Columns: varchar(50), int

The problem is that the current code caches the mappers based on the tuple type, which is (string, int) in both cases. However the mapper code needs to be different for each one. In my application I have a custom mapper (see below) which maps from Guid to string which gets correctly used in the first query, but then the second query also tries to use the same custom converter, which fails because the column value in the DbDataReader isn't a Guid!

        public override Func<object, object> GetFromDbConverter(Type destType, Type sourceType) {
            if (destType == typeof(string) && sourceType == typeof(Guid))
                return guid => ((Guid)guid).ToString();

            if (typeof(IEntityKey).IsAssignableFrom(destType) && sourceType == typeof(DBNull)) {
                return guid => null!;
            }

In my ValueTupleRowMapper I've approached it as seen below, but not sure if the best way of doing it. Seems like it will hurt performance, but I don't see a better way about it, hopefully it doesn't impact too much?

        private static Func<DbDataReader, object> GetRowMapper(Type type, MapperCollection mappers, DbDataReader dataReader) {
            // Get a list of column types to ensure that the caching is done correctly.
            // This is necessary because the mapping depends on both the result type _and_ what types the database returns.
            StringBuilder columnTypes = new();
            for (var i = 0; i < dataReader.VisibleFieldCount; i++)
                columnTypes.AppendLine(dataReader.GetFieldType(i).ToString());

            return cache.Get((type, columnTypes.ToString(), mappers), () => CreateRowMapper(type, mappers, dataReader));
        }