Attendance_Add_tr with lost update protection

Messages.sql

--------------------------------------------------
-- Transaction misuse messages
--------------------------------------------------
exec sp_addmessage
  @msgnum = 100001,
  @msgtext = N'%s: is a transaction, which is Atomic. It has been called from within an open transaction, which would render it a non-transaction. That is not allowed.',
  @severity = 16,
  @lang = 'us_english',
  @with_log = 'false',
  @replace = 'replace';

exec sp_addmessage
  @msgnum = 100002,
  @msgtext = N'%s: is a utility transaction. It must be called from within a open transaction.',
  @severity = 16,
  @lang = 'us_english',
  @with_log = 'false',
  @replace = 'replace';

exec sp_addmessage
  @msgnum = 100003,
  @msgtext = N'%s: does not allow implicit transactions. Ensure that implicit transactions are not enabled (run command ''set implicit_transactions off;'') before trying again.',
  @severity = 16,
  @lang = 'us_english',
  @with_log = 'false',
  @replace = 'replace';

--------------------------------------------------
-- Organisation messages
--------------------------------------------------
exec sp_addmessage
  @msgnum = 100100,
  @msgtext = N'%s: Organisation does not exist.',
  @severity = 16,
  @lang = 'us_english',
  @with_log = 'false',
  @replace = 'replace';

--------------------------------------------------
-- Person messages
--------------------------------------------------
exec sp_addmessage
  @msgnum = 100200,
  @msgtext = N'%s: Person does not exist.',
  @severity = 16,
  @lang = 'us_english',
  @with_log = 'false',
  @replace = 'replace';

exec sp_addmessage
  @msgnum = 100201,
  @msgtext = N'%s: The Person record has been modified by another user. Please refresh your data.',
  @severity = 16,
  @lang = 'us_english',
  @with_log = 'false',
  @replace = 'replace';

--------------------------------------------------
-- Visit messages
--------------------------------------------------
exec sp_addmessage
  @msgnum = 100300,
  @msgtext = N'%s: Visit does not exist.',
  @severity = 16,
  @lang = 'us_english',
  @with_log = 'false',
  @replace = 'replace';

exec sp_addmessage
  @msgnum = 100301,
  @msgtext = N'%s: Visit has already been recorded for the given date.',
  @severity = 16,
  @lang = 'us_english',
  @with_log = 'false',
  @replace = 'replace';

go

Visit_Add_tr.sql

create procedure Visit_Add_tr
  @OrganisationId int,
  @PersonId int,
  @VisitDate date,
  @PersonLastUpdatedDtm datetimeoffset(3)
as

set transaction isolation level read committed;

declare @ProcName sysname = object_name(@@procid);
declare @CurrentLastUpdatedDtm datetimeoffset(3);

------------------------------------------------------------
-- Validation block
------------------------------------------------------------

-- Ensure we are not running in an already-opened transaction.
if @@TRANCOUNT > 0
begin
  raiserror (100001, -1, -1, @ProcName);
  return 1;
end

-- Ensure we are not running in 'implicit transactions' mode
-- (also known as 'chained transactions' on some platforms)
if (select @@OPTIONS & 2) <> 0
begin
  raiserror (100003, -1, -1, @ProcName);
  return 1;
end


if not exists (
  select 1
  from Organisation
  where OrganisationId = @OrganisationId
)
begin
  raiserror (100100, -1, -1, @ProcName); -- Organisation does not exist
  return 1;
end

select @CurrentLastUpdatedDtm = UpdatedDtm
from Person
where OrganisationId = @OrganisationId
  and PersonId = @PersonId;

if (@@ROWCOUNT != 1)
begin
  raiserror (100200, -1, -1, @ProcName); -- Person does not exist
  return 1;
end

if (@CurrentLastUpdatedDtm != @PersonLastUpdatedDtm)
begin
  raiserror (100201, -1, -1, @ProcName); -- Person has been updated by another user
  return 1;
end

if exists (
  select 1
  from Visit
  where OrganisationId = @OrganisationId
    and PersonId = @PersonId
    and VisitDate = @VisitDate
)
begin
  raiserror (100301, -1, -1, @ProcName); -- Visit on the given date has already been recorded.
  return 1;
end

------------------------------------------------------------
-- Execute block
------------------------------------------------------------

-- This 'set transaction isolation level' statement is an
-- alternative to specifying the 'holdlock' int as done below.
set transaction isolation level serializable;

begin transaction;

if not exists (
  select 1
  from Organisation
  with (holdlock)
  where OrganisationId = @OrganisationId
)
begin
  rollback;
  raiserror (100100, -1, -1, @ProcName); -- Organisation does not exist
  return 1;
end

select @CurrentLastUpdatedDtm = UpdatedDtm
from Person
with (updlock)
where OrganisationId = @OrganisationId
  and PersonId = @PersonId;

if (@@ROWCOUNT != 1)
begin
  rollback;
  raiserror (100200, -1, -1, @ProcName); -- Person does not exist
  return 1;
end

if (@CurrentLastUpdatedDtm != @PersonLastUpdatedDtm)
begin
  rollback;
  raiserror (100201, -1, -1, @ProcName); -- Person has been updated by another user
  return 1;
end

if exists (
  select 1
  from Visit
  with (updlock)
  where OrganisationId = @OrganisationId
    and PersonId = @PersonId
    and VisitDate = @VisitDate
)
begin
  rollback;
  raiserror (100301, -1, -1, @ProcName); -- Visit on the given date has already been recorded.
  return 1;
end

update Person
set UpdatedDtm = GETUTCDATE()
where OrganisationId = @OrganisationId
  and PersonId = @PersonId;

insert into Visit (OrganisationId, PersonId, VisitDate)
values (@OrganisationId, @PersonId, @VisitDate);

commit;

return 0;

Dan

Sorry, I did not look at the updated code earlier.

1. Just checking, that you have no Organisation.UpdatedDtm, because you have deemed Person.UpdatedDtm is enough for this example.

   • If so, fine.
   • If not, please add:

   exec sp_addmessage
     @msgnum = 100101,
     @msgtext = N'%s: The Organisation record has been modified by another user. Please refresh your data.',
     @severity = 16,
     @lang = 'us_english',
     @with_log = 'false',
     @replace = 'replace';
   

   • and I will upgrade the stored proc.

2. I don't understand why you are doing:

   • BEGIN TRAN and HOLDLOCK

3. HOLDLOCK is redundant here.

   • HOLDLOCK is relevant when in ISOLATION LEVEL 1 or 2, wherein the SELECT may be complex and query many tables, it applies to a single table, which is why the keyword is placed after the FROM.

4. The SET ISOLATION LEVEL (yes, I appreciate it is intended as documentary only) is not an alternative for HOLDLOCK, it is just documenting what BEGIN TRAN does.

Cheers
Derek

@dan

and I will upgrade the stored proc

There are a few issues. Rather than query every little thing in comments, it is much easier if I just submit an upgrade. It is ready o go. How do I do that ?

Let's keep the comments about minor matters here, and the comments about matters that may be relevant to all, in c_d_t.

Cheers
Derek

Hi Derek,

Yeah, there are a few ways to do it and I have mixed them together.

In SQL Server the default isolation level is read committed / isolation level 1. By issuing the statement set transaction isolation serializable; to change it to isolation level 3 I could do away with the holdlock hints.

As you say, the holdlock hint allows you to control precisely which tables you actually hold locks for. Setting the transaction isolation level, on the other hand, implicitly applies holdlock semantics for all rows touched.

That set transaction isolation level serializable statement does actually have an impact on the acquisition and retention of locks during the subsequent begin transaction / commit transaction block.
There's more on holdlock in SQL Server here: https://docs.microsoft.com/en-us/sql/t-sql/queries/hints-transact-sql-table?view=sql-server-ver15

