Dapper: Querying abstract types

Created on 29 Mar 2015  Â·  22Comments  Â·  Source: StackExchange/Dapper

[Table("Animal")]
public abstract class Animal
{
    public int AnimalId { get; set; }
    public string Discriminator { get { return GetType().Name; } }
}

[Table("Animal")]
public class Bird : Animal
{
    public string FeatherColour { get; set; }
}

[Table("Animal")]
public class Dog : Animal
{
    public string FurColour { get; set; }
}
var animals = Connection.Query<Animal>("SELECT * FROM Animal")

When doing the query above you get the exception Instances of abstract classes cannot be created. I would hope that this would return a list of animals with their values being their respective derived types.

In my case I'm using TPH inheritance so there's a field on the Animal table that specifies the derived type.

My attempts to add support for this have been unsuccessful. Before SqlMapper.cs::GetTypeDeserializer() is called if the type being passed in is an abstract class then I replace the type with the one returned in the following method:

static Type GetDerivedType(Type abstractType, IDataReader reader)
{
    var discriminator = abstractType.GetProperty("Discriminator");
    if (discriminator == null)
        throw new InvalidOperationException("Cannot create instance of abstract class " + abstractType.FullName + ". To allow dapper to map to a derived type, add a Discriminator field that stores the name of the derived type");

    return Type.GetType((string)reader["Discriminator"]);
}

However it looks like at this point the reader hasn't been opened so it fails with Invalid attempt to read when no data is present.

Any recommendations on how to proceed? If there has been an effort to implement this elsewhere, please let me know!

deserialization enhancement

Most helpful comment

Pretty sure that has been in the public API for several builds. Will check
when at PC.

On 26 Nov 2016 10:41 p.m., "verysimplenick" notifications@github.com
wrote:

Hi, any progress?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/StackExchange/dapper-dot-net/issues/262#issuecomment-263090334,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABDsEeoeiznEfksE6ItFlgB5E-QbpARks5rCLWpgaJpZM4D2udo
.

All 22 comments

The purpose of an abstract class is to share functionality but not implementation. If your example, Animal should simply be a class, since it contains all the members that you're trying to hydrate upon serialization. To put it simply: you cannot instantiate an abstract class, since it _should_ have members that just aren't implemented. For example if I say public abstract string MyProperty { get; set; } that property isn't there to hydrate...what would Dapper (or anything) do? It's the implementation of that property in inheritors where it actually exists.

If you have a set of properties that needs to be hydrated that _are_ all implemented beside abstract members, then make a normal class with those and have your abstract class inherit from it. Use that base class in your Query<T> call.

If you're expecting the Query<T> call to actually make all the various inherited types - that part just can't happen automatically. For example: what if we had 20 classes in an inheritance chain all with the properties? Which one would it be? Picking _which_ of several possibilities based on hydrated properties would be an unsolvable problem, narrows down to a "best guess" scenario, so unfortunately there's no practical way we could support that.

If none of the above applies, can you clarify further? I'll try and help you find a solution here.

Yes, I'm expecting Query<T> to make all the various inherited types. I believe this would be possible if the database was as I suggested - with a dedicated discriminator field that describes what type the row is. This is how entity framework supports this sort of query, as seen here.

I was able to hack up dapper to add this support however the changes to the public API were dumb and it had some severe performance issues such that I wasn't able to use it in production. Be warned, the branch will be seriously out of date by now and was never intended to be merged basically because I had no idea what I was doing. Maybe it will be helpful in any case.

Here's a relevant SO question that @mgravell responded to and he seemed positive that a solution was possible. I was unsure if anything had progressed since then, hence me making this issue.

I'm interested in being able to solve this, so consider this an upvote =)

Most of what I'm reading in the code seems to reflect a desire to maximize performance by getting as much information ahead of time as possible so as to prevent conditionals inside the loop as rows are getting read. Ideally if one could intercept SqlMapper.GetTypeDeserializerImpl(...) with something like this:

// ZOMG pseudocode warning! This probably won't compile
if(type != TypeICareAbout)
    return SqlMapper.GetTypeDeserializerImpl(type, reader, startBound, length, returnNullIfFirstMissing);

var deserializers = new []{
    Tuple.Create(1, TypeDeserializationCache.GetReader(typeof(Derived1), reader, startBound, length, returnNullIfFirstMissing)),
    Tuple.Create(2, TypeDeserializationCache.GetReader(typeof(Derived2), reader, startBound, length, returnNullIfFirstMissing)),
    // and so on
}.ToDictionary(x => x.Item1, x => x.Item2);

