Entity Framework 6 storedprocedure InputOut field DbNull check

Posts   
 
    
tbeyhan
User
Posts: 23
Joined: 23-May-2006
# Posted on: 07-Jan-2014 15:47:03   

Hello, It seems generated stored procedure call functions don't check InputOutput parameters values against null. Below you'll see the generated code.


public int CallSgGetPartajbranskomhplkodu(System.Int32 partajbransno, ref System.String partajkomhplkodu, ref System.String partajkomalacakhplkodu)
{
    var cmd = CreateStoredProcCallCommand("[dbo].SG_GET_PARTAJBRANSKOMHPLKODU]", true);
    AddParameter(cmd, "@PARTAJBRANSNO", 0, ParameterDirection.Input, partajbransno);
    AddParameter(cmd, "@PARTAJKOMHPLKODU", 20, ParameterDirection.InputOutput, partajkomhplkodu);
    AddParameter(cmd, "@PARTAJKOMALACAKHPLKODU", 20, parameterDirection.InputOutput, partajkomalacakhplkodu);
    var toReturn = ExecuteNonQueryCommand(cmd);
        
        partajkomhplkodu = (System.String)cmd.Parameters[1].Value;
        partajkomalacakhplkodu = (System.String)cmd.Parameters[2].Value;
        
    return toReturn;
        }

Also generated ExecuteNonQueryCommand method closes the connection after executing the query but with EF6 as you know DbContext.Database.Open method opens the connection and stay open until explicitly closed or context disposed.


private int ExecuteNonQueryCommand(DbCommand cmd)
{
    var connection = GetConnection();
    bool closeLocally = (connection.State==ConnectionState.Open);
    var toReturn = cmd.ExecuteNonQuery();
    if(closeLocally)
    {
          connection.Close();
     }
    return toReturn;
}

Is there anything that i missed on protect configuration or should i change the templates which is an easy thing to do.

Ideas? tunc

Walaa avatar
Walaa
Support Team
Posts: 14950
Joined: 21-Aug-2005
# Posted on: 07-Jan-2014 18:58:20   

Which version of LLBLGen Pro are you using? (Designer's Release Date)

tbeyhan
User
Posts: 23
Joined: 23-May-2006
# Posted on: 08-Jan-2014 08:06:03   

Sorry flushed i forgot to mention 4.1 Final November 26th, 2013

Thanks, tunc

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 08-Jan-2014 10:56:34   

tbeyhan wrote:

Hello, It seems generated stored procedure call functions don't check InputOutput parameters values against null. Below you'll see the generated code.


public int CallSgGetPartajbranskomhplkodu(System.Int32 partajbransno, ref System.String partajkomhplkodu, ref System.String partajkomalacakhplkodu)
{
    var cmd = CreateStoredProcCallCommand("[dbo].SG_GET_PARTAJBRANSKOMHPLKODU]", true);
    AddParameter(cmd, "@PARTAJBRANSNO", 0, ParameterDirection.Input, partajbransno);
    AddParameter(cmd, "@PARTAJKOMHPLKODU", 20, ParameterDirection.InputOutput, partajkomhplkodu);
    AddParameter(cmd, "@PARTAJKOMALACAKHPLKODU", 20, parameterDirection.InputOutput, partajkomalacakhplkodu);
    var toReturn = ExecuteNonQueryCommand(cmd);
        
        partajkomhplkodu = (System.String)cmd.Parameters[1].Value;
        partajkomalacakhplkodu = (System.String)cmd.Parameters[2].Value;
        
    return toReturn;
        }

You mean, it should test for null and if so, it should pick a default value or something? As there's not much to do with a null if the target can't hold it. I.e.: the variable is of type int and the parameter returns null.... If a parameter can return null, it should be marked as such in the designer so the variable is generated as a nullable type, if it's a value type.

Also generated ExecuteNonQueryCommand method closes the connection after executing the query but with EF6 as you know DbContext.Database.Open method opens the connection and stay open until explicitly closed or context disposed.


private int ExecuteNonQueryCommand(DbCommand cmd)
{
    var connection = GetConnection();
    bool closeLocally = (connection.State==ConnectionState.Open);
    var toReturn = cmd.ExecuteNonQuery();
    if(closeLocally)
    {
          connection.Close();
     }
    return toReturn;
}

Are you sure EF6 opens the connection as soon as you create a context object? I have not read about that. To my knowledge they open a connection when they need to, i.e. when you execute a query or when you save the changes. It then closes the connection afterwards. To my knowledge: http://msdn.microsoft.com/en-us/data/dn456849 says what our code does, or am I misinterpreting it? (could be, their connection management is a moving target sometimes).

Frans Bouma | Lead developer LLBLGen Pro
tbeyhan
User
Posts: 23
Joined: 23-May-2006
# Posted on: 09-Jan-2014 10:15:01   

Hi,

You mean, it should test for null and if so, it should pick a default value or something? As there's not much to do with a null if the target can't hold it. I.e.: the variable is of type int and the parameter returns null.... If a parameter can return null, it should be marked as such in the designer so the variable is generated as a nullable type, if it's a value type.

Yes, the generated code for LLBLGen has a small difference

public static int SgGetPartajbranskomhplkodu(System.Int32 partajbransno, ref System.String partajkomhplkodu, ref System.String partajkomalacakhplkodu, ref System.Int32 returnValue, DataAccessAdapter adapter)
        {
            // create parameters. Add 1 to make room for the return value parameter.
            SqlParameter[] parameters = new SqlParameter[3 + 1];
            parameters[0] = new SqlParameter("@PARTAJBRANSNO", SqlDbType.Int, 0, ParameterDirection.Input, true, 10, 0, "",  DataRowVersion.Current, partajbransno);
            parameters[1] = new SqlParameter("@PARTAJKOMHPLKODU", SqlDbType.VarChar, 20, ParameterDirection.InputOutput, true, 0, 0, "",  DataRowVersion.Current, partajkomhplkodu);
            parameters[2] = new SqlParameter("@PARTAJKOMALACAKHPLKODU", SqlDbType.VarChar, 20, ParameterDirection.InputOutput, true, 0, 0, "",  DataRowVersion.Current, partajkomalacakhplkodu);
            parameters[3] = new SqlParameter("RETURNVALUE", SqlDbType.Int, 0, ParameterDirection.ReturnValue, true, 10, 0, "",  DataRowVersion.Current, returnValue);
            int toReturn = adapter.CallActionStoredProcedure("[ELWIN2].[dbo].[SG_GET_PARTAJBRANSKOMHPLKODU]", parameters);
            if(parameters[1].Value!=System.DBNull.Value)
            {
                partajkomhplkodu = (System.String)parameters[1].Value;
            }
            if(parameters[2].Value!=System.DBNull.Value)
            {
                partajkomalacakhplkodu = (System.String)parameters[2].Value;
            }
            
            returnValue = (int)parameters[3].Value;
            return toReturn;
        }

It checks the field for DBNull. The code fragment has generated by v 2.6 but i don't think it's changed in v4.1. Also i tried to use IsOptional field setting in designer but it did't make any difference.

Are you sure EF6 opens the connection as soon as you create a context object? I have not read about that. To my knowledge they open a connection when they need to, i.e. when you execute a query or when you save the changes. It then closes the connection afterwards. To my knowledge: http://msdn.microsoft.com/en-us/data/dn456849 says what our code does, or am I misinterpreting it? (could be, their connection management is a moving target sometimes).

I'm using the same document as a reference and my implementation exactly fits Database.Connection.Open() / Behavior in EF6 and future versions paragraph. Opening connection explicitly and calling stored procedures and SaveChanges repeatedly.

Walaa avatar
Walaa
Support Team
Posts: 14950
Joined: 21-Aug-2005
# Posted on: 10-Jan-2014 10:55:50   

As the I/O parameters here are of string types, then it can accept a null. If they were int for example, the IsOptional should have changed the types to int? to be able to accept nulls.

I don't see the need to check for nulls, if the types are strings, if the DB return nulls so the parameter should hold null indeed for the caller to know what the database have returned.

tbeyhan
User
Posts: 23
Joined: 23-May-2006
# Posted on: 10-Jan-2014 15:55:54   

As the I/O parameters here are of string types, then it can accept a null. If they were int for example, the IsOptional should have changed the types to int? to be able to accept nulls.

I don't see the need to check for nulls, if the types are strings, if the DB return nulls so the parameter should hold null indeed for the caller to know what the database have returned.

If you don't check the value for DBNull you'll get an exception as i get. Also as i stated before Templates for LLBLGen Pro inject if statements to check DBNull. You can see the generated code in my previous mail

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 10-Jan-2014 17:49:30   

We'll look into the dbnull issue. v2.6 code isn't comparable with v4 code wrt parameter values, however dbnull <-> null conversions should still work. (our v4 code does that internally in StoredProcedureCall, where parameter values which are DBNull or null, are converted to default values for the type at hand. The EF proc code doesn't do that indeed.

With respect to the connection open/close... I still don't fully grasp what you mean with the connection open / close:

I'm using the same document as a reference and my implementation exactly fits Database.Connection.Open() / Behavior in EF6 and future versions paragraph. Opening connection explicitly and calling stored procedures and SaveChanges repeatedly.

What exactly do you do (actions in what order) and what issue do you currently run into (e.g. exceptions, code outside transactions etc.)..

Frans Bouma | Lead developer LLBLGen Pro
tbeyhan
User
Posts: 23
Joined: 23-May-2006
# Posted on: 13-Jan-2014 08:45:07   

With respect to the connection open/close... I still don't fully grasp what you mean with the connection open / close:

Here is my code looks like

using (ElmaDataContext ctx = new ElmaDataContext())
{
    ctx.Database.Connection.Open();
    var txn = ctx.Database.BeginTransaction();

...Insert some entities 

    ctx.SaveChanges();

    ctx.Call... Call some sp

    ... Update some entities    
    
    ctx.SaveChanges();
    
    txn.Commit();
}

Looking at the generating code i assume closing the connection will raise unexpected behaviour. May be i thought wrong, i'll debug and trace to be sure

Thanks

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 13-Jan-2014 09:56:20   

if you start the transaction explicitly, then things can go wrong indeed, but they will go wrong this way anyway, as the proc calls don't wire the transaction to the command, which means the proc call isn't running inside a transaction which could cause deadlocks due to locks being set. if the procs don't have to run inside the transaction, they should be executed outside the transaction. The design of the procs is meant to be that way: a call which controls its own connection.

(edit) why do you call savechanges twice? you don't have to do that, just call it once, at the end. As the procs don't run inside the transaction, it's best to do them outside the connection open, so first call the procs, then add/update the entities (no connection open/transaction started) and then call save changes. If that's impossible, the procs have to be wired to the transaction, there's no other way to avoid deadlocks/lock issues otherwise.

Frans Bouma | Lead developer LLBLGen Pro
tbeyhan
User
Posts: 23
Joined: 23-May-2006
# Posted on: 13-Jan-2014 13:15:44   

(edit) why do you call savechanges twice? you don't have to do that, just call it once, at the end. As the procs don't run inside the transaction, it's best to do them outside the connection open, so first call the procs, then add/update the entities (no connection open/transaction started) and then call save changes. If that's impossible, the procs have to be wired to the transaction, there's no other way to avoid deadlocks/lock issues otherwise.

Hi Frans,

Yes i am aware of that smile But for our implementation we need that also the sp should be in the same transaction. Anyway i'll build some templates to solve my problem.

Thanks,

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 13-Jan-2014 18:06:55   

No that's ok, I'll add it, I just wondered whether you needed it as the current templates don't do that as well (which is indeed an oversight in line with the connection issue). As your use case is a valid use case, we'll update our templates so they'll be usable in your scenario.

I hope to have the fix tomorrow (tuesday).

Frans Bouma | Lead developer LLBLGen Pro
Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 14-Jan-2014 10:25:58   

There's no way to obtain the current transaction, as the connection itself doesn't hold a reference to it, nor does the context. This means that the proc call won't run inside the transaction, as it doesn't know the transaction. You open it yourself in your code, and there's no way to know what the ambient transaction is: you might use a transactionscope and it then has to enlist the transaction there, which it currently doesn't do either.

So we skip the transaction enlistment for proc calls. The dbnull / connection open/close issues will be addressed however.

Frans Bouma | Lead developer LLBLGen Pro
Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 14-Jan-2014 11:53:36   

See attached template.

Store in <llblgen installation folder>\Frameworks\Entity Framework\Templates\Shared\C#

as administrator

we'll also release a new build of the installer later today.

Attachments
Filename File size Added on Approval
contextClassInclude.lpt 32,449 14-Jan-2014 11:53.50 Approved
Frans Bouma | Lead developer LLBLGen Pro
tbeyhan
User
Posts: 23
Joined: 23-May-2006
# Posted on: 14-Jan-2014 12:03:14   

Hi Frans,

I've been using LLBLGen since 2006. Thank you very much for your excellent support and your excellent products for all these years.

Otis avatar
Otis
LLBLGen Pro Team
Posts: 39614
Joined: 17-Aug-2003
# Posted on: 14-Jan-2014 14:40:55   

smile

Frans Bouma | Lead developer LLBLGen Pro