If you'd like to share the code here in a comment, you can use three backticks (```) to start and end a code block.

Like this:
```
Code here
```

Just an update - I've looked at the code here: https://www.softwaregems.com.au/Documents/Article/Database/Transaction/Visit_Add_tr%20DA.sql

There's a few things that aren't quite right if written for SQL Server. The execute block, for example, is still running in read committed mode from the previous set statement. Thus the use of either holdlock or set transaction isolation level serializable in my own code.

I'll write more here tomorrow.

Dan

Just an update - I've looked at the code here: https://www.softwaregems.com.au/Documents/Article/Database/Transaction/Visit_Add_tr%20DA.sql

There's a few things that aren't quite right if written for SQL Server. The execute block, for example, is still running in read committed mode from the previous set statement. Thus the use of either holdlock or set transaction isolation level serializable in my own code.

And:

There's more on holdlock in SQL Server here: https://docs.microsoft.com/en-us/sql/t-sql/queries/hints-transact-sql-table?view=sql-server-ver15

Ok, you are right. Changed the sql file.

By issuing the statement set transaction isolation serializable; to change it to isolation level 3 I could do away with the holdlock hints.

Yes, at 3, HOLDLOCK is redundant. It is useful (same as Sybase) when in 1, to request that shared locks for the elected table to be held until end of tran.

Btw. Whereas I see your comments on the Pull Request pages, I don't see your comments on the Gist pages, such as this one. I have tor reload the page and check. So how is it that you see my comments on this Gist page ?

Cheers
Derek

Dan

For understanding. There is one more issue with your Vist_Add_tr.sql that I do not understand: the use of GETUTCDATE() instead of GETDATE(), and the messing around with it in the code. It could pertain to the concern amount clock skew, I don’t know. On the face of it, it seems that you are still in the MVCC mindset, collected a TimeStamp at retrieval time, rather than OLTP, which is collecting the TimeStamp of the row (Data Currency).

The MVCC mindset is totally broken, a complete fantasy. Not a little broken. There is nothing at all that can be salvaged from it.

Notice that the retrieval time is irrelevant. Please study Transaction Sanity p7. Study all the TimeStamps which ar [Tx], and ask specific questions.

Notice again, [if it did] pertain to clock skew, that the code it immune to it.

Cheers
Derek

Hi Derek,

So how is it that you see my comments on this Gist page

I'm subscribed to the Gist. There's a button at the top with a bell icon.

Dan

So how is it that you see my comments on this Gist page

I'm subscribed to the Gist. There's a button at the top with a bell icon.
...

I am Subscribed to this Gist.

There are other issues, GitHub is plain clunky. Eg. if I enter @D in a PullRequest comment, it shows a list and I can choose @Daniel.Loth, which shows up bold @Daniel.Loth, and I don't have to reload the page: new comments are already there when I go to the page. If I enter @D in a Gist comment, it does nothing. Hence here it is regular "Dan", and a reload to check for new comments.

Anyway, too small to worry about. Could be Firefox.

Cheers
Derek

There is one more issue with your Vist_Add_tr.sql that I do not understand: the use of GETUTCDATE() instead of GETDATE(), and the messing around with it in the code.

Either could be used. I use UTC time out of habit.

When you mentioned messing around with it in code, are you referring to this?

select @CurrentLastUpdatedDtm = UpdatedDtm
from Person
where OrganisationId = @OrganisationId
  and PersonId = @PersonId;

if (@@ROWCOUNT != 1)
begin
  raiserror (100200, -1, -1, @ProcName); -- Person does not exist
  return 1;
end

if (@CurrentLastUpdatedDtm != @PersonLastUpdatedDtm)
begin
  raiserror (100201, -1, -1, @ProcName); -- Person has been updated by another user
  return 1;
end

If so, I was just using this approach to check both existence and currency with a single select statement.