Skip to content

Instantly share code, notes, and snippets.

@DanielLoth
Last active July 11, 2021 11:22
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save DanielLoth/a272a601e38c9c6f2c002956b08d534e to your computer and use it in GitHub Desktop.
Save DanielLoth/a272a601e38c9c6f2c002956b08d534e to your computer and use it in GitHub Desktop.
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;
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;
create function dbo.GetObjectName_fn (
@ObjectId int
)
returns nvarchar(257)
as
begin
return object_schema_name(@ObjectId) + '.' + object_name(@ObjectId);
end
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
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
@Derek-Asirvadem
Copy link

Derek-Asirvadem commented Jun 29, 2021

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

@DanielLoth
Copy link
Author

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:
image

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.

@Derek-Asirvadem
Copy link

Danno

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

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

@DanielLoth
Copy link
Author

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.

@Derek-Asirvadem
Copy link

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

@DanielLoth
Copy link
Author

DanielLoth commented Jul 4, 2021

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

@Derek-Asirvadem
Copy link

Derek-Asirvadem commented Jul 4, 2021

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

@DanielLoth
Copy link
Author

Hi Derek,

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

Cheers,
Dan

@DanielLoth
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment