`Variable Already Declared` when Shadowed/New Properties exist in Hierarchy of Model

PS: Sorry for the VB.NET :stuck_out_tongue:

Imagine these classes:

Public Class EntityBase
  Public Property ColumnA As String
  Public Property ColumnB As String

  Public Class QueryEntity
    Inherits EntityBase

    Public Shadows Property ColumnA As DbString
    Public Shadows Property ColumnB As DbString
  End Class
End Class

The idea of this is to use EntityBase as the Model for the application, but be able to construct a QueryEntity whenever I need to Query/UPSERT data. However, using these instance hierarchy like this ends up in:

System.Data.SqlClient.SqlException (0x80131904): The variable name '@ColumnA' has already been declared. Variable names must be unique within a query batch or stored procedure. Must declare the scalar variable "@ColumnB". 
at System.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction) 
at System.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction) 
at System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose) 
at System.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady) 
at System.Data.SqlClient.SqlCommand.FinishExecuteReader(SqlDataReader ds, RunBehavior runBehavior, String resetOptionsString) 
at System.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean async, Int32 timeout, Task& task, Boolean asyncWrite) 
at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method, TaskCompletionSource`1 completion, Int32 timeout, Task& task, Boolean asyncWrite) 
at System.Data.SqlClient.SqlCommand.InternalExecuteNonQuery(TaskCompletionSource`1 completion, String methodName, Boolean sendToPipe, Int32 timeout, Boolean asyncWrite) 
at System.Data.SqlClient.SqlCommand.ExecuteNonQuery() 
at Dapper.SqlMapper.ExecuteCommand(IDbConnection cnn, CommandDefinition& command, Action`2 paramReader) in D:\Dev\dapper-dot-net\Dapper NET40\SqlMapper.cs:line 3397 
at Dapper.SqlMapper.ExecuteImpl(IDbConnection cnn, CommandDefinition& command) in D:\Dev\dapper-dot-net\Dapper NET40\SqlMapper.cs:line 1346 
at Dapper.SqlMapper.Execute(IDbConnection cnn, String sql, Object param, IDbTransaction transaction, Nullable`1 commandTimeout, Nullable`1 commandType) in D:\Dev\dapper-dot-net\Dapper NET40\SqlMapper.cs:line 1221 
at ...

This may very well be a known-issue and leave it be, but thought it worth bringing it up.

You mention hierarchy which suggests inheritance - in VB, is this an implicit inheritance scenario? Or is this just type nesting? Just want to make sure I understand the scenario. On 4 Nov 2015 1:52 pm, "Mauricio Morales" notifications@github.com wrote:

PS: Sorry for the VB.NET [image: :stuck_out_tongue:]

Imagine these classes:

Public Class EntityBase Public Property ColumnA As String Public Property ColumnB As String

Public Class QueryEntity Public Shadows Property ColumnA As DbString Public Shadows Property ColumnB As DbString End Class End Class

The idea of this is to use EntityBase as the Model for the application, but be able to construct a QueryEntity whenever I need to Query/UPSERT data. However, using these instance hierarchy like this ends up in:

System.Data.SqlClient.SqlException (0x80131904): The variable name '@ColumnA' has already been declared. Variable names must be unique within a query batch or stored procedure. Must declare the scalar variable "@ColumnB". at System.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action1 wrapCloseInAction) at System.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection, Action1 wrapCloseInAction) at System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose) at System.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady) at System.Data.SqlClient.SqlCommand.FinishExecuteReader(SqlDataReader ds, RunBehavior runBehavior, String resetOptionsString) at System.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean async, Int32 timeout, Task& task, Boolean asyncWrite) at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, String method, TaskCompletionSource1 completion, Int32 timeout, Task& task, Boolean asyncWrite) at System.Data.SqlClient.SqlCommand.InternalExecuteNonQuery(TaskCompletionSource1 completion, String methodName, Boolean sendToPipe, Int32 timeout, Boolean asyncWrite) at System.Data.SqlClient.SqlCommand.ExecuteNonQuery() at Dapper.SqlMapper.ExecuteCommand(IDbConnection cnn, CommandDefinition& command, Action2 paramReader) in D:\Dev\dapper-dot-net\Dapper NET40\SqlMapper.cs:line 3397 at Dapper.SqlMapper.ExecuteImpl(IDbConnection cnn, CommandDefinition& command) in D:\Dev\dapper-dot-net\Dapper NET40\SqlMapper.cs:line 1346 at Dapper.SqlMapper.Execute(IDbConnection cnn, String sql, Object param, IDbTransaction transaction, Nullable1 commandTimeout, Nullable`1 commandType) in D:\Dev\dapper-dot-net\Dapper NET40\SqlMapper.cs:line 1221 at ...

This may very well be a known-issue and leave it be, but thought it worth bringing it up.

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

You are right. The nesting is not necessary to my example (but is how I have it in my project where I noticed the issue).

As for the inheritance, I realized after posting that I missed the "Inherits EntityBase" in the QueryEntity spec. I added it later but looks like you got the message as posted the first time.

Sent from my iPhone

As for the inheritance, I realized after posting that I missed the "Inherits EntityBase" in the QueryEntity spec. I added it later but looks like you got the message as posted the first time.

Yup, that was exactly it - and was why I wanted to be very sure I understood the scenario before jumping in with both feet.

That is ... an interesting scenario. I can guarantee that it isn't one that we've specifically designed for, and it prompts some very interesting thoughts on what should happen. So let me ask you your opinion - am I right in thinking that you would choose only the most derived property?

Let me complicate that a bit more, by adding case sensitivity - imagine the base type declares foo and the derived type declares Foo; this prompts all sorts of interesting dilemmas:

in C# foo and Foo are distinct; in VB they are not
in SQL Server, whether @foo and @Foo are distinct depends on the server configuration - it can be either

I don't know if there is a single right or wrong answer here; I'm just throwing it open for discussion.

I don't think there will be a right answer, so I'd start by arguing if this is even something worth supporting. Best case scenario it'll imply some sort of (ideally) hash lookup over the already added SqlParameters to avoid duplication (or perhaps tweak the Reflection member listing). This effectively adds cycles to the overall performance for a less than common use case.

On the other hand, let me paint the picture of why we use it this way: We have an Application Model class (EntityBase in this example) that we use for all application logic and on which I need them to be String (instead of DbString). But when inserting and particularly on UPDATE WHERE clauses, the SqlParameter for String ends up being NVARCHAR(MAX) (or similar) but that fails to match the type of indexes and hence performs full table scans. That's why for "UPSERT"s I need (at least) the index columns to be DbString with the correct Ansi/Length/Vary settings. So of course our classes have a bit more stuff (clone constructors, etc).

I can achieve all this by appending Query to my Upsertable properties, but that causes Dapper to push all inherited properties effectively pushing SQL params for both ColumnA and ColumnAQuery. That's how we have it today and works so much faster than NHibernate :stuck_out_tongue: but would love it if I could avoid the unnecessary data push without losing the polymorphism I get from EntityBase ent = new EntityQuery();.

Now back to your line of though, I personally would submit that if we wanted to make Dapper support all possible configurations, we'd soon end up with performances similar to NHibernate, EntityFramework and what not. I would also submit that while casing would be a conflict, it sounds like a conflict rooted in bad programming practices (a subjective matter, I know). But even though C# is case sensitive, I still consider it a code smell to mix casing on Properties (much worst in inherited properties). So maybe this argument helps us simplify which scenarios we'd want to live by and which ones can be an accepted risk.

As for SQL Server's configurability on parameter casing... kind of the same argument: passing @foo & @Foo in the same hit feels like a junior'ish move. Am I off here?

The problem isn't whether or not it is a junior move, but what the engine can figure out. It has to assume case insensitivity because someone could use both @Foo and @foo in their SQL to mean the same thing, so we look for a case insensitive match (typically Foo). But I think a reasonable route forward here is simply:

if your model declares members with the same name (case insensitive) at different levels in the hierarchy, only the one from the most derived type is used by dapper

This would solve your scenario, and doesn't introduce any problems as we know that currently doesn't work in any sense (from your error). On 5 Nov 2015 6:05 am, "Mauricio Morales" notifications@github.com wrote:

I don't think there will be a right answer, so I'd start by arguing if this is even something worth supporting. Best case scenario it'll imply some sort of (ideally) hash lookup over the already added SqlParameters to avoid duplication (or perhaps tweak the Reflection member listing. This effectively adds cycles to the overall performance for a less than common use case.

On the other hand, let me paint the picture of why we use it this way: We have an Application Model class (EntityBase in this example) that we use for all application logic and on which I need them to be String (instead of DbString). But when inserting and particularly on UPDATE WHERE clauses, the SqlParameter for String ends up being NVARCHAR(MAX) (or similar) but that fails to match the type of indexes and hence performs full table scans. That's why for "UPSERT"s I need (at least) the index columns to be DbString with the correct Ansi/Length/Vary settings. So of course our classes have a bit more stuff (clone constructors, etc).

I can achieve all this by appending Query to my Upsertable properties, but that causes Dapper to push all inherited properties effectively pushing SQL params for both ColumnA and ColumnAQuery. That's how we have it today and works so much faster than NHibernate [image: :stuck_out_tongue:] but would love it if I could avoid the unnecessary data push without losing the polymorphism I get from EntityBase ent = new EntityQuery();.

Now back to your line of though, I personally would submit that if we wanted to make Dapper support all possible configurations, we'd soon end up with performances similar to NHibernate, EntityFramework and what not. I would also submit that while casing would be a conflict, it sounds like a conflict rooted in bad programming practices (a subjective matter, I know). But even though C# is case sensitive, I still consider it a code smell to mix casing on Properties (much worst in inherited properties). So maybe this argument helps us simplify which scenarios we'd want to live by and which ones can be an accepted risk.

As for SQL Server's configurability on parameter casing... kind of the same argument: passing @foo https://github.com/foo & @Foo https://github.com/Foo in the same hit feels like a junior'ish move. Am I off here?

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

Hmm. over 5 years old, still Open, unresolved? Yeah, I just got bit by this.

"if your model declares members with the same name (case insensitive) at different levels in the hierarchy, only the one from the most derived type is used by dapper"

Nope. Not in this case. Check this out:

public class CompanyVM
{
    public string ID { get; set; }

    public string Name { get; set; }
}

public class CompanyDM : CompanyVM
{
    public new int ID { get; set; }
}

Notice the parent has a string ID, and the child has "new int ID". Why use the new keyword with a different type? Because of a security layer that masks int ID from the DB, converts to a string before sending to the client. A totally legit scenario, works awesome through the entire app... until I hit Dapper with ExecuteScalarAsync(query, myCompanyDM), because it "sees" both parent and child properties.

The variable name '@ID' has already been declared

Dang it.

DapperLib / Dapper

`Variable Already Declared` when Shadowed/New Properties exist in Hierarchy of Model #381