return reader => deserializers[reader.ReadInt32(anOrdinalOrColumnName)];

Then the dictionary lookup to get the concrete type would only apply per-row to those queries that need it, preserving the existing performance characteristics instead of adding a new performance tax to every row deserialized by Dapper. Just a thought.

If anyone plans to work on this, could you please post here? I'm tempted to take a crack at it, but I won't if I see that someone else is already working on it.

@floyd-may For the discriminated union case, this actually came up internally a week or so ago. I added this example following example, that used the new-ish GetRowParser method. It would be nice to make this more elegant (supported internally etc), but there are a range of things that would need to be addressed, including:

  • how are such hierarchies advertised to the engine?
  • what data types are supported?
  • single-column? multi-column?
  • does it impact the left-to-right column-access optimization that is passed to the command?
  • what does it do if there is no match?

note: that code is only in an pre-release deploy at the moment; I have a plan to make use of delegate variance to make it more elegant, so GetRowParser<Foo> gives a Func<..., Foo> - which I _think_ is fine, because a Func<..., Foo> is castable (via variance) to Func<..., object>, at least on the recent .NET frameworks. I just haven't done that bit yet. Of course, it is slightly more problematic for value-types, but...

Note: I (just now) added the GetRowParser<T> using delegate variance; seems to work great. Updated the discriminated union example too.

Here are my thoughts for addressing your questions here (switched to numbers for easy reference):

  1. how are such hierarchies advertised to the engine?
  2. what data types are supported?
  3. single-column? multi-column?
  4. does it impact the left-to-right column-access optimization that is passed to the command?
  5. what does it do if there is no match?

For 2, 3, and 5, that, in my mind, is the responsibility of whatever code gets inserted into the engine. The pseudocode I put above is obviously going to throw a KeyNotFoundException if there's not a match (item 5), and that may be sufficient for most use cases; i.e. if you want better exception messaging when there's no match, that's your job. That also covers 3, since it's the custom implementer's job to inspect the row from the reader and determine which concrete type to instantiate. Look at as many columns as you want. If you go the route of allowing an intercept of the type deserializer, that means (re: 2) the types don't need any inheritance relationship at all.

HTH.

