Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ValueTupleRowMapper caching mappers too much #663

Open
asztal opened this issue May 9, 2022 · 0 comments
Open

ValueTupleRowMapper caching mappers too much #663

asztal opened this issue May 9, 2022 · 0 comments

Comments

@asztal
Copy link

asztal commented May 9, 2022

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));
        }
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant