Attendance_Add_tr.sql

create procedure dbo.Attendance_Add_tr
  @OrganisationId int,
  @PersonId int,
  @AttendanceDate date
as

-- MSSQL equivalent of disabling chained transaction
set implicit_transactions off;

set nocount on;

------------------------------------------------------------
-- Validation block
------------------------------------------------------------
set transaction isolation level read committed;

if @@TRANCOUNT > 0
begin
  exec ThrowError_OpenTransaction @@PROCID;
end

if not exists (
  select 1
  from dbo.Organisation
  where OrganisationId = @OrganisationId
)
begin
  return 9; -- Organisation does not exist
end

if not exists (
  select 1
  from dbo.Person
  where OrganisationId = @OrganisationId
    and PersonId = @PersonId
)
begin
  return 8; -- Person does not exist
end

if exists (
  select 1
  from dbo.Attendance
  where OrganisationId = @OrganisationId
    and PersonId = @PersonId
    and AttendanceDate = @AttendanceDate
)
begin
  return 7; -- Attendance on the given date has already been recorded.
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 dbo.Organisation
  with (holdlock)
  where OrganisationId = @OrganisationId
)
begin
  rollback;
  return 9; -- Organisation does not exist
end

if not exists (
  select 1
  from dbo.Person
  with (holdlock)
  where OrganisationId = @OrganisationId
    and PersonId = @PersonId
)
begin
  rollback;
  return 8; -- Person does not exist
end

if exists (
  select 1
  from dbo.Attendance
  with (holdlock)
  where OrganisationId = @OrganisationId
    and PersonId = @PersonId
    and AttendanceDate = @AttendanceDate
)
begin
  rollback;
  return 7; -- Attendance on the given date has already been recorded.
end

insert into dbo.Attendance (OrganisationId, PersonId, AttendanceDate)
select @OrganisationId, @PersonId, @AttendanceDate;

commit;

Attendance_Get.sql

create procedure dbo.Attendance_Get
  @OrganisationId int,
  @PersonId int,
  @AttendanceDate date
as

set nocount on;
set transaction isolation level read committed;

select OrganisationId, PersonId, AttendanceDate
from dbo.Attendance
where OrganisationId = @OrganisationId
  and PersonId = @PersonId
  and AttendanceDate = @AttendanceDate;

if @@ROWCOUNT != 1
  return 1;
else
  return 0;

GetObjectName_fn.sql

create function dbo.GetObjectName_fn (
  @ObjectId int
)
returns nvarchar(257)
as
begin

return object_schema_name(@ObjectId) + '.' + object_name(@ObjectId);

end

ThrowError_OpenTransaction.sql

create procedure dbo.ThrowError_OpenTransaction (
  @ProcId int
)
as
begin

declare @ErrorMessage nvarchar(512) = concat(
  'The procedure ''', dbo.GetObjectName_fn(@ProcId), ''' is a transaction, which is atomic. ',
  'It has been called within an open transaction, which would render it a non-transaction. ',
  'This is not allowed.'
);

throw 100000, @ErrorMessage, 0;

end

ThrowError_UtilityTransactionRequiresOpenTransaction.sql

create procedure dbo.ThrowError_UtilityTransactionRequiresOpenTransaction (
  @ProcId int
)
as
begin

declare @ErrorMessage nvarchar(512) = concat(
  'The procedure ''', dbo.GetObjectName_fn(@ProcId), ''' is a utility transaction. ',
  'It must be called from within an open transaction.'
);

throw 100001, @ErrorMessage, 0;

end

Nice work.

Attendance_Add_tr.sql

set nocount on;

Generally the count is a session config parm, it is shown in interactive apps, either character or GUI. It does not show from a transaction, and you don't want to set it, because it is a thing that the caller sets, and you don't want to not-show when he wants it shown.

insert into dbo.Attendance (OrganisationId, PersonId, AttendanceDate) select @OrganisationId, @PersonId, @AttendanceDate

INSERT dbo.Attendance( OrganisationId, PersonId, AttendanceDate )
    VALUES( @OrganisationId, @PersonId, @AttendanceDate )

is faster.

dbo.

Is that a MS thing ?
It is hard-coding.

THROW
Compared with RAISERROR, it is horrible.

Cheers
Derek

My specifying the schema name is mostly just out of habit. Not strictly necessary when database objects are living in the dbo schema.

So RAISERROR still exists, and is still usable. It's just that it contravenes the advice in Microsoft's documentation:

INSERT dbo.Attendance( OrganisationId, PersonId, AttendanceDate )
VALUES( @OrganisationId, @personid, @AttendanceDate )

Yeah, reasonable change. The version above is a hangover from when it had the in-lined EXISTS check.

Danno

I have posted the context for my comments re generic SQL on your c.d.t/Stored Proc Structure thread.

• Generic SQL context

I am exchanging info re specific SQL, MS/SQL in this github thread.

Schema

The notion of schema is bankrupt, duplication of any thing sucks dead bears. It is a late addition to ISO/IEEE/ANSI SQL. We do not use it.

Our Standard, which is at home in the original SQL context, and existed long before the "schema" concept, requires the following. Maintenance of four versions of the database; object migration; and version control (eg. CVS or whatever) have specific requirements:

• DBO as initially implemented is a great concept ... but like everything else, it needs to be used properly, with a plan

• DBO is the only User in any Production or UAT database
  --- DBO is the default. It should never be coded (if it is, it breaks the overall no-hard-coding rule). Users do not know about DBO, and their report tools; etc default to DBO.
  --- Any non-DBO objects in Prod/UAT are considered security violations, and removed immediately.

• In Dev & Test databases, the above goal (the target the Dev code is intended for) is kept in mind.
  --- the data modeller/DBA provides a set of DBO tables; etc
  --- Each developer has their own tables (non-DBO), and maintains permissions, which means they consciously grant/revoke access to other developers. This allows completely unrestricted use of the database and SQL. Usually such is started by copying the DBO table.
  --- if and when their code objects (not table) is ready to be promoted, after passing a formal peer review; etc, the DBA moves the developer code objects to DBO. Literally, change <user.><object_name> to dbo.<object_name> and execute.
  --- At that point, their code objects get tested against DBO tables in the Dev/Test database. When qualified, the code can be migrated to UAT. But that is usually a collection, an app version increment.

RAISERROR

Noting my context. My advice does not change, I will still deliver RAISERROR in the code. Your are free to translate that to MS/SQL as you wish, as per standards and determinations at your end, you do not need to justify it. Your call, same as set implicit_transactions off.

MS is as stupid as they ever were, and they have a long history of forcing users to comply with their particular flavour of SQL, their particular way of doing things. This is so that people cannot get off MS/SQL once they are in it (the freeware uses the same seduce-and-kidnap method). They could have changed RAISERROR to "honour" SET_XACT_ABORT, maintaining backward compatibility, but they did not, they implemented a newer method, which makes RAISERROR obsolete.

Here, it is the result that is most offensive: instead of a single set of "user defined" error messages, which we have had since the 1980's, they force you into private messages (hard coding) and a full set of Functions. THROW plus an additional set of objects.

Cheers
Derek

Hi Derek,

I'll probably update the above with RAISERROR. Given that the code is written to explicitly perform the rollback, I don't see the need to rely on XACT_ABORT.

Dan

Could you please start another thread or fork or whatever, for the discussion re your DM. If possible, the comments above that pertain to it.

Cheers
Derek

Hi Derek,

I've created a repository which has current code and diagrams here: https://github.com/DanielLoth/ShootingClubDatabase

Pull request with the above diagram here: DanielLoth/ShootingClubDatabase#2

I created another pull request with current SQL code here: DanielLoth/ShootingClubDatabase#3
Note that most stuff is not to specification - only the Attendance-related methods that have already been discussed.
For the most part, I'd ignore this pull request. I just put it there to make it available.

Cheers,
Dan

Dan

Thank you.

Can you possibly remove the comments in thread that are NOT related to the c.d.t thread.
That guy was confused to begin with, and getting more confused as we progress, hindering progress.
Let's keep this thread for your initial stated purpose only, which is associated with the c.d.t thread.

In this thread (sproc template for ACID Transaction using a Lock Manager), there are (b) two issues or conditions that need to be identified and named, before I can give the solution (c).

In the sense of keeping one's eye on the goal, we are heading to is (c) Optimistic Locking (don't tell Nicola). But first we need to understand why it is necessary, which is [b].

Cheers
Derek

Hi Derek,

Busy week for me at work. I'll look to clean this up on the weekend.

Cheers,
Dan

Hi Derek,

I've now cleaned this thread up. I've quoted most of the messages in the modelling-centric discussion here: DanielLoth/ShootingClubDatabase#2

Cheers,
Dan