FeatureSupport broken when using a wrapped IDbConnection

[jhaygood86]

Our code uses a "wrapped connection" for APM tracing (inspired by mini profiler), and so this code in FeatureSupport.cs doesn't work:

public static FeatureSupport Get(IDbConnection connection) { string name = connection?.GetType().Name; if (string.Equals(name, "npgsqlconnection", StringComparison.OrdinalIgnoreCase)) return Postgres; return Default; }

Essentially, connection is a "TracingDbConnection" that wraps and proxies an underlying "NpgsqlConnection" while sending traces to DataDog (that part isn't of concern here). There doesn't appear to be a way for code outside of Dapper to "correct" this.

It looks like using MiniProfiler has the same issue: MiniProfiler/dotnet#319 (which makes sense, since our approach was inspired by MiniProfiler's)

[mgravell]

Yes, totally agreed and acknowledged; and yes: I feel this pain too (Stack Overflow uses something also related to, but not identical to, mini-profiler as a decorator layer)

ideally IMO what we need here is to move away from internal detection based on the type, and towards some kind of external configuration where the caller can influence this; that's part of the "major API overhaul" changes planned.

Note: I would have said "v2", but we're in the process of deploying v2 currently but with fairly invisible API surface changes, so in reality I think this means "v3" (this change from v2 to v3 has zero effect on timing; the current v2 is purely a semver thing)

[kieranbenton]

We have exactly the same issue, wrapping the NpgSqlConnection to add telemetry which prevents Dapper from detecting the connection as being 'array supporting'.

Whilst I 100% do not condone doing this, if you are looking to work around this issue and have Dapper treat all connections as supporting arrays GLOBALLY - then we do some awful reflection based operations on application start-up as follows:

public static void DisableArrayExpansion()
{
	var featureSupportType = Type.GetType("Dapper.FeatureSupport, Dapper", throwOnError: true)!;

	var d = featureSupportType.GetField("Default", BindingFlags.Static | BindingFlags.NonPublic)!.GetValue(null);
	var arraysField = featureSupportType.GetField("<Arrays>k__BackingField", BindingFlags.Instance | BindingFlags.NonPublic);

	bool currentArraySupport = (bool)arraysField!.GetValue(d)!;
	if (currentArraySupport)
		throw new InvalidOperationException("Dapper was already configured for use with databases that have direct array support");

	// overwrite the 'default' to be 'true' - database is aware of array types and we do not want dapper expanding them
	arraysField.SetValue(d, true);
}

YMMV - works against Dapper 2.0.78.

[mgravell]

I have been investigating options for this for V2; it is a known problem area, and isn't being ignored

…

On Mon, 4 Jan 2021, 12:03 Kieran Benton, ***@***.***> wrote: We have exactly the same issue, wrapping the NpgSqlConnection to add telemetry which prevents Dapper from detecting the connection as being 'array supporting'. Whilst I 100% do not condone doing this, if you are looking to work around this issue and have Dapper treat all connections as supporting arrays GLOBALLY - then we do some awful reflection based operations on application start-up as follows: public static void DisableArrayExpansion() { var featureSupportType = Type.GetType("Dapper.FeatureSupport, Dapper", throwOnError: true)!; var d = featureSupportType.GetField("Default", BindingFlags.Static | BindingFlags.NonPublic)!.GetValue(null); var arraysField = featureSupportType.GetField("<Arrays>k__BackingField", BindingFlags.Instance | BindingFlags.NonPublic); bool currentArraySupport = (bool)arraysField!.GetValue(d)!; if (currentArraySupport) throw new InvalidOperationException("Dapper was already configured for use with databases that have direct array support"); // overwrite the 'default' to be 'true' - database is aware of array types and we do not want dapper expanding them arraysField.SetValue(d, true); } YMMV - works against Dapper 2.0.78. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#1318 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAEHMGWS2WVN3QMYVZ54ELSYGU7VANCNFSM4IRLDZ3Q> .

[NickCraver]

We're looking at having an ambient and passable IDapperConfig or similar here to handle things...and not rely on detection since that model is fundamentally broken in these cases. Early on still, but it's being thought of!

[Voronkov-A]

Hello. Are there any updates on this?
Can't it be just some

enum ArrayHandling { Detect, Parameter, Expand }
SqlMapper.Settings {
  ArrayHandling ArrayHandling {get;set;}
}

or should it be more flexible like

interface IFeatureSupport { bool ExpandArray(IDbConnection); }
SqlMapper.Settings {
  IFeatureSupport FeatureSupport {get;set;}
}

Not sure I understand clearly the idea of IDapperConfig, but is this possible to introduce some simple solution using Settings for now?

[ian-a-anderson]

I believe the simplest workaround here is to name your wrapper class identically to the expected "NpgsqlConnection" that Dapper is looking for in the above snippet. If you are using something from a third party (EG MiniProfiler) then you would have to further wrap that with a custom class with the expected name. This is an ugly hack, but it seems to work at least in my use case. EG:

    public IDbConnection GetDbConnection()
    {
        // wrap the underlying connection in a custom class with the same name
        return new NpgsqlConnection(new Npgsql.NpgsqlConnection(databaseSettings.GetConnectionString());
    }