May 1, 2020 at 7:25 pm
Many examples on the web, eg. sqlauthority show comment block before the CREATE AS
https://www.mssqltips.com/sqlservertutorial/167/using-comments-in-a-sql-server-stored-procedure/
Is this correct, or is there an argument for placing it after AS? Why?
notes: by comment block I mean this thing that we put at the top of a sproc to describe it and track modifications
/*
-this procedure gets a list of addresses based
on the city value that is passed
-this procedure is used by the HR system
*/
--Quote me
May 1, 2020 at 7:52 pm
it is a matter of opinion.
on my own opinion I enforce them to be after the create.
one of the reasons is that if I ever need to do a automated change of the code ensuring that the create proc and the proc name are at the very start is a rather good thing (and speaking for experience as I have done this recently)
one other reason - I do really hate the fact that MS treats things that are OUTSIDE the create block as being part of it.
May 1, 2020 at 8:56 pm
Frederico,
on the other post you suggested my code would actually FAIL "if you are one of those that puts comments BEFORE the create procedure command"
So now you are saying 'it's a matter of opinion'???
I would like to see additional members pitch in their KT on this one please.
--Quote me
May 1, 2020 at 9:11 pm
My preference is before partly because that is what was done before I started and partly because that is my habit from .NET development.
From a C# side, my opinion is that comment blocks should not exist inside of a function. If you need a whole comment block to describe a subset of a function, then that subset should be its own function. A comment line may exist in the function, but blocks of comments mean the code is too complicated or your comment should be simplicied.
For consistency sake, we like having our code with comments before the function to describe the function in C# and follow that pattern with tables, views, stored procedures and functions in SQL.
That is just the rule we have at work, but it doesn't mean it is some universal rule. Everything I've seen seems to indicate this is a matter of opinion, but once you pick one method, you should stick to it.
The above is all just my opinion on what you should do.
As with all advice you find on a random internet forum - you shouldn't blindly follow it. Always test on a test server to see if there is negative side effects before making changes to live!
I recommend you NEVER run "random code" you found online on any system you care about UNLESS you understand and can verify the code OR you don't care if the code trashes your system.
May 1, 2020 at 10:00 pm
Frederico,
on the other post you suggested my code would actually FAIL "if you are one of those that puts comments BEFORE the create procedure command"
So now you are saying 'it's a matter of opinion'???
I would like to see additional members pitch in their KT on this one please.
yes it is a matter of opinion
lets put this as an example.your code to change the SP does the following replacement (and making the assumption that you change your code to look at the first occurrence of CREATE to replace with ALTER
/* this proc is to create a copy of a table and alter its indexes for performance
for all intents and purposes this is an alter procedure type
*/
create procedure procname
as
begin
if object_id('tempdb..#xxx') is not null
drop table #xxx;
create table #xxx
(id int
,description varchar(200)
);
alter table #xxx add constraint pk_xxx primary key nonclustered
(id
);
end
when you do your your code (which by the way is incorrect as
select definition
from sys.sql_modules
returns a "CREATE PROC..." and not an ALTER PROC so your code
SELECT
Replace ((REPLACE(definition,'subscriptonguid','subscriptionguid')),'ALTER','create')
FROM sys.sql_modules a
isn't even doing what you intended.
in any case on the sample proc above your code would return the following (words in BOLD are the ones your code replaces)
/* this proc is to create a copy of a table and alter its indexes for performance
for all intents and purposes this is an CREATE procedure type
*/
create procedure procname
as
begin
if object_id('tempdb..#xxx') is not null
drop table #xxx;
create table #xxx
(id int
,description varchar(200)
);
CREATE table #xxx add constraint pk_xxx primary key nonclustered
(id
);
end
as you did a drop of the proc the create works - but you loose all grants on it and have to do them again
so when you try and recreate the proc above it will fail as your add primary constraint got changed to an invalid CREATE table statement
had you used the correct replacement of CREATE by ALTER (so you could keep the grants)
then the code would be
/* this proc is to create a copy of a table and alter its indexes for performance
for all intents and purposes this is an CREATE procedure type
*/
ALTER procedure procname
as
begin
if object_id('tempdb..#xxx') is not null
drop table #xxx;
ALTER table #xxx
(id int
,description varchar(200)
);
alter table #xxx add constraint pk_xxx primary key nonclustered
(id
);
end
this time it fails because your code is trying to issue an invalid ALTER table statement
had you searched for the first "create" to replace with an "ALTER" again it would fail because it would find the "CREATE procedure" within the comments - had the comments been after the create proc this would not be an issue.
so as you see there are lots of things to take in consideration - a few more even but no point in stating them as it is the wrong thing to do for a simple replacement that should be done on your source code, not directly on the database.
and apologies - my "will fail" should have been "may fail" - not that it matters
Viewing 5 posts - 1 through 4 (of 4 total)
You must be logged in to reply to this topic. Login to reply