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

Dapper v3 planning and discussion #688

Open
4 of 28 tasks
NickCraver opened this issue Jan 28, 2017 · 29 comments
Open
4 of 28 tasks

Dapper v3 planning and discussion #688

NickCraver opened this issue Jan 28, 2017 · 29 comments
Labels
area:api API Additions or Changes proposal v3.0 Changes awaiting the next breaking release
Milestone

Comments

@NickCraver
Copy link
Member

NickCraver commented Jan 28, 2017

It's time for a major release and we've just been busy with other libraries. In context though, Dapper is far less work to get there. netstandard is already supported and we don't have a long list of desired (breaking) changes to the APIs. Starting a list here to track:

API changes (breaking):

Package Changes (breaking):

Missing APIs (non-breaking):

PRs to examine:

Cleanup

Discussion - Should we add a [Column] or [Table] attributes and/or mapping methods?

Discussion in #722, please chime in!

Discussion - Should we add a DynamicParameters.Add() overload in for DB params?

See #659 for details, specifically:

DynamicParameters.Add(string name, DbType type, ...)

...okay that took a while, and it's a lot of info but I'm trying to relate all the issues I see with a common solution path. What did I forget? What's wrong? Thoughts on discussion items above (or pretty much anything of course)?

There will also be some background work I'll take on here. Like being able to more readily test Dapper & Dapper.Contrib against all providers. Luckily Docker has matured enough that having a rich test bed against most providers is finally a reality. I'm working with others in the container and cloud space to make this an option for all, but that's not a blocker by any means.

I haven't even shown this to @mgravell yet. So hopefully he doesn't kinda sorta give me a mild case of death at the next meetup. Due to how # links work...this is the best way to share it for review as well, so here it is.

@NickCraver NickCraver added the v3.0 Changes awaiting the next breaking release label Jan 28, 2017
@NickCraver NickCraver added this to the v2.0 milestone Jan 28, 2017
@supersonicclay
Copy link

I don't think 8aa10a0 alone is a breaking change since it is additive. And from the testing I've done, it is a backward-compatible solution to #471.

Would it be possible to get a new version pushed to NuGet that is simply 1.50.2 + 8aa10a0 without including everything else from master that may cause breaking changes or waiting on v2?

@aluanhaddad
Copy link

aluanhaddad commented Mar 6, 2017

Personally, I would expect to see all of the public APIs given much improved source level documentation. Specifically, all parameters should have full doc comments, consistent names, and usage examples. I frequently have to read the source code to verify that certain methods behave the way I think they might.

Even currently existing comments leave much to be desired. For example, splitOn has an awful doc comment that reads

/// <param name="splitOn">
/// The Field we should split and read the second object from (default: id)
/// </param>

Another example is the DynamicParameters constructor that takes an object parameter named template. This makes one uneasy about assuming it behaves the same way as void AddDynamicParams(object param) because... well... why is the name different?
I do not like to mutate objects after initialization and I thought passing a parameter object to the constructor might behave the same way as AddDynamicParams, but I had to read the source code to confirm this because there is not a clear naming pattern and so many methods already have arguments that are weakly typed and optional.

@paulmrozowski
Copy link

I would LOVE to have the ability to easily access the returned results metadata - things like the original field's width for strings, or scale/precision for numeric types, etc.

@unsafePtr
Copy link

unsafePtr commented Mar 20, 2017

I will leave here an idea. I think that will be very good to add support of ValueTuple<> (c# 7 feature) with named arguments:

public async Task<(int tickeTypeId, int ticketsSold, decimal price)> Test()
{
    return await context.QueryAsync<(int tickeTypeId, int ticketsSold, decimal price)>(SqlResources.GetCardSales);
}

I understood that there is some magic with ValueTuples run-time, but maybe you have an idea how to get it work.

@RobThree
Copy link

RobThree commented Jun 6, 2017

Maybe add some form of query-interception? If we want to do it now we have to implement a Dapper "wrapper" exposing all extension-methods Dapper provides and implement logging there before/after invoking the actual Dapper method.

Maybe a simple event(s) like BeforeExecute (and AfterExecute) can be raised with the actual CommandDefinition as (one of) the events BeforeExecuteEventArgs (and AfterExecuteEventArgs) argument properties.

So what I'm saying:

public class BeforeExecuteEventArgs : EventArgs {
    public CommandDefinition CommandDefinition { get; set; }
    // ... maybe more? Maybe context/IDbConnection etc.?
}

And then raise the event(s) in the Execute/Query/etc. extensionmethods.

This way logging of queries, for example, can be simplified/more easily be hooked.

@ricsmania
Copy link

About enum handlers, it would be extremely productive for us if there were a way to add a global handler. For example, most of our enums are declared like this:

enum DocumentStatus
{
    Normal = 'N',
    Canceled = 'C'        
}

And the corresponding tables are char(1) fields with the char value. The most productive way would be adding a global handler that converts the char value from the enum to the char value from the database.

@NickCraver
Copy link
Member Author

@ricsmania Can you explain that one a little bit? The enum declaration you have is equivalent to:

enum DocumentStatus
{
	Normal = 78,
	Canceled = 67
}

...would you expect an implicit char translation understanding to integer positions for the enum? Inherently, that has some problems as it could easily be mis-used to trigger enum values that aren't there. For example (DocumentStatus)320 is perfectly legal, yet practically invalid.

@ricsmania
Copy link

@NickCraver Right now what we do is we actually use a database function to explicitly convert chars to the ascii value and vice versa. But we don't want to do that conversion using database functions anymore.

What I'm suggesting is actually creating a way to add a global enum handler, so that we could create a single handler that does that converts any enum using whatever rule we define. I'm not suggesting that Dapper should convert the values, but actually that it could provide the means for us to implement our own conversion. Right now it only converts from numeric types it seems.

Also, the current version doesn't work for enum handlers, but testing the suggested solutions it seems we would need to add a handler for each specific enum, which isn't very productive.

@antgel
Copy link

antgel commented Dec 18, 2017

Hi Dapper team, it would be great to know what the forecast date is for getting v2 out of the door, if there is a forecast. There are things there that look really interesting.

@CShepartd
Copy link

CShepartd commented Feb 5, 2018

it would be great to know what the forecast date is for getting v2 out of the door

Yeah, mby a roadmap?

@CShepartd
Copy link

.Net Core and Asp.Net Core from 3.0 will support C# 8.0 with nullable reference types, Dapper from 2.0 also should support it

@MichaelTontchev
Copy link

Can we expect Dapper 2 at any point soon...er or later?

@NickCraver
Copy link
Member Author

The current plan is to start later this year.

The major items currently on our plate (read: fighting for time) are:

  • Dapper: Needs a lot of project management and code love (v2 release)
  • MiniProfiler: At v4, now live and running smooth
  • StackExchange.Redis: At v2, but we're fighting some performance and connection issues - these are affecting users unexpectedly and unpredictably in production (so: higher urgency).
  • StackExchange.Exceptional: At v2, stable and running smooth
  • protobuf-net: No work ATM I'm aware of (C# generators would change this...and Dapper for that matter)
  • Porting Stack Overflow to ASP.NET Core (and everything we actually do as part of our jobs)

Overall, my expected order here is:

  1. Wrap up StackExchange.Redis work
  2. Get Stack Overflow to .NET Core
  3. Spin up Dapper v2

The goal is later this year (at least 2H2019) for that to happen, but the list of things to do and sub-tasks is in the hundreds, so it is admittedly a best guess. I'm happy to provide updates at any time though.

@jnm2
Copy link
Contributor

jnm2 commented Apr 20, 2019

Would you be open to breaking the IMemberMap interface in v2 or introducing a new more powerful abstract MemberMap class? I'm looking at fixing #982, the issue of Dapper only setting the first seven fields of C# tuples. C# tuples encode the eighth item of a tuple as valueTuple.Rest.Item1 and the 20th item as valueTuple.Rest.Rest.Item6. Ideally, you'd put all the values on the stack and then call a series of ValueTuple constructors. The other option would be a series of ldflda for each .Rest field followed by stfld to set the final ItemX field.

There is no way to support this via IMemberMap. If a more powerful API is not introduced, ValueTuple will have to be special-cased in SqlMapper's IL generation logic.

The limitations of the IMemberMap API have been frustrating in my own code in the past as well, but I don't have any use cases on hand. We just decided not to use Dapper in those scenarios. It could be nice for Dapper users in general if we were able to map to actions other than passing a constructor parameter or setting a property or field.

@ghost
Copy link

ghost commented Jun 5, 2019

@NickCraver Are you able to provide any updates on the work for Dapper v2 since a few months ago? It looks like something I'm having issue with is on the roadmap to be addressed, so I'm just curious if any progress has been made for v2.

@mgravell mgravell mentioned this issue Jul 11, 2019
13 tasks
@NickCraver NickCraver changed the title Dapper v2 planning and discussion Dapper v3 planning and discussion Sep 21, 2019
@grahambunce
Copy link

grahambunce commented Nov 29, 2019

@NickCraver For v3 is there a piece of work to improve extensibility of Dapper.Contrib? e.g. There is no way for me to add/enhance the UpdateAsync method to enable me to add optimistic concurrency; even as simple as adding WHERE Version = @Version. The only way I've been able to do this easily is to copy/paste the entire SqlMapperExtensions and SqlMapperExtensions.Async classes into my own code and add a new UpdateOptimistic method.

Reflection would work too (to make method calls into the static class), but that introduces a performance hit which is why i'm using Dapper and not something like NHibernate that does this out of the box, and still means I need to copy/edit the UpdateAsync method as a whole.

This is because the existing UpdateAsync uses Property caches that are private objects within SqlMapperExtensions so are not available to people trying to add additional functionality for a small area.

Of course the additional of an Optimistic concurrent Update method would solve my specific problem too!

@TroySchmidt
Copy link

TroySchmidt commented Jan 6, 2020

@NickCraver @grahambunce I have been doing some work in a fork of Dommel to add a SqlQuery class. It basically allows for writing queries with predicates instead of raw SQL. This adds the benefit to leverage the mappings of columns and table names so that those naming aliases can be configured once in the mappings and then leveraged in the predicates with strong names (properties). I was thinking about contributing this back to Dapper.Contrib instead, but it seems woefully underpowered as it currently stands. It seems like for years it has been referenced that Dapper.Contrib functionality is going to be completely overhauled.

The goal is that this is used internally and then also provides a way to use fluentmap style API to write for custom relationship joins with predicates.

It also uses the Dapper SqlBuilder internally to generate the SQL.

For instance the code below is what I am writing that can then be completely translated with custom table and column names to working SQL.

 var sql = _sqlQuery
  .InnerJoin<Child>((p, c) => p.ChildId == c.Id)
  .Select<Parent>(p => new { p.Id, p.Name })
  .Select<Child>(c => new { c.Id, c.Name, c.TypeId})
  .Where(x => x.Name == "Test")
  .Where<Child>(x => x.TypeId == 5)
  .ToSql(out var parameters);

@NickCraver
Copy link
Member Author

@TroySchmidt I'd say honestly: that's getting to "a different library" territory since it's not our goal to generate complex SQL in Dapper. It's both very complicated to maintain (across DB providers), but also hard to make generic (e.g. arrays in Postgres vs. IN list in SQL vs. string split in newer SQL, etc.). It's also crazy complex when you get into nested children, applies, etc. Abstracting this away a great deal then gets both hard to implement and confusing to use past the simplest case.

Overall, it sounds like something that'd be more aimed at the https://github.com/henkmollema/Dapper-FluentMap style - would suggest proposing an issue there and seeing if they're interested in such? Otherwise: you're getting into Entity Framework territory.

We intend to overhaul the workings of Dapper.Contrib, but not largely the functionality past attribute and code mapping support (e.g. in this issue). The surface area shouldn't be unfamiliar at the end, if that make sense. It should still be low-allocation, etc.

@petterek
Copy link

Is supporting parameter classes with fields instead of properties something that could/should be considered?
Just spent 3 hours debugging an insert statement and it all boiled down to my param object used public fields instead of properties..

@jnm2
Copy link
Contributor

jnm2 commented Mar 11, 2020

A Dapper.Analyzers project could solve the usability side of that.

@TroySchmidt
Copy link

@NickCraver My purpose is to leverage Dapper (and Dommel) to translate GraphQL into SQL. Dommel is much more flexible with relationships, complex fields, and fluent mapping instead of attributes on the entities themselves like Dapper.Contrib. It would be nice to utilize only Dapper with a Dapper based GraphQL library that could provide the extensions necessary.
So is there a way to work on those type of use cases and make sure that Dapper and Dapper.Contrib have the proper improvements to support extensibility for that through other libraries like what I am interested in doing for GraphQL support?

@mgravell
Copy link
Member

mgravell commented Apr 13, 2020 via email

@TroySchmidt
Copy link

TroySchmidt commented Apr 16, 2020

Okay for instance how to implement composite keys in Dapper.Contrib? Then SqlBuilder which could be very powerful, but someone might want to extend the functionality of it with additional libraries cannot do so because it doesn't use interface programming. So there is no way to override the functionality with extended functionality for things like using the SqlBuilder nomenclature with the functionality of custom property names that Dapper.Contrib or a similar library could add.
Dapper.Contrib does finally allow for custom table name mapping, but not a similar attribute for custom column name mapping to properties. There is also people that follow separation of concerns and prefer Fluent mapping style implementation and with the attribute method that doesn't provide a way to support both.
So when rethinking what could be done with v3 I am asking about making it extensible either via interface programming or other mechanisms.

@NickCraver NickCraver added the area:api API Additions or Changes label May 4, 2020
@vitalybibikov
Copy link

vitalybibikov commented May 5, 2020

@NickCraver
It would be nice to see an ability to hydrate types with private default ctors and complex custom objects. As there are lots and lots of projects that incorporated DDD style.

Imagine a simple object: Menu
It is quite easy to solve this issue when there is only 1 type in the query.

When you have 1 to many relationship or 1 to many + (1 to 1 ), it is quite impossible to use and handle all the parameters in ctor.

image

Currently, you either need to decouple your database objects from your domain models or to write impossibly hard-to-read-and-maintain queries.

If perfromance is the only concern, can we benefit from Source Generators, that were presented in .NET 5 prev? https://devblogs.microsoft.com/dotnet/introducing-c-source-generators/

@giammin
Copy link

giammin commented Oct 29, 2020

Dapper.Contrib new attribute for automatically set a datetime when inserting or updating a row #1055

@NightWatchman
Copy link

Just a quick note about nullable reference types with the current iteration of Dapper (v2.0.78).

Using Dapper in a project, and enabling nullable reference types in that project results in false negatives for the null value checking mechanism as implemented by Microsoft.

For example the method QuerySingleorDefaultAsync<T> has the return type Task<T>. In this case Microsoft inaccurately assumes that T is never null. This causes the nullable reference type checking mechanism to break down so it cannot be relied upon.

This is a known limitation of the nullable reference type checking when not every single library used has it enabled and configured properly. Doing so in Dapper v3 will obviously fix this.

@jnm2
Copy link
Contributor

jnm2 commented Jan 26, 2021

@NightWatchman It's a little more subtle than that. C# tracks it as neither "never null" nor "maybe null" but a third state, "null-oblivious," due to the method not being declared in a context that has nullability annotations enabled. The effect is the same as what you're saying, in that you won't be warned if you dereference.

@NightWatchman
Copy link

@jnm2 That makes sense

@johandanforth
Copy link
Contributor

Dapper.Contrib support for inserting anonymous objects? Got this suggestion from a fellow dev at work today, Not sure the best way to infer the tablename though, as there obviosly is no TableAttribute. Perhaps as an overload with a table name parameter:

var id = connection.Insert(new { Name = "Anonymous", Age = 2 }, "Users");

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:api API Additions or Changes proposal v3.0 Changes awaiting the next breaking release
Projects
None yet
Development

No branches or pull requests