(edited so it doesn't look like references to bugs 1,2,3,4,5 in GitHub)

I don't think I disagree, but that doesn't really do much to narrow down
what a suitable and convenient API would look like...

On Thu, 14 Jan 2016 17:06 Floyd May [email protected] wrote:

Here are my thoughts for addressing your questions here (switched to
numbers for easy reference):

  1. how are such hierarchies advertised to the engine?
  2. what data types are supported?
  3. single-column? multi-column?
  4. does it impact the left-to-right column-access optimization that is
    passed to the command?
  5. what does it do if there is no match?

For 2, 3, and 5, that, in my mind, is the responsibility of whatever code
gets inserted into the engine. The pseudocode I put above is obviously
going to throw a KeyNotFoundException if there's not a match (item 5),
and that may be sufficient for most use cases; i.e. if you want better
exception messaging when there's no match, that's your job. That also
covers #3 https://github.com/StackExchange/dapper-dot-net/pull/3, since
it's the custom implementer's job to inspect the row from the reader and
determine which concrete type to instantiate. Look at as many columns as
you want. If you go the route of allowing an intercept of the type
deserializer, that means (re: #2
https://github.com/StackExchange/dapper-dot-net/pull/2) the types don't
need any inheritance relationship at all.

HTH.

—
Reply to this email directly or view it on GitHub
https://github.com/StackExchange/dapper-dot-net/issues/262#issuecomment-171706023
.

From your earlier comments, it sounds like the GetTypeDeserializer(...) method is going to get replaced with something called GetRowParser(...) and returns a strongly-typed Func<IDataReader, T> rather than the existing Func<IDataReader, object>. In my mind, there are three major components:

  1. Identifying what type's default serialization you want to override. I could see that being done similarly to how ITypeHandler implementations are registered (a public static method on SqlMapper, for instance).
  2. How to specify your custom code. Maybe something like:
public interface IRowParser<T>
{
    Func<IDataReader, T> GetRowParser(IDataReader reader, int startBound = 0, int length = -1, bool returnNullIfFirstMissing = false);
}

Just brainstorming here...

  1. Access to the cache that contains and lazily builds IRowParser<T>s, which is currently private. Maybe another parameter in IRowParser<T>.GetRowParser(...)? I'm not particularly happy with the interface name IRowParser<T> but hopefully this helps? Not sure if this has wrinkles in it for value vs. reference type.

Love to hear your input if you think this is on the right track. I'm looking forward to seeing the strongly-typed GetRowParser(...) code you mentioned earlier.

Really appreciate your work on Dapper, @mgravell!

The GetRowParser method is simply some plumbing code I've been meaning to expose for ages. It isn't specifically related to this scenario, but if it helps: _great_. The Func<IDataReader, T> is purely a convenience thing. Keep in mind that for reference-type T, this is fully compatible with Func<IDataReader, object>, thanks to delegate variance. I confess I'm not quite sure what IRowParser<T> is adding to things; what is the intent of that interface? I'm much more interested in discussing the consumer API than the implementation details - basically, how the 1 works. In my mind, the key things any API _there_ needs to do is something like

  • accept an IDataReader
  • return a Type

or

  • specify a column name (or names?) - presumably by a string get property
  • accept a value (or values?)
  • return a Type

For example:

class EventTypeDiscriminator : IDiscriminator {
    public string ColumnName { get { return "EventTypeId"; } }
    public Type ResolveType(object value) {
        switch((int)value) {
            case 1: return typeof(CaseOpened);
            case 3: return typeof(CaseClosed);
            default: return null;
        }
    }
}

(or possibly an IDiscriminator<T>, but that seems unnecessarily awkward for the library code)

With some kind of:

SqlMapper.RegisterDiscriminator(typeof(BaseType), new EventTypeDiscriminator());

one problem with that approach is when there is no shared hierarchy - clearly we don't want to register object for discriminated unions at the global level; that would have to be at the Query level.

I see what you mean. I like the way that IDiscriminator looks. I think what I had going was overly-complex.

As for shared hierarchy, though.... In my mind, registering an IDiscriminator for object would only apply when someone does, say, connection.Query<object>(...) (or similar). A strict type equality check seems like the right approach (rather than an .IsAssignableFrom(...) or similar) when trying to determine if a discriminator is present. For instance, given these classes:

abstract class A {}

abstract class B : A {}

class C : A {}

class D : B {}

class E : B {}

Let's say we registered an IDiscriminator for type B. I would expect Query<B>(...) to use that discriminator; however, I would expect Query<A>(...) to complain and _not_ use the discriminator registered for B, even though, technically, that would be type-safe.

I would also expect, given an IDiscriminator registered for A, calling Query<B>(...) would also complain.

Is exposing GetRowParser method confirmed to be available in the next release? (1.50)

BTW, Dapper is great, thanks! :-)

Hi, any progress?

Pretty sure that has been in the public API for several builds. Will check
when at PC.

On 26 Nov 2016 10:41 p.m., "verysimplenick" notifications@github.com
wrote:

Hi, any progress?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/StackExchange/dapper-dot-net/issues/262#issuecomment-263090334,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABDsEeoeiznEfksE6ItFlgB5E-QbpARks5rCLWpgaJpZM4D2udo
.

@NickCraver, @mgravell many thanks!
Maybe need add this in docs?

It made it into the release notes, at least:
http://stackexchange.github.io/dapper-dot-net/

On 28 Nov 2016 7:15 a.m., "verysimplenick" notifications@github.com wrote:

[image: Found test]
https://github.com/StackExchange/dapper-dot-net/blob/61e965eed900355e0dbd27771d6469248d798293/Dapper.Tests/Tests.IDataReader.cs
Many thanks!
Maybe need add this in docs?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/StackExchange/dapper-dot-net/issues/262#issuecomment-263199237,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABDsCg6Q-2riNnF12UBF46jg_s2iSU6ks5rCn-TgaJpZM4D2udo
.

I need multimapping with "Discriminated" RowParser and seem that you release this functionality, but don't found test for this. Please see #646

@verysimplenick merged, thanks

Thanks for the work done on exposing GetRowParser - one step closer!

Has any more thought gone in to how the consumer API for this might look? I really like the IDiscriminator (IRowParser?) approach suggested by @mgravell. I also agree with @floyd-may that I would expect a IDiscriminator to be used only when one is registered for the type being queried (ie. strict equality check).

Was this page helpful?
0 / 5 - 0 ratings

Related issues

huobazi picture huobazi  Â·  4Comments

valinoment picture valinoment  Â·  4Comments

CrescentFresh picture CrescentFresh  Â·  4Comments

PeterWone picture PeterWone  Â·  5Comments

fishjimi picture fishjimi  Â·  5Comments