Insert doesn't return ID when Primary Key is uniqueidentifier #128

Closed
deevus opened this Issue Feb 14, 2012 · 2 comments

3 participants

@deevus

If you have a table where the Primary Key is uniqueidentifier the use of @@IDENTITY will never return the new inserted value.

I had an idea to change the stub variable in CreateInsertCommand:
var stub = "INSERT INTO {0} ({1}) \r\n OUTPUT inserted.{2} \r\n VALUES ({3})";

Which would then format the sql as so:
var sql = string.Format(stub, TableName, keys, PrimaryKeyField, vals);

Then just change Insert to use ex.ID = cmd.ExecuteScalar(); and remove the call to ExecuteNonQuery()

The only problem being that if the table doesn't provide a primary key or one wasn't set when inheriting DynamicModel this will break. Currently I have overridden CreateInsertCommand and Insert myself for the specific table with a uniqueidentifier PK, but I thought maybe there could be a solution that solves the problem of non-INT PK's all round.

@shankarab

I didn't face the uniqueidentifier issue, but ex.ID = cmd.ExecuteScalar() poses a problem for me if my primary key is named "Id" instead of "ID" (or anything other than ID). Hence I had to change the code like this (cast ex to IDictionary so that I can set "Id" to cmd.ExecuteScalar)

if (BeforeSave(ex)) {
using (dynamic conn = OpenConnection()) {
var cmd = CreateInsertCommand(ex);
cmd.Connection = conn;
cmd.ExecuteNonQuery();
cmd.CommandText = "SELECT @@IDENTITY as newID";
var result = ex as IDictionary;
result[PrimaryKeyField] = cmd.ExecuteScalar();
//ex.ID = cmd.ExecuteScalar();
Inserted(ex);
}
}

@robconery
Collaborator

The code currently is written to support SQL CE as well as SQL Server. If you need to use a guid you should change the file to use SCOPE_IDENTITY. I believe there is a comment inline about this.

Better yet - don't use newid(). Pass the GUID in.

Better better yet, don't use GUIDs as a PK. It's absolutely horrid as a primary key from a perf perspective and also data size. You care about this because indexing GUIDs takes a long time, and having a unique constraint on top of GUIDs is silly (which the PK enforces).

Best bet is to have a separate column and just use an integer/auto increment. Yes I understand that you might not have control over this - which is sad indeed. Just some thoughts (not mine, a DBA friend of mine).

@robconery robconery closed this May 2, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment