September 17, 2018 at 1:09 pm
This is pretty typical SQL Code for me:
-- SET NOCOUNT ON added to prevent extra result sets from
-- interfering with SELECT statements.
SET NOCOUNT ON;
-- Insert statements for procedure here
SELECT
T1.Id,
T1.IsActive,
T1.StartDate,
T1.EndDate,
T1.Title,
T1.Body,
T2.NetId AS CreateName,
T1.CreateDate
FROM
Alert AS T1,
[User] AS T2
WHERE
T1.CreateId = T2.Id
I've been writing SQL code like this for 15 years; however, I've begun to wonder if there is a better (or more professional) way.
I don't interact with other developers so sometimes I wonder if I'm making mistakes that are glaringly obvious but I'm never told about them.
Does this look like NOOB code, any way to make it more 'professional'?
September 17, 2018 at 1:15 pm
It's fine, except I'd add in a JOIN instead of using the old-style WHERE:
SELECT
T1.Id
, T1.IsActive
, T1.StartDate
, T1.EndDate
, T1.Title
, T1.Body
, CreateName = T2.NetId
, T1.CreateDate
FROM
Alert T1
JOIN [User] T2 ON T1.CreateId = T2.Id;
I'd also suggest that you try to get into the habit of terminating your statements with a semicolon (or let SQL Prompt do it for you).
If you haven't even tried to resolve your issue, please don't expect the hard-working volunteers here to waste their time providing links to answers which you could easily have found yourself.
September 17, 2018 at 1:16 pm
thelenj - Monday, September 17, 2018 1:09 PMThis is pretty typical SQL Code for me:
-- SET NOCOUNT ON added to prevent extra result sets from
-- interfering with SELECT statements.
SET NOCOUNT ON;-- Insert statements for procedure here
SELECT
T1.Id,
T1.IsActive,
T1.StartDate,
T1.EndDate,
T1.Title,
T1.Body,
T2.NetId AS CreateName,
T1.CreateDate
FROM
Alert AS T1,
[User] AS T2
WHERE
T1.CreateId = T2.IdI've been writing SQL code like this for 15 years; however, I've begun to wonder if there is a better (or more professional) way.
I don't interact with other developers so sometimes I wonder if I'm making mistakes that are glaringly obvious but I'm never told about them.
Does this look like NOOB code, any way to make it more 'professional'?
If you're going to use a comma to make a join and a WHERE condition to enforce it, you're better off using an INNER JOIN, and let the condition in the WHERE clause become the JOIN condition as the ON clause instead. Also, you could indent the SELECTed columns, as well as any WHERE clause conditions, to assist with readability, but there's nothing about this query that screams NOOB... You did aliase your columns and tables, and the lack of that is rather common in NOOB queries. Keep stopping in at this site and read the posts and answers, and you may learn quite a bit from that.
Steve (aka sgmunson) 🙂 🙂 🙂
Rent Servers for Income (picks and shovels strategy)
September 17, 2018 at 1:21 pm
Phil Parkin - Monday, September 17, 2018 1:15 PMIt's fine, except I'd add in a JOIN instead of using the old-style WHERE:
SELECT
T1.Id
, T1.IsActive
, T1.StartDate
, T1.EndDate
, T1.Title
, T1.Body
, CreateName = T2.NetId
, T1.CreateDate
FROM
Alert T1
JOIN [User] T2 ON T1.CreateId = T2.Id;I'd also suggest that you try to get into the habit of terminating your statements with a semicolon (or let SQL Prompt do it for you).
Does using the JOIN statement help with speed?
I find your formatting to be interesting, is this common?
Thanks for the input.
September 17, 2018 at 1:21 pm
sgmunson - Monday, September 17, 2018 1:16 PMthelenj - Monday, September 17, 2018 1:09 PMThis is pretty typical SQL Code for me:
-- SET NOCOUNT ON added to prevent extra result sets from
-- interfering with SELECT statements.
SET NOCOUNT ON;-- Insert statements for procedure here
SELECT
T1.Id,
T1.IsActive,
T1.StartDate,
T1.EndDate,
T1.Title,
T1.Body,
T2.NetId AS CreateName,
T1.CreateDate
FROM
Alert AS T1,
[User] AS T2
WHERE
T1.CreateId = T2.IdI've been writing SQL code like this for 15 years; however, I've begun to wonder if there is a better (or more professional) way.
I don't interact with other developers so sometimes I wonder if I'm making mistakes that are glaringly obvious but I'm never told about them.
Does this look like NOOB code, any way to make it more 'professional'?If you're going to use a comma to make a join and a WHERE condition to enforce it, you're better off using an INNER JOIN, and let the condition in the WHERE clause become the JOIN condition as the ON clause instead. Also, you could indent the SELECTed columns, as well as any WHERE clause conditions, to assist with readability, but there's nothing about this query that screams NOOB... You did aliase your columns and tables, and the lack of that is rather common in NOOB queries. Keep stopping in at this site and read the posts and answers, and you may learn quite a bit from that.
Thank you - two calls to indent and use JOIN - so I'll start doing that.
September 17, 2018 at 1:32 pm
Using JOIN won't make it faster but it does make it more understandable as you use join to join and where to filter. It's also the standard now.
September 17, 2018 at 1:34 pm
No, the ANSI-92 style joins aren't faster than the ANSI-89 style. What it does is help separate the joins that create the full result set from the filter conditions in the WHERE clause. Aids in readability.
September 17, 2018 at 1:36 pm
Good, I like the explaining of things. This helps.
September 17, 2018 at 1:40 pm
thelenj - Monday, September 17, 2018 1:21 PMPhil Parkin - Monday, September 17, 2018 1:15 PMIt's fine, except I'd add in a JOIN instead of using the old-style WHERE:
SELECT
T1.Id
, T1.IsActive
, T1.StartDate
, T1.EndDate
, T1.Title
, T1.Body
, CreateName = T2.NetId
, T1.CreateDate
FROM
Alert T1
JOIN [User] T2 ON T1.CreateId = T2.Id;I'd also suggest that you try to get into the habit of terminating your statements with a semicolon (or let SQL Prompt do it for you).
Does using the JOIN statement help with speed?
I find your formatting to be interesting, is this common?
Thanks for the input.
You may well find that formatting is somewhat personal in terms of preferences. I happen to hate having the commas at the beginning of a line. I also prefer to be specific about the JOIN that I want to use, so I'll never use just JOIN. It will always be INNER JOIN, LEFT JOIN, or FULL OUTER JOIN for me. And I happen to set my tabs to equal 4 spaces in SSMS. I also indent my JOINs. and an additional indent for the ON clause as well. On SELECT, GROUP BY, and ORDER BY, the list of columns always gets started on the next line and indented, and I never use an equal sign to assign a column name, and I always use AS SomeColumnName for that purpose. I also insist on making data types as all lower case, and all SQL keywords capitalized. Not everyone is going to agree with my preferences, or yours, but these kinds of things tend to make code a LOT more readable and easy on the eyes.
Steve (aka sgmunson) 🙂 🙂 🙂
Rent Servers for Income (picks and shovels strategy)
September 17, 2018 at 2:02 pm
sgmunson - Monday, September 17, 2018 1:40 PMthelenj - Monday, September 17, 2018 1:21 PMPhil Parkin - Monday, September 17, 2018 1:15 PMIt's fine, except I'd add in a JOIN instead of using the old-style WHERE:
SELECT
T1.Id
, T1.IsActive
, T1.StartDate
, T1.EndDate
, T1.Title
, T1.Body
, CreateName = T2.NetId
, T1.CreateDate
FROM
Alert T1
JOIN [User] T2 ON T1.CreateId = T2.Id;I'd also suggest that you try to get into the habit of terminating your statements with a semicolon (or let SQL Prompt do it for you).
Does using the JOIN statement help with speed?
I find your formatting to be interesting, is this common?
Thanks for the input.You may well find that formatting is somewhat personal in terms of preferences. I happen to hate having the commas at the beginning of a line. I also prefer to be specific about the JOIN that I want to use, so I'll never use just JOIN. It will always be INNER JOIN, LEFT JOIN, or FULL OUTER JOIN for me. And I happen to set my tabs to equal 4 spaces in SSMS. I also indent my JOINs. and an additional indent for the ON clause as well. On SELECT, GROUP BY, and ORDER BY, the list of columns always gets started on the next line and indented, and I never use an equal sign to assign a column name, and I always use AS SomeColumnName for that purpose. I also insist on making data types as all lower case, and all SQL keywords capitalized. Not everyone is going to agree with my preferences, or yours, but these kinds of things tend to make code a LOT more readable and easy on the eyes.
I go even further on the joins, INNER JOIN, LEFT OUTER JOIN, RIGHT OUTER JOIN, FULL OUTER JOIN. Always use table aliases in the FROM clause and use those to explicitly identify which tables columns are in EVEN IF the column names are uniquely named. I used to insist on the commas at the end but have switched since I started using SQL Prompt and finding that I rarely comment out the first column in a SELECT list.
The bottom line on formatting, be consistent.
September 17, 2018 at 3:02 pm
sgmunson - Monday, September 17, 2018 1:40 PMthelenj - Monday, September 17, 2018 1:21 PMPhil Parkin - Monday, September 17, 2018 1:15 PMIt's fine, except I'd add in a JOIN instead of using the old-style WHERE:
SELECT
T1.Id
, T1.IsActive
, T1.StartDate
, T1.EndDate
, T1.Title
, T1.Body
, CreateName = T2.NetId
, T1.CreateDate
FROM
Alert T1
JOIN [User] T2 ON T1.CreateId = T2.Id;I'd also suggest that you try to get into the habit of terminating your statements with a semicolon (or let SQL Prompt do it for you).
Does using the JOIN statement help with speed?
I find your formatting to be interesting, is this common?
Thanks for the input.You may well find that formatting is somewhat personal in terms of preferences. I happen to hate having the commas at the beginning of a line. I also prefer to be specific about the JOIN that I want to use, so I'll never use just JOIN. It will always be INNER JOIN, LEFT JOIN, or FULL OUTER JOIN for me. And I happen to set my tabs to equal 4 spaces in SSMS. I also indent my JOINs. and an additional indent for the ON clause as well. On SELECT, GROUP BY, and ORDER BY, the list of columns always gets started on the next line and indented, and I never use an equal sign to assign a column name, and I always use AS SomeColumnName for that purpose. I also insist on making data types as all lower case, and all SQL keywords capitalized. Not everyone is going to agree with my preferences, or yours, but these kinds of things tend to make code a LOT more readable and easy on the eyes.
I loathe commas at the start, because then you always have to read the next line to see if you're done with the current column definition. That's a royal pita when you're dealing with a query where many/most columns are defined using multiple lines ... and even worse when it's just the one column you're working on, and you didn't notice it, and you coded in the middle of the existing column def.
SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.
September 17, 2018 at 3:24 pm
100% not using a JOIN clause screams "noob".
But I see many more issues.
A serious one is not including a schema name before the table names. You would only leave off schema in the (hopefully rare) instances where you wanted different people to see different tables based on their own default schema (a rather rare technique, nowadays, except perhaps for certain vendors(?)).
While certainly not a noob issue (most long-term developers don't add them to their code), the lack of the correct SET statements are also major and are added in the code below. The SETs in the proc are required for certain "advanced" indexes, among them filtered indexes. Your DBA will appreciate you adding the SETs -- because his/her index tuning won't go to waste -- and you'll appreciate it, because your code won't crash at some point after the DBA adds a filtered index. For my own ease of use, I put the SETs in alpha order.
SET XACT_ABORT ON has some serious implications, so you'll want to check with in-your-shop experts before using that one. In fact, ditto for the SET statements in general. But they are the future, so you might as well start preparing for them now.
A final, minor point, get rid of the comment lines regarding NOCOUNT and "--Insert statements for procedure here" -- only a noob would need them!
I often leave a blank line, or a '--', before the FROM statement if the list of columns is long (or even kinda long), but of course that's just a preference and not a noob vs non-noob thing.
SET ANSI_NULLS ON;
SET QUOTED_IDENTIFIER ON;
GO
CREATE PROCEDURE dbo.proc_name
@param_1 ...
AS
SET ANSI_PADDING ON;
SET ANSI_WARNINGS ON;
SET ARITHABORT ON;
SET CONCAT_NULL_YIELDS_NULL ON;
SET NOCOUNT ON;
SET NUMERIC_ROUNDABORT OFF;
SET XACT_ABORT ON;
SELECT
T1.Id,
T1.IsActive,
T1.StartDate,
T1.EndDate,
T1.Title,
T1.Body,
T2.NetId AS CreateName,
T1.CreateDate
FROM dbo.Alert AS T1
INNER JOIN dbo.[User] AS T2 ON T1.CreateId = T2.Id
/*End of proc: "dbo.proc_name"*/
SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.
September 18, 2018 at 7:11 am
ScottPletcher - Monday, September 17, 2018 3:24 PM100% not using a JOIN clause screams "noob".But I see many more issues.
A serious one is not including a schema name before the table names. You would only leave off schema in the (hopefully rare) instances where you wanted different people to see different tables based on their own default schema (a rather rare technique, nowadays, except perhaps for certain vendors(?)).
While certainly not a noob issue (most long-term developers don't add them to their code), the lack of the correct SET statements are also major and are added in the code below. The SETs in the proc are required for certain "advanced" indexes, among them filtered indexes. Your DBA will appreciate you adding the SETs -- because his/her index tuning won't go to waste -- and you'll appreciate it, because your code won't crash at some point after the DBA adds a filtered index. For my own ease of use, I put the SETs in alpha order.
SET XACT_ABORT ON has some serious implications, so you'll want to check with in-your-shop experts before using that one. In fact, ditto for the SET statements in general. But they are the future, so you might as well start preparing for them now.
A final, minor point, get rid of the comment lines regarding NOCOUNT and "--Insert statements for procedure here" -- only a noob would need them!
I often leave a blank line, or a '--', before the FROM statement if the list of columns is long (or even kinda long), but of course that's just a preference and not a noob vs non-noob thing.
SET ANSI_NULLS ON;
SET QUOTED_IDENTIFIER ON;
GO
CREATE PROCEDURE dbo.proc_name
@param_1 ...
AS
SET ANSI_PADDING ON;
SET ANSI_WARNINGS ON;
SET ARITHABORT ON;
SET CONCAT_NULL_YIELDS_NULL ON;
SET NOCOUNT ON;
SET NUMERIC_ROUNDABORT OFF;
SET XACT_ABORT ON;SELECT
T1.Id,
T1.IsActive,
T1.StartDate,
T1.EndDate,
T1.Title,
T1.Body,
T2.NetId AS CreateName,
T1.CreateDateFROM dbo.Alert AS T1
INNER JOIN dbo.[User] AS T2 ON T1.CreateId = T2.Id/*End of proc: "dbo.proc_name"*/
Shouldn't those SET options be specified against the database and then left out of the stored procedure so that it takes the database defaults? Otherwise there'd be the probability of some stored procedures using different settings and therefore having behaviour that could cause confusion.
September 18, 2018 at 7:49 am
Chris Wooding - Tuesday, September 18, 2018 7:11 AMScottPletcher - Monday, September 17, 2018 3:24 PM100% not using a JOIN clause screams "noob".But I see many more issues.
A serious one is not including a schema name before the table names. You would only leave off schema in the (hopefully rare) instances where you wanted different people to see different tables based on their own default schema (a rather rare technique, nowadays, except perhaps for certain vendors(?)).
While certainly not a noob issue (most long-term developers don't add them to their code), the lack of the correct SET statements are also major and are added in the code below. The SETs in the proc are required for certain "advanced" indexes, among them filtered indexes. Your DBA will appreciate you adding the SETs -- because his/her index tuning won't go to waste -- and you'll appreciate it, because your code won't crash at some point after the DBA adds a filtered index. For my own ease of use, I put the SETs in alpha order.
SET XACT_ABORT ON has some serious implications, so you'll want to check with in-your-shop experts before using that one. In fact, ditto for the SET statements in general. But they are the future, so you might as well start preparing for them now.
A final, minor point, get rid of the comment lines regarding NOCOUNT and "--Insert statements for procedure here" -- only a noob would need them!
I often leave a blank line, or a '--', before the FROM statement if the list of columns is long (or even kinda long), but of course that's just a preference and not a noob vs non-noob thing.
SET ANSI_NULLS ON;
SET QUOTED_IDENTIFIER ON;
GO
CREATE PROCEDURE dbo.proc_name
@param_1 ...
AS
SET ANSI_PADDING ON;
SET ANSI_WARNINGS ON;
SET ARITHABORT ON;
SET CONCAT_NULL_YIELDS_NULL ON;
SET NOCOUNT ON;
SET NUMERIC_ROUNDABORT OFF;
SET XACT_ABORT ON;SELECT
T1.Id,
T1.IsActive,
T1.StartDate,
T1.EndDate,
T1.Title,
T1.Body,
T2.NetId AS CreateName,
T1.CreateDateFROM dbo.Alert AS T1
INNER JOIN dbo.[User] AS T2 ON T1.CreateId = T2.Id/*End of proc: "dbo.proc_name"*/
Shouldn't those SET options be specified against the database and then left out of the stored procedure so that it takes the database defaults? Otherwise there'd be the probability of some stored procedures using different settings and therefore having behaviour that could cause confusion.
As coded the SET options are outside of the stored procedure. The stored procedure uses the setting as set at the time it is created, iirc.
September 18, 2018 at 8:00 am
Chris Wooding - Tuesday, September 18, 2018 7:11 AMScottPletcher - Monday, September 17, 2018 3:24 PM100% not using a JOIN clause screams "noob".But I see many more issues.
A serious one is not including a schema name before the table names. You would only leave off schema in the (hopefully rare) instances where you wanted different people to see different tables based on their own default schema (a rather rare technique, nowadays, except perhaps for certain vendors(?)).
While certainly not a noob issue (most long-term developers don't add them to their code), the lack of the correct SET statements are also major and are added in the code below. The SETs in the proc are required for certain "advanced" indexes, among them filtered indexes. Your DBA will appreciate you adding the SETs -- because his/her index tuning won't go to waste -- and you'll appreciate it, because your code won't crash at some point after the DBA adds a filtered index. For my own ease of use, I put the SETs in alpha order.
SET XACT_ABORT ON has some serious implications, so you'll want to check with in-your-shop experts before using that one. In fact, ditto for the SET statements in general. But they are the future, so you might as well start preparing for them now.
A final, minor point, get rid of the comment lines regarding NOCOUNT and "--Insert statements for procedure here" -- only a noob would need them!
I often leave a blank line, or a '--', before the FROM statement if the list of columns is long (or even kinda long), but of course that's just a preference and not a noob vs non-noob thing.
SET ANSI_NULLS ON;
SET QUOTED_IDENTIFIER ON;
GO
CREATE PROCEDURE dbo.proc_name
@param_1 ...
AS
SET ANSI_PADDING ON;
SET ANSI_WARNINGS ON;
SET ARITHABORT ON;
SET CONCAT_NULL_YIELDS_NULL ON;
SET NOCOUNT ON;
SET NUMERIC_ROUNDABORT OFF;
SET XACT_ABORT ON;SELECT
T1.Id,
T1.IsActive,
T1.StartDate,
T1.EndDate,
T1.Title,
T1.Body,
T2.NetId AS CreateName,
T1.CreateDateFROM dbo.Alert AS T1
INNER JOIN dbo.[User] AS T2 ON T1.CreateId = T2.Id/*End of proc: "dbo.proc_name"*/
Shouldn't those SET options be specified against the database and then left out of the stored procedure so that it takes the database defaults? Otherwise there'd be the probability of some stored procedures using different settings and therefore having behaviour that could cause confusion.
Unless not every SP in the database expects the same settings for some of those things.
Viewing 15 posts - 1 through 15 (of 33 total)
You must be logged in to reply to this topic. Login to reply