Open NickCraver opened 7 years ago
Attributes Vs Config...
I really think Dapper should only support 1 convention, even if there's a group of people who don't like Attributes or prefer attributes and don't like Config.
Dapper is a beautiful tiny lightweight ORM that does the bare minimum. Yes it needs some mapping to get around legacy and poorly named tables/columns in databases, and vice versa. But not at the sacrifice of bloating Dapper.
/my 2 cents
I'm impartial to Attributes vs Config. As long as Dapper doesn't lose focus and become bloat.
I appreciate your attention to this @NickCraver! Hopefully there is a good solution to cover the vast majority of issues/preferences.
I highly prefer config (not attributes) based setup because I may not own the class/model, the other big ones are conventions. Working with legacy schema (I can't change) where all the tables are prefixed like tb_TableName
or the columns are chrColumnName
🤢
I prefer my database identity columns to be in the form of TableNameID
, and my c# classes as just ID
. I would like the flexibility to setup a convention to map ID
to [ClassName]Id
and just reuse it for all of my queries and stored procs.
Thanks!
I hope that the Dapper is supported in two ways, and Attribute is simple and intuitive. But I think Dapper in complex scenes require enhanced Mapper function, especially in the one-to-many, many-to-many scenario, EF-like configuration method based on Lambda expression expression ability.
A suggestion for the Attributes vs Config discussion:
Go with the Config option, and supply an implementation (in Dapper.Contrib
?) which would do attribute mapping. That way by default Dapper would remain small, but if Attribute mapping is required, add a reference to Dapper.Contrib
. You could even supply a .UseAttributeMapping
extension which would apply all of the Func<Type...>
required in one go if needs be.
Personal bias is towards Config rather than Attributes, but I could live with either. Config feels more flexible however.
+1 for @Pondidum. The Config is a must for the Dapper
Core, the Attributes are for Dapper.Contrib
only (optional). Since I use Dapper.Contrib
I will use the Attributes - and I would like to have an option to change/enhance the Attributes by the Config.
Just a minor note: you should decide, what will happen, when TableNameMapper
, ColumnNameMapper
return null
or empty string - either fall back to the default mappers or throw an exception.
@Gonkers another option there is a virtual method on the attribute and subclass to change default behavior; worth consideration
I'm seeing a lot of surprising replies, that's awesome. Let me make sure we're on the same page when discussing first:
The proposal isn't for an either/or, it's for both. But they wouldn't be 2 separate approaches, they work together. For example, SqlMapper. DefaultTableNameMapper
would look for the [Table]
attribute and use it if present, otherwise it'd do what we do today. The same holds for SqlMapper. DefaultColumnNameMapper
, it'd look for ColumnAttribute
if present, and default to the current behavior if not.
You'd free to use the attributes or not. You'd be free to use none of this and get the exact same behavior we have today.
Is that what everyone understood from the proposal? If not I'll do some editing to clarify.
@phillip-haydon I'm not sure I understand the bloat comments here - what we're talking about is very minimal and removes several one-off options both in the project and proposed today - they could instead be included mappers...or the other mappers could be in another project, or something else. Can you explain a bit more? Or did I botch mentioning the totally optional part?
@Gonkers same thing - did you read this as either/or - or would you only want to see 1 for some reason?
@uliian I don't think we'll be tackling 1:many here, that's a very different generation and consumption problem, likely in Dapper.Contrib for the foreseeable future...but all together another feature entirely.
@Pondidum Same question - was the either/or and not both confusing, or does your opinion stay the same regardless?
@xmedeko Both are optional...same question, confusing on the proposal?
It looks like I need to edit the copy to clarify some things, will wait for responses there. Also related proposal: I think we should move all of these static settings to another place, probably:
namespace Dapper
{
public static class Options
{
public Func<Type, string> TableNameMapper { get; set; }
public Func<Type, string> DefaultTableNameMapper { get; }
public Func<Type, PropertyInfo, string> ColumnNameMapper { get; set; }
public Func<Type, PropertyInfo, string> DefaultColumnNameMapper { get; }
}
}
...along with all other options in V2, so they're contained in an intuitive spot. This would make access more intuitive, like this:
Dapper.Options.TableNameMapper = ...;
Thoughts?
Yep, we are on the same page. Yes, all is optional for the user. By optional I was thinking something else, never mind about it.
You can make just:
public Func<Type, string> TableNameMapper { get; set; }
public Func<Type, PropertyInfo, string> ColumnNameMapper { get; set; }
And the user should do the mapper chaining:
var origMapper = Options.TableNameMapper;
Options.TableNameMapper = (t, s) => {
if (type == typeof(User)) return "Users";
return origMapper(type);
}
The Caliburn.Micro
project does config the same way.
Dapper.Contrib
may just plug in own mappers to provide Attribute mappings.
@nickcraver - For conn.Query
@xmedeko I think so many people have asked for this type of mapping, it simply no longer belongs in .Contrib
, it's something we have to take on and maintain. It also means it can work by default, which is easier for the user. Without that, I think we get a lot of people that forget Options.TableNameMapper = ....
and Options.ColumnNameMapper = ...
and wonder why their attributes aren't working everywhere.
It also means core things like type mapping conversions could never be on those attributes (e.g. handling GUID <> byte[]
) and such across providers. There so much extensibility we can do if core is aware of these. That's the rationale for splitting them into .Contrib
, given they'd be completely optional to use?
@philn5d That's done via the generic args on the multi-map overloads, e.g. .Query<Blog, Post, Comment, User, Blog>()
, so I'm not sure where string names would come into play, can you clarify a bit? Or do we just suck at making people aware the multi-map functionality exists? I want to do some Jekyll docs with v2, so please, let us have it on where the docs suck.
I think I misunderstood the 1/both/many part - suggested implementation so they work together is fine. I would only be marginally concerned with bloat, but it seems like it'll be pretty minor either way.
I just updated the issue to clarify the confusing points, hopefully a bit better for getting on the same page.
@NickCraver I don't think using Options as static is a good thing.
Someone might need to connect to two different databases at the same time using two different mapping schemes, with static that would require using AppDomains.
Examples:
@Flash3001 - What's the alternative, passing the options as an overload to every single mapper? Note this affects the deserializer cache, etc. Not making them static (as all options are today) would be a huge burden to take on, for a very small subset of cases. As an example, none of the examples you're mentioning would be practically supported today, since type mappers and such are global/static today.
Unless there's a way I can't think of to share this without passing it around everywhere, forking the cache, etc. - I just can't see us supporting those use cases in Dapper, just as we don't really today.
I really like it. I've been using other libraries on top of Dapper that provide column mapping, and this is a much cleaner solution.
I love the decision to implement the attributes using the config, keeping everything flexible and extensible, and up to the user to choose how they want to use it.
As for your question about controlling other things from attributes... I personally like the idea of defining the keys via attributes. This lets me keep all my table-level configuration in one place.
@NickCraver I haven't seen the big picture. All alternatives I could think of after your explanation would do more harm than good.
@nickcraver that's on me - wasn't thinking clearly about that.
I really like the attributes, they are unsurprising and smooth and cover the 95% use case quite nicely. Not so much the Options
class. Here are two reasons for it and some alternatives.
TableNameMapper
and a DefaultTableNameMapper
. I understand what you are trying to achieve, but I don't think it's a clean solution because it sort-of forces the client to use methods in-order. A possible alternative that would get us most of the way there would be to have only one set and a MappingBehavior
enum telling Dapper whether to chain the default behavior on null or empty response from the Func
s. public static class Options
{
public Func<Type, string> TableNameMapper { get; set; }
public Func<Type, PropertyInfo, string> ColumnNameMapper { get; set; }
public MappingBehavior Behavior { get; set; }
}
Options
being set with the current proposal. We could solve this by treating Options
a bit like an event handlerOptions.AddMapping(Func<Type, string> mapping);
Options.AddMapping(Func<Type, PropertyInfo, string> mapping);
Dapper would run all the user mappings last to first until one returns a non-empty, non-null string. It would eventually run the default mapper as a last option. If there are no plugins I think this would have a minimal perf impact, but of course it's O(N) on the number of plugins.
All in all, great job, and a good step forward @NickCraver & @mgravell
@NickCraver Sorry I misunderstood. I'm all for both optional config and attribute.
@sklivvz an event handler model is interesting, but ultimately there's a fundamental race to attach rather than explicit control. For example: " it sort-of forces the client to use methods in-order" ...that's true with either approach 1 or 2. But with a setter, you get explicit control.
If the defaults are confusing, what if we put the default behaviors all in their own namespace? Like this:
namespace Dapper
{
public static class Options
{
public Func<Type, string> TableNameMapper { get; set; }
public Func<Type, PropertyInfo, string> ColumnNameMapper { get; set; }
public static class Defaults
{
public Func<Type, string> TableNameMapper { get; }
public Func<Type, PropertyInfo, string> ColumnNameMapper { get; }
}
}
}
Note that Dapper has many more options - this proposes moving them all under the same scheme to be consistent. Users wouldn't need to type SqlMapper
for almost all use cases, which is also more consistent with the extension nature of Dapper. The defaults don't have to be exposed at all, we could force people to get
then set
- but I think that's not very user friendly, so IMO they should be available.
Just want to throw this out there if you had not seen it before... https://github.com/henkmollema/Dapper-FluentMap I have used it before and it was pretty good for many of my needs.
@NickCraver @Flash3001 note you can go without static Options
- you may create some kind of Context
or Session
and do session.Query(....)
. E.g. that's what Java Hibernate does. AFAIK it goes beyond the scope of this issue, and maybe is not welcome for simple ORM like Dapper is.
@Gonkers Yep, if that style suits you then by all means. In terms of performance it's hard to do that without allocations and analysis overhead at many points along the way, so an awesome task for a separate library to tackle.
@xmedeko That brings a big dependency injection dependency ;) I'd definitely say not something that's in the goals of Dapper, IMO.
I'm using Dapper exclusively (for the moment) on .NET Core projects, and at least for one of those projects, I haven't decided between PostgreSQL and MSSQL for production--the semantics may be slightly different for each one.
So, my preference would be a configuration object (which may or may not utilize attributes if available) that I can instantiate and inject. IMHO, the configuration object should be responsible for falling back to the default behavior. This would, for example, make it child's play to obtain the default value and then optionally decorate it -- adding a table prefix, lowercasing, etc. -- or to "disable" a field so it isn't read.
Some minor thoughts:
If the configuration responds with null/empty for a given property, Dapper should ignore the property.
The configuration object may need access to the POCO, not just its type, to determine the correct field name.
If Insert and Update are planned for the future (I'm using SimpleCRUD now, so I hope this is the case!), the interface for obtaining the table/field name should include a flag for the action at hand. For example, a dev may need to read from a view but insert/update against a table, or certain fields may be computed or otherwise managed in the background for reading but should not be part of an INSERT/UPDATE.
10+ years ago, I had my own .NET ORM that used attributes for the field name and a few other odds and ends (default value for null, max length for string fields, etc.). Back then there was a performance hit for enumerating the attributes, enough so that I believe I ended up caching them. Is that still an issue? (If so, the default configuration implementation could cache them. Maybe JSON.NET would be a good library to check to see if they've had to work around any perf issues with attributes.)
@richardtallent thanks for chiming in, follow-ups with some more context:
If the configuration responds with null/empty for a given property, Dapper should ignore the property.
I'm not sure exactly how this will work in both directions, but yes - probably this behavior.
The configuration object may need access to the POCO, not just its type, to determine the correct field name.
This won't be possible, Dapper only figures out these bits once and caches the (de)serializer, via writing IL. So it's not per-object...we can't do that in a performant way. This functionality is what makes Dapper so fast.
If Insert and Update are planned for the future...
I think these will remain in .Contrib, and will be able to access fields, but differing view vs. table read/writes, etc. likely will never be a target use case. If it's something that works then great, but I think it'd be hard to justify complication for them.
Back then there was a performance hit for enumerating the attributes
See point 2 above - this isn't a tremendous concern since we cache the deserializer (and we'd cache this mapping, mental note: clearing it on a Func<>
set). We want it to be fast, but there is an overall limited impact given the nature of Dapper here. @mgravell also maintains protobuf-net, which we can steal a few tricks from here.
@NickCraver - no you didn't botch anything. I'm just giving my opinion :) my concern is ok so you're adding support for configuring mismatch between classes and database naming. But you're adding support for 2 groups of people. Those for attributes, and those against attributes. So what happens in the future when another group of people want another feature. I'm worried that over time all these 'little' things will just begin to bloat and slow Dapper.
I just hope I can still have a toggle for supporting underscores, since I use PostgreSQL, it's nice to be able to just toggle that on and be done with it.
@NickCraver just about non-static Options
(Context
, Session
). It does not mean big dependency injection dependency for everyone, just for those, who need to use more contexts. For the most of the users there may be a static default instance, so they may use Default.Options
and they may keep using connection.Query()
since the Query would use the Default
context. Anyway, IMO it a little bit off-topic. I think @Flash3001 should create a new issue, if he needs this feature.
@NickCraver I would love this set up. It captures the possibility to map everything using your own conventions in one function. Just the way my PR https://github.com/StackExchange/Dapper/pull/653 was about.
This could also return a null as an alternative for the [Ignore] attribute. Is that a possibility also?
Agree with the approach - flexible to allow anyone to implement their own conventions.
@phillip-haydon : you talked about toggling a postgres convention. I think a standard set of conventions should be made available as part of the official project... however not in the core Dapper library.
for example: a Dapper.PostgresConventions library that allows easy application of this convention. Similarly, a Dapper.AttributeMapping library .. etc etc.
A bit late to the discussion, but I thought I'd chime in anyway since we extensively use both Dapper and Dapper.Contrib
Majority of our Dapper projects connects to a PostgreSQL database so the column sensitivity when using Dapper.Contrib was an issue. For this purpose, we forked Dapper.Contrib and disabled quoting by default.
The upcoming changes are exciting and feels like a great step forward.
I'm +1 on the configuration/convention and attributes change and bringing [Table]
and [Column]
to Core Dapper.
One prime example here is [Column(PrimaryKey = true)] (of which there could be many), and possibly some others along those lines.
@NickCraver: Could you please elaborate where something like this would be useful in Dapper Core? I can see its use more in Contrib for Get
, Update
but a little hazy on where Dapper Core can make use of it.
for example: a Dapper.PostgresConventions library that allows easy application of this convention
I really like how the approach would allow for things like this. :+1:
Additionally, I think that we have to spend some extra time with documentation. I remember recently having to probe the code base and look around on StackOverflow (tsk tsk) for quite a while before I could understand how to express the JsonB
type in C# for me to use it via Dapper.
I'm all up for a documentation overhaul and would be more than happy to contribute to it. Have in the past as well.
Thanks for the efforts @NickCraver and @mgravell :tada:
I saw some early commits where we defined the Table & Column attributes within Dapper. Any reason we shouldn't reuse the TableAttribute & ColumnAttributes types from System.Componentmodel.DataAnnotations?
I believe they don't want to take the dependency.
It would be nice to be able to use either. I believe they use a trick for [key], where they only look by name rather than type so you can use the KeyAttribute from DataAnnotations or Dapper.
My 0,02€: I think supporting both our own attributes and DataAnnotations would make sense. However, it might mean that Dapper2 has more dependencies if it doesn't already depend on System.Componentmodel
from beforehand, which might be undesirable if we want to keep the dependency footprint down.
Just discovered the issue where GetAsync isn't escaping the identity column name for Postgres. And yes also discovered the lack of the ColumnAttribute to specify a different column name. So I changed the column names to fit but to resolve GetAsync is the solution to download and build a custom version that includes that functionality and wait for it to be included in version 2?
Please could you add tableName as an optional parameter to the Dapper.Contrib methods please as per https://github.com/StackExchange/Dapper/issues/797.
This would allow the reuse of a single model for multiple tables. At the moment it is necessary to create a model just to add the [Table] attribute for models that have identical structure,
Examples:
Connection.GetAll<Person>("Employees"); // Gets list of Persons (People) from the Employees table.
Connection.Get<Person>(id, "Employees"); // Gets one specific Person from the Employees table based on id
Connection.Insert(person, "Employees") // Inserts a person into the Employees table
Hi, I'm very new to Dapper & Contrib - only been looking at it the last couple of days.
Any eta on when v2 might be out?
For [Table()] attribute I noticed I can't put it on the Interface - I was trying to do the dirty tracking on contrib, but the name of the interface (IUser) is completely different from the table name in the database (existing db)
Other attributes that might be of use / interest a) [MaxLength(x)] - for strings / parameters b) [Required] - fails if value is null
Contib - I've noticed it uses the [key] field when getting, might be nice to specify a field - e.g. Invoice details might contain DetailID (identity), InvoiceID (FK), I want to get all where invoiceID = x -- Yes I know I can use the .Query with a sql to get this, but a GetAllByField("fieldname", value) might be useful in contrib (sorry if off topic on this one).
Thanks
From @colinbreame commented on Feb 11 in #698
A table is created in Postgres like this:
CREATE TABLE Metric (
"MetricId" SERIAL,
"Name" VARCHAR(255),
"CategoryId" INT,
"Type" VARCHAR(255),
"Text" TEXT,
PRIMARY KEY (MetricId)
)
When inserting the following SQL is created:
insert into Metric ("Name", "CategoryId", "Type", "Text") values ($1, $2, $3, $4) RETURNING MetricId
However in Postgres the columns names that are not escaped are lower cased. So the column name MetricId does not equal "MetricId". As my property name is "MetricId" this is causing an issue, as it is referred to inconsistently.
The suggestion solutions (as applied to PostgresAdapter) would be to escape the primary key column name.
I very much would like a column attribute, we already have a table and key attribute, I don't know why a column one was never made. If you don't want it in the core library, I don't see why it couldn't be with the contribute library.
This might be a new issue for now or part of v2...
How to ignore the specific columns when I use generic methods (Get
[Write(true/false)]
- this property is (not) writeable
[Computed]
- this property is computed and should not be part of updates
I've tried both attributes. Not working as I want. Write
attribute is always ignored. Computed
also it is.
In this case:
I want to use generic methods for insert, update, delete or select. But in same case I don't want to be part of update, insert, delete or select queries of the specific columns.
For example: This is my database table.
[Table("Stuff")]
public class Stuff
{
[Key]
public long Id { get; set; }
public string Name { get; set; }
[Write(false)]
public string Ignored { get; set; } // there is no this column in table. ok.
public int InsertedBy { get; set; } // this column can use only on insert and select
public DateTime InsertedOn { get; set; } // this column can use only on insert and select
public int? UpdatedBy { get; set; } // this column can use only on update and select
public DateTime? UpdatedOn { get; set; } // this column can use only on update and select
// I don't want to hard delete the data so:
public int? DeletedBy { get; set; } // this column can use only on delete and select
public DateTime? DeletedOn { get; set; } // this column can use only on delete and select
public bool? IsDeleted { get; set; } // this column can use only on delete and select
}
INSERT
var id = connection.Insert(new Stuff
{
Name = "Insert",
Ignored = "Ignored",
UpdatedBy = 1,
UpdatedOn = DateTime.Now,
InsertedBy = 1,
InsertedOn = DateTime.Now
});
DEFAULT INSERT QUERY:
insert into Stuff ([Name], [InsertedBy], [InsertedOn], [UpdatedBy], [UpdatedOn], [DeletedBy], [DeletedOn], [IsDeleted]) values (...);select SCOPE_IDENTITY() id
EXPECTED INSERT QUERY:
insert into Stuff ([Name], [InsertedBy], [InsertedOn]) values (...);select SCOPE_IDENTITY() id
UPDATE
connection.Update(new Stuff
{
Id = id,
Name = "Update",
Ignored = "Ignored",
UpdatedBy = 1,
UpdatedOn = DateTime.Now,
InsertedBy = 1,
InsertedOn = DateTime.Now
});
DEFAULT UPDATE QUERY:
update Stuff set [Name] = @Name, [InsertedBy] = @InsertedBy, [InsertedOn] = @InsertedOn, [UpdatedBy] = @UpdatedBy, [UpdatedOn] = @UpdatedOn, [DeletedBy] = @DeletedBy, [DeletedOn] = @DeletedOn, [IsDeleted] = @IsDeleted where [Id] = @Id
EXPECTED UPDATE QUERY:
update Stuff set [Name] = @Name, [UpdatedBy] = @UpdatedBy, [UpdatedOn] = @UpdatedOn where [Id] = @Id
I don't want to write custom query for every entity like this. How to do with generic methods that.
I thought add a new attribute to ignore in generic method for columns like[IgnoreInsert], [IgnoreUpdate], [IgnoreDelete], [IgnoreSelect]
.
What could be the best way?
I'm not sure I get the issue here. If you only eant to update certain columns either write the SQL for each INSERT/UPSATE etc or use the Dapper.Contrib extension methods and mark properties as [Computed]. Am i missing something?
@Caltor I put the [Computed] to UpdatedBy and @UpdatedOn columns.
[Table("Stuff")]
public class Stuff
{
[Key]
public long Id { get; set; }
public string Name { get; set; }
[Write(false)]
public string Ignored { get; set; } // there is no this column in table. ok.
public int InsertedBy { get; set; } // this column can use only on insert and select
public DateTime InsertedOn { get; set; } // this column can use only on insert and select
[Computed]
public int? UpdatedBy { get; set; } // this column can use only on update and select
[Computed]
public DateTime? UpdatedOn { get; set; } // this column can use only on update and select
// I don't want to hard delete the data so:
public int? DeletedBy { get; set; } // this column can use only on delete and select
public DateTime? DeletedOn { get; set; } // this column can use only on delete and select
public bool? IsDeleted { get; set; } // this column can use only on delete and select
}
But...
var id = connection.Insert(new Stuff
{
Name = "Insert",
Ignored = "Ignored",
InsertedBy = 1,
InsertedOn = DateTime.Now
});
In this query @UpdatedBy, @UpdatedOn columns are not in query. It's OK.
insert into Stuff ( [Name], [InsertedBy], [InsertedOn], [DeletedBy], // this column is nullable, ok [DeletedOn], // this column is nullable, ok [IsDeleted] // this column is nullable, ok ) values (@Name, @InsertedBy, @InsertedOn, @DeletedBy, @DeletedOn, @IsDeleted); select SCOPE_IDENTITY() id
connection.Update(new Stuff
{
Id = id,
Name = "Update",
Ignored = "Ignored",
UpdatedBy = 1,
UpdatedOn = DateTime.Now,
});
In this query @InsertedBy, @InsertedOn (also @DeletedBy, @DeletedOn, @IsDeleted) columns could not be in here. Cause @InsertedBy and @InsertedOn only can use on insert. Also @UpdatedBy and @UpdatedOn columns should be in here.
update Stuff set
[Name] = @Name, [InsertedBy] = @InsertedBy, [InsertedOn] = @InsertedOn, [DeletedBy] = @DeletedBy, [DeletedOn] = @DeletedOn, [IsDeleted] = @IsDeleted where [Id] = @Id
As a result, I am getting an error because @InsertedBy and @InsertedOn is null (in database is not null).
Message "SqlDateTime overflow. Must be between 1/1/1753 12:00:00 AM and 12/31/9999 11:59:59 PM." string
I think I get you now. You don't want to update the inserted and deleted columns on Update. Forget the computed attribute cos that says you never want to write that property to the database. Either handcraft your SQL update query to ignore those columns or pull the current values from the database/model first and overwrite them with the same value. HTH
@Caltor thanks. But that is not solve my problem. I've already thought that but I dont want to use this way. I want to update directly for specific columns.
So I do have to contribute to this discussion here. I think there are essentially 2 camps:
To be honest it's beyond frustrating that Contrib ignores DefaultTypeMap.MatchNamesWithUnderscores
for these simple CRUD operations and that we've removed the ability to specify a Custom type mapper!.
When building a PostGres DB we have to choose between hideously adding extra parentheses to all our handmade queries for case sensitivity or building our own flavor of the repo pattern with potentially expensive custom mappers to save hundreds of lines of code. Or, we could just do [Column("")]
if this feature was supported.
I think it's okay to lock this mapping capability into .contrib so that if you're obsessed with pounding out boilerplate CRUD code you can use the vanilla dapper package. The advantage of using the existing DataAnnotations.Schema
column and table attributes in .NetCore are potentially huge. It means Dapper becomes a lot more Snap-In with already built EF entities, and the use of a config file honestly sounds like a nightmare compared to being able to manage each entity individually.
If contrib includes its own attributes for people who don't want to rely on DataAnnotations (a great idea!) I assume they'd be in separate namespace ( Dapper.Contrib.Attributes)?
Wondering if there is an elegant workaround for above limitation, specifically I'm using Dapper with PostgreSQL & npgsql driver?
The tables and fields are lower case in PostgreSQL with _ so I've used SetTypeMap to tell Dapper how to map results from PostgreSQL to C# objects which are Title cased. Mapping results works fine.
For example:
[Table("product_category")]
public class ProductCategory
{
[Key]
[Column("id")]
public long Id { get; set; }
[Column("category")]
public string Category { get; set;
The issue arises though when I wish to use Dapper.Contrib (Repository pattern)
So it appears for mapping results from PostgreSQL to C# Dapper is fine, but when I wish to insert data into PostgreSQL Dapper is still using the Property name of the class instead of the mapping, in this case Category as opposed to [Column("category")].
For the mapping from PostgreSQL I use:
SqlMapper.SetTypeMap(typeof(ProductCategory), new CustomPropertyTypeMap(typeof(ProductCategory), (type, columnName) => type.GetProperties()
.FirstOrDefault(prop => prop.GetCustomAttributes(false).OfType<ColumnAttribute>()
.Any(attr => attr.Name.Equals(columnName, StringComparison.OrdinalIgnoreCase)))));
Thx.
Please, make GetTableName method public, so it would be possible to get a name of a table as the sum of all the applied table-name-mappers and there will be no need to use reflection like this: https://stackoverflow.com/questions/43032797/get-table-name-from-tableattribute-dapper-contrib/49347600#49347600
I create SQL-queries this way in order to ensure their consistency when auto-renaming entity classes or their properties:
string sql = $@"
SELECT *
FROM {_TableName_<PostTag>()} AS pt
JOIN {_TableName_<PostTagIndex>()} AS pti
ON pt.[{nameof(PostTag.Id)}] = pti.[{nameof(PostTagIndex.PostTagId)}]
WHERE (pt.[{nameof(PostTag.Level)}] = @{nameof(tagLevel)} AND pt.[{nameof(PostTag.Alias)}] IN @{nameof(tagAliases)}) ";
That's why I have to use the GetTableName
method, which is private now.
The same wishes for the future GetColumnName
method - let it be public too.
@DevelAx I agree infact I already had patched both to allow tablename and column name in: https://github.com/mitchcapper/dapper-dot-net/compare/master...mitchcapper:additional_accessibility_helpers?w=1
allowing for getting the table name or with the larger helper even something like: $"{db.GetSelectSql<Comment>()} WHERE ..."
I would very much benefit from this feature. What is the timeframe for the v2 release? It's been almost a year and a half since the Dapper v2 planning and discussion #688 thread. I'll probably end up just compiling and using the changes in my pull request until v2 is released. For my personal use they won't pose a sudden behavior change in the way I'm using the ColumnAttributes with my properties. Thanks.
I feel the default mappings need to ensure that the database provider tokens for wrapping table names and column names is adhered to - this does mean that there needs to be a default per provider. Mainly for tables that have reserved-word names.
Dapper.Contrib pluralisation is just adding an 's' to the type name, from memory. This needs to be more accurate for the OCD out there.
And just spitballing, but instead of exposing a single Func<Type, string> mapper, you could have a class that handles the addition of multiple maps:
interface ITableMappings
{
ITableMappings AttachMap<T>(Func<T, string> map);
}
The (default) implementation would store a collection (dictionary?) indexed on Type, and when a key is not found, the default mapper is used. The default mapper could also be set, to handle specific, yet more generic, convention.
I'm not sure this much control is really needed, but for very little overhead it can cater to many different scenarios.
I wonder if, as @Caltor suggested as above, an optional parameter in the Connection extensions could be an ITableMappings instead of a string for a table name This would allow the use of an override to handle cases where you need different contexts for different database types. Users could implement their own contexts/repositories and inject the appropriate ITableMappings implementation.
I've addressed much of this through Components Attributes in my Dapper.Database project.
This would be part of the overall Dapper v2 project (#688)
There's obviously been a lot of interest in controlling column and table name mappings in a coherent and fully featured way. But all of the PRs we've seen are all over the map as to how to go about this. They're also Dapper.Contrib restricted, whereas we need something to cover things overall. For example calling
.Insert<T>()
and then calling.Query<T>
afterwards and that not working isn't expected or awesome.Let me preface this. Both of these are optional. They work together, but are 100% optional - if you use neither then Dapper's existing behavior works as-is with mappings. The proposal is also for both, not an either/or. See below for how they work together:
Basically, this can't be
Dapper.Contrib
, it needs to be global. This type, this member goes to this table, this column...in a controllable way. I'm picturing an API like this:Now, this doesn't work for everyone. You can't add attributes everywhere and some people don't want to. They want conventions, or covering types they don't control - so we need something else. I'm not sure what these are called, but basically we make the type and member to table and column mappings pluggable. For example, something like this:
So if your convention is init caps, underscores, emoji, or whatever really, you could define this in one place. You could call the default mapper as a fallback in your own function as well, if you just want to handle a few cases and fall back for the rest. Or, you could make your own attributes, and entirely base all mappings on whatever scheme you could come up with. As a simple example:
Note: these work together, the
Options.DefaultTableNameMapper
andOptions.DefaultColumnNameMapper
would look at the attributes in their implementation. So this isn't an either/or proposal, the two approaches collaborate to handle all the cases we've seen thus far, while being totally optional all together.All of these would apply across the board, to
.Query<T>
,.Insert<T, TKey>
,.Update<T>
,.Get<T>
, and...well, you get the idea. They need to work everywhere, that's why it's needs to be in Dapper core and consumed all the way through.Contrib
, etc.Now, that deals with naming (I hope) - but there's more we can do here too. For example, is there interest in controlling defaults or something else in attributes? Naming is the biggest thing I've seen by far, but there may be some other areas we can solve along the same lines.
One prime example here is
[Column(PrimaryKey = true)]
(of which there could be many), and possibly some others along those lines.Some other problems the above potentially solves (or not) would be case sensitivity (or not) and being something that the mutation methods (like
.Insert()
) can use. For example, whether to quote a field or not across.Contrib
provider implementations when generating T-SQL would be important. PerhapsColumn
has aCaseSensitive
property...or something else. The two-way nature on the deserialization of that is a thing, but generally handled by fallbacks today. Or it could be handled by exact names in the column attribute orFunc
overrides...or any of the combination of those things. Lots of options here - what makes sense to expose in a simple way for lots of use cases?Related Info
Related issues: #447, #463, #515, #605, #623, #670, #681, #572 Potentially covered: #482, #571
Thoughts? Suggestions? Things you'd like to throw in my general direction?
cc @mgravell