September 14, 2016 at 3:47 pm
In my experience, putting the 'ON' clause on a new line is more of an 'old school' habit going back to the days when you could only fit 80 characters on each line :hehe:
In the old school habits for the database world, there would be no ON clause used at all. It was a matter of trying to figure out the joins vs conditions in the huge mess of a where clause.
Sue
September 15, 2016 at 2:16 am
Sue_H (9/14/2016)
In the old school habits for the database world, there would be no ON clause used at all. It was a matter of trying to figure out the joins vs conditions in the huge mess of a where clause.
Sue
Very true: there was some terrible code floating around in those days.
To make things as readable as possible, I used to insist that the tables in the FROM clause appeared in 'join' order; the WHERE clause joined the tables in the same order; that each join condition was on a separate line and that each was aligned with the ones above it.
September 15, 2016 at 6:13 am
Luis Cazares (9/7/2016)
colin.dunn (9/7/2016)
Put the join ON clauses on the same line since they were so short.
I usually have my ON clauses on the same line as my JOIN, but when he was testing it, he changed it to the next line.
<rant>
I don't like when people put the ON clauses in separate lines. It feels as if they weren't part of the JOIN. It seems like they still want to write code with the SQL-86 standard, where the tables and join conditions were set apart.
</rant>
That's one of the reasons for your DBA to tell you what means clunky to him/her. However, don't hesitate to come back for guidance. 😉
And I feel just the opposite, Luis. I can't read a JOIN line when the ON clause is on the same physical line. I do, however, tend to indent the ON with a space or two to make it clear to me.
September 15, 2016 at 6:17 am
The Dixie Flatline (9/7/2016)
Did he say it looks clunky or it runs clunky?I think your DBA is being really picky unless you have company standards for formatting, but.
SELECT d.WORK_TYPE AS [Work Type],CONVERT(varchar, a.ACT_DATE, 103) AS [Date Opened],
CONVERT(varchar, a.ACT_DATE, 108) AS [Time Opened],
b.IT_IDEN AS [Item ID],
CONVERT(varchar, @checkDate, 103) AS [Run Date],
a.CUSTID AS [Customer Number],
CONVERT(varchar, ACT_CLOSE, 103) AS [Close Date],
DATEDIFF(day, a.ACT_DATE, CAST(checkDate AS DATE)) AS [Days Elapsed],
a.PERSON AS [Started By],
c.HANDOFF AS [Assigned To],
a.TYPE_STAT AS [Status Code]
FROM main.SOURCE_DATA a
JOIN main.LINKED_DATA bON a.ID=b.ID
JOIN main.LOOK_UP cON a.ID=c.ID
JOIN main.LOOK_UP_TYPE d ON a.ID_TYPE=d.ID_TYPE
WHERE a.ACT_DATE >= CONVERT(DATE,DATEADD(day, DATEDIFF(day, 1, GETDATE()), 0))
OR a.ACT_CLOSE IS NULL
ORDER BY [Days Elapsed] DESC, [Work Type]
Got rid of your variable declaration and just pit the conversion code in the WHERE clause
So I read through the entire post to verify no one had responded to this one...
I have to disagree with the notion of replacing the variable declaration. Depending on the situation, it is possibly easier to process all those date functions and converts only once for the variable and not have to process it each time the query grabs a row.
Yes I know this is set-based code, but the WHERE clause does sometimes (again, depending on the situation and how the DB is set up) process those functions multiple times. Though I don't have an example off the top of my head to prove the point...
September 15, 2016 at 6:21 am
Fast.Eddie (9/7/2016)
Good Afternoon All, This is my first post.I created the below code and if works fine. However my DBA said "It does the job but is clunky" how could this be re-written so it's not "clunky"?
USE TESTDBGO
DECLARE @checkDate DATE
SET @checkDate = DATEADD(day, DATEDIFF(day, 1, GETDATE()), 0)
SELECT d.WORK_TYPE AS 'Work Type',
CONVERT(varchar, a.ACT_DATE, 103) AS 'Date Opened',
CONVERT(varchar, a.ACT_DATE, 108) AS 'Time Opened',
b.IT_IDEN AS 'Item ID',
CONVERT(varchar, @checkDate, 103) AS 'Run Date',
a.CUSTID AS 'Customer Number',
CONVERT(varchar, ACT_CLOSE, 103) AS 'Close Date',
DATEDIFF(day, a.ACT_DATE, CAST(checkDate AS DATE)) AS 'Days Elapsed',
a.PERSON AS 'Started By',
c.HANDOFF AS 'Assigned To',
a.TYPE_STAT AS 'Status Code'
FROM main.SOURCE_DATA a
JOIN main.LINKED_DATA b
ON a.ID=b.ID
JOIN main.LOOK_UP c
ON a.ID=c.ID
JOIN main.LOOK_UP_TYPE d
ON a.ID_TYPE=d.ID_TYPE
WHERE CAST(a.ACT_DATE AS DATE) >= @checkDate OR a.ACT_CLOSE IS NULL
ORDER BY 8 DESC,1
As a last comment on your code, some companies / DBAs prefer it when JOINs are better defined. I.E., instead of JOIN, use INNER JOIN and then either LEFT OUTER JOIN or LEFT JOIN and either RIGHT OUTER JOIN or RIGHT JOIN, just so everyone coming to examine the code after you knows you deliberately were doing an INNER JOIN as opposed to an OUTER JOIN where you may have forgotten to put in the direction of the JOIN.
This is more of a semantics, but it's something that we preach in our office. As well as making our devs use "AS <alias>" in the SELECT list for the column aliases so we know they didn't accidentally forget a comma in the SELECT list and force one column to take the name of a following column as an alias. (This happens more then you might think).
We don't use AS in the table aliases, though, because that's usually pretty clear.
September 15, 2016 at 6:58 am
Brandie Tarvin (9/15/2016)
Fast.Eddie (9/7/2016)
Good Afternoon All, This is my first post.I created the below code and if works fine. However my DBA said "It does the job but is clunky" how could this be re-written so it's not "clunky"?
USE TESTDBGO
DECLARE @checkDate DATE
SET @checkDate = DATEADD(day, DATEDIFF(day, 1, GETDATE()), 0)
SELECT d.WORK_TYPE AS 'Work Type',
CONVERT(varchar, a.ACT_DATE, 103) AS 'Date Opened',
CONVERT(varchar, a.ACT_DATE, 108) AS 'Time Opened',
b.IT_IDEN AS 'Item ID',
CONVERT(varchar, @checkDate, 103) AS 'Run Date',
a.CUSTID AS 'Customer Number',
CONVERT(varchar, ACT_CLOSE, 103) AS 'Close Date',
DATEDIFF(day, a.ACT_DATE, CAST(checkDate AS DATE)) AS 'Days Elapsed',
a.PERSON AS 'Started By',
c.HANDOFF AS 'Assigned To',
a.TYPE_STAT AS 'Status Code'
FROM main.SOURCE_DATA a
JOIN main.LINKED_DATA b
ON a.ID=b.ID
JOIN main.LOOK_UP c
ON a.ID=c.ID
JOIN main.LOOK_UP_TYPE d
ON a.ID_TYPE=d.ID_TYPE
WHERE CAST(a.ACT_DATE AS DATE) >= @checkDate OR a.ACT_CLOSE IS NULL
ORDER BY 8 DESC,1
As a last comment on your code, some companies / DBAs prefer it when JOINs are better defined. I.E., instead of JOIN, use INNER JOIN and then either LEFT OUTER JOIN or LEFT JOIN and either RIGHT OUTER JOIN or RIGHT JOIN, just so everyone coming to examine the code after you knows you deliberately were doing an INNER JOIN as opposed to an OUTER JOIN where you may have forgotten to put in the direction of the JOIN.
This is more of a semantics, but it's something that we preach in our office. As well as making our devs use "AS <alias>" in the SELECT list for the column aliases so we know they didn't accidentally forget a comma in the SELECT list and force one column to take the name of a following column as an alias. (This happens more then you might think).
We don't use AS in the table aliases, though, because that's usually pretty clear.
My opinion is that if you have SQL devs who confuse JOIN, LEFT JOIN and RIGHT JOIN, they are not fit for the job and should be further trained or reassigned.
Using AS in column aliases is just a ton of extra white noise which (again, my opinion) causes clutter and makes code harder to read.
And rather than this format:
Select
Col1 SomeCol,
Column2 SomeOtherCol,
ThisOtherCol SomeCol3,
I find this much easier to scan
Select
SomeCol = Col1,
SomeOtherCol = Column2,
SomeCol3 = ThisOtherCol
But every developer has their own take on this ...
The absence of evidence is not evidence of absence
- Martin Rees
The absence of consumable DDL, sample data and desired results is, however, evidence of the absence of my response
- Phil Parkin
September 15, 2016 at 7:37 am
Please note I'm looking at the layout, not the detail. Here's a suggestion:
USE TESTDB
GO
DECLARE @checkDate DATE = DATEADD(day, DATEDIFF(day, 1, GETDATE()), 0);
SELECT[Work Type]= d.WORK_TYPE,
[Date Opened]= CONVERT(varchar, a.ACT_DATE, 103),
[Time Opened]= CONVERT(varchar, a.ACT_DATE, 108),
[Item ID]= b.IT_IDEN,
[Run Date]= CONVERT(varchar, @checkDate, 103),
[Customer Number]= a.CUSTID,
[Close Date]= CONVERT(varchar, ACT_CLOSE, 103),
[Days Elapsed]= DATEDIFF(day, a.ACT_DATE, CAST(checkDate AS DATE)),
[Started By]= a.PERSON,
[Assigned To]= c.HANDOFF,
[Status Code]= a.TYPE_STAT
FROM main.SOURCE_DATAa
JOIN main.LINKED_DATAb ON a.ID= b.ID
JOIN main.LOOK_UPc ON a.ID= c.ID
JOIN main.LOOK_UP_TYPEd ON a.ID_TYPE= d.ID_TYPE
WHERE CAST(a.ACT_DATE AS DATE) >= @checkDate
OR a.ACT_CLOSE IS NULL
ORDER BY 8 DESC,1;
You'll see you don't have to parse the code to get a list of field names (I put them first because the definitions tend to be longer and more variable) or the table aliases - they're all underneath each other, as are the aliases they reference.
I know it's not perfect, and we'll never get total agreement, but particularly when taking on someone else's code, I find the above makes things (particularly bugs) a lot clearer.
September 15, 2016 at 11:06 am
We don't use AS in the table aliases, though, because that's usually pretty clear.
I find it clearer if AS is used consistently when aliasing anywhere. If I can apply your reasoning here on spelling out joins, a "b" could be a errant left over character that was not meant to be there. AS, including the color coding, is easier on the eyes.
A pet peeve of mine is when the columns used in the select clause as the result of joins are not prefixed. You may know that someColumn came from TableC and only exists in that table. But I don't. I like to see this entered as c.someColumn consistently for all columns.
----------------------------------------------------
September 15, 2016 at 11:18 am
MMartin1 (9/15/2016)
We don't use AS in the table aliases, though, because that's usually pretty clear.
I find it clearer if AS is used consistently when aliasing anywhere. If I can apply your reasoning here on spelling out joins, a "b" could be a errant left over character that was not meant to be there. AS, including the color coding, is easier on the eyes.
A pet peeve of mine is when the columns used in the select clause as the result of joins are not prefixed. You may know that someColumn came from TableC and only exists in that table. But I don't. I like to see this entered as c.someColumn consistently for all columns.
I didn't used to use 'AS', but I've since been persuaded by the same reasoning.
I also agree that every column reference should be prefixed by the alias, except perhaps where there is only one table. But SQL Prompt puts those in for me anyway, so it doesn't happen 😛
September 15, 2016 at 11:35 am
I don't think I've ever done a RIGHT JOIN in production. :ermm:
__________________________________________________
Against stupidity the gods themselves contend in vain. -- Friedrich Schiller
Stop, children, what's that sound? Everybody look what's going down. -- Stephen Stills
September 15, 2016 at 11:52 am
MMartin1 (9/15/2016)
A pet peeve of mine is when the columns used in the select clause as the result of joins are not prefixed. You may know that someColumn came from TableC and only exists in that table. But I don't. I like to see this entered as c.someColumn consistently for all columns.
Preach it!
+1
September 15, 2016 at 11:55 am
MMartin1 (9/15/2016)
We don't use AS in the table aliases, though, because that's usually pretty clear.
I find it clearer if AS is used consistently when aliasing anywhere. If I can apply your reasoning here on spelling out joins, a "b" could be a errant left over character that was not meant to be there. AS, including the color coding, is easier on the eyes.
A pet peeve of mine is when the columns used in the select clause as the result of joins are not prefixed. You may know that someColumn came from TableC and only exists in that table. But I don't. I like to see this entered as c.someColumn consistently for all columns.
Easier on your eyes. For me, 'as' in aliases is just extra noise that my brain needs to filter out.
I agree about object qualification, however. SQL Prompt takes care of that, in cases where it is missing.
The absence of evidence is not evidence of absence
- Martin Rees
The absence of consumable DDL, sample data and desired results is, however, evidence of the absence of my response
- Phil Parkin
September 15, 2016 at 11:56 am
The Dixie Flatline (9/15/2016)
I don't think I've ever done a RIGHT JOIN in production. :ermm:
I have a power user who does it all the time because he doesn't seem to get it's easier in a "reading left to right" culture to just put tables in that order.
September 15, 2016 at 11:58 am
The Dixie Flatline (9/15/2016)
I don't think I've ever done a RIGHT JOIN in production. :ermm:
Ditto... Keep the "left table" to the left...
September 16, 2016 at 8:25 am
Phil Parkin (9/15/2016)
Brandie Tarvin (9/15/2016)
Fast.Eddie (9/7/2016)
Good Afternoon All, This is my first post.I created the below code and if works fine. However my DBA said "It does the job but is clunky" how could this be re-written so it's not "clunky"?
USE TESTDBGO
DECLARE @checkDate DATE
SET @checkDate = DATEADD(day, DATEDIFF(day, 1, GETDATE()), 0)
SELECT d.WORK_TYPE AS 'Work Type',
CONVERT(varchar, a.ACT_DATE, 103) AS 'Date Opened',
CONVERT(varchar, a.ACT_DATE, 108) AS 'Time Opened',
b.IT_IDEN AS 'Item ID',
CONVERT(varchar, @checkDate, 103) AS 'Run Date',
a.CUSTID AS 'Customer Number',
CONVERT(varchar, ACT_CLOSE, 103) AS 'Close Date',
DATEDIFF(day, a.ACT_DATE, CAST(checkDate AS DATE)) AS 'Days Elapsed',
a.PERSON AS 'Started By',
c.HANDOFF AS 'Assigned To',
a.TYPE_STAT AS 'Status Code'
FROM main.SOURCE_DATA a
JOIN main.LINKED_DATA b
ON a.ID=b.ID
JOIN main.LOOK_UP c
ON a.ID=c.ID
JOIN main.LOOK_UP_TYPE d
ON a.ID_TYPE=d.ID_TYPE
WHERE CAST(a.ACT_DATE AS DATE) >= @checkDate OR a.ACT_CLOSE IS NULL
ORDER BY 8 DESC,1
As a last comment on your code, some companies / DBAs prefer it when JOINs are better defined. I.E., instead of JOIN, use INNER JOIN and then either LEFT OUTER JOIN or LEFT JOIN and either RIGHT OUTER JOIN or RIGHT JOIN, just so everyone coming to examine the code after you knows you deliberately were doing an INNER JOIN as opposed to an OUTER JOIN where you may have forgotten to put in the direction of the JOIN.
This is more of a semantics, but it's something that we preach in our office. As well as making our devs use "AS <alias>" in the SELECT list for the column aliases so we know they didn't accidentally forget a comma in the SELECT list and force one column to take the name of a following column as an alias. (This happens more then you might think).
We don't use AS in the table aliases, though, because that's usually pretty clear.
My opinion is that if you have SQL devs who confuse JOIN, LEFT JOIN and RIGHT JOIN, they are not fit for the job and should be further trained or reassigned.
Using AS in column aliases is just a ton of extra white noise which (again, my opinion) causes clutter and makes code harder to read.
And rather than this format:
SelectCol1 SomeCol,
Column2 SomeOtherCol,
ThisOtherCol SomeCol3,
I find this much easier to scan
SelectSomeCol = Col1,
SomeOtherCol = Column2,
SomeCol3 = ThisOtherCol
But every developer has their own take on this ...
As a DBA, I strongly prefer that you always code "INNER JOIN" and not just JOIN. Otherwise, if I need to test adding a join hint, I have to add the "INNER" myself. "INNER" is required when adding a hint for the join type.
SQL DBA,SQL Server MVP(07, 08, 09) "It's a dog-eat-dog world, and I'm wearing Milk-Bone underwear." "Norm", on "Cheers". Also from "Cheers", from "Carla": "You need to know 3 things about Tortelli men: Tortelli men draw women like flies; Tortelli men treat women like flies; Tortelli men's brains are in their flies".
Viewing 15 posts - 31 through 44 (of 44 total)
You must be logged in to reply to this topic. Login to reply