search

denisenkom / go-mssqldb

Microsoft SQL server driver written in go language
BSD 3-Clause "New" or "Revised" License
1.82k stars 501 forks source link

proposal: disable Result.LastInsertId #355

Open Fank opened 6 years ago

Fank commented 6 years ago

https://github.com/denisenkom/go-mssqldb/blob/master/mssql.go#L777

https://docs.microsoft.com/en-us/sql/t-sql/functions/scope-identity-transact-sql

SCOPE_IDENTITY and @@IDENTITY return the last identity values that are generated in any 
table in the current session. However, SCOPE_IDENTITY returns values inserted only 
within the current scope; @@IDENTITY is not limited to a specific scope.

SCOPE_IDENTITY is not affected by trigger on the table.

Fank commented 6 years ago

BTW Because i received There is no generated identity value multiple times, hope this will fix this.

kardianos commented 6 years ago

We went over this before in a PR ( https://github.com/denisenkom/go-mssqldb/pull/246#issuecomment-292509157 ). Yes, APIs have this Last insert concept. But it just doesn't map well to SQL Server. I would highly recommend not using it.

In Go1.10 on a recent version of this driver it may be impossible now that we have a sane connection reset policy in place.

Fank commented 6 years ago

You should disable this func (return only error) so people will stop using it, because currently it does work at all.

kardianos commented 6 years ago

I've updated the README. https://github.com/denisenkom/go-mssqldb/blob/master/README.md#important-notes .

Fair point. I'll do that soon.

kardianos commented 6 years ago

@denisenkom Thoughts?

denisenkom commented 6 years ago

I vote for disabling of Result.LastInsertId. Is there a way to mark this method as deprecated? I would keep it as deprecated for about a year and then remove, to avoid breaking users without notice.

mmargerumImpactrx commented 6 years ago

I just blew half a day on this so I vote for always throwing an error as well.

kardianos commented 6 years ago

Let's at least emit an error for go1.10+.

Fank commented 6 years ago

I would recommend something like this, as error text: Due a context issue T-SQL does not support LastInsertId. You should use SCOPE_IDENTITY(), an example can be found here (godoc link, with example)