# SQL - How's my formatting?

I'm somewhat new to SQL (using it in the past but only being exposed to it heavily in my current role). Unfortunately nobody at my current company has really given me any advice on formatting. How can I format this better / what should I avoid doing that I may have done below? Any feedback on re-factoring would be greatly appreciated also.

/* Uncomment when testing this SQL */
declare @period_id as int = 252 -- ### Potentially redundant ###
declare @building_id as varchar(10) = '20500'
declare @section_name as varchar(20) = 'ALL'
declare @lease_expiry_period as int = 1;

/* SQL for Section_Name parameter on report */
select   'ALL',
1 as order_by
union all
select distinct upper(isnull(section_name, '')) as section_name,
2 as order_by
from     property.lease_period
where    section_name <> ''
order by 2;

declare @truncateddate as date = dateadd(d,0,datediff(d,0,getdate()));

/* SQL for Report (dsDetail) */
select
lp.period_id ,
lp.building_id ,
lp.suite_id ,
lp.lease_id ,
lp.suite_status ,
lp.suite_name ,
lp.suite_area,
lp.lease_status ,
lp.lease_current_start_date ,
lp.lease_current_stop_date ,
lp.lease_current_term ,
lp.lease_occupancy_date ,
lp.lease_vacated_date
from
property.lease_period lp
where
lp.lease_current_stop_date < @truncateddate
and (upper(lp.lease_status) = 'ACTIVE' or upper(lp.lease_status) = 'OVERHOLDING')
--and lp.period_id = @period_id -- ##Potentially redundant (query now running into the future)
and lp.building_id = @building_id
and not exists
(
select  1
from    lease_deal.lease l
where   lp.building_id = l.building_id
and lp.lease_id = l.lease_id
)
and (@section_name = 'ALL'
or (@section_name <> 'ALL' and upper(section_name) = upper(@section_name)))
and lp.lease_current_stop_date between @truncateddate
order by period_id desc

• FYI, Unless you have a persisted calculated column for upper(lp.lease_status) and upper(lp.section_name), you may want to research the impact of using a function within your where clause. Check out the following SO article for more details: stackoverflow.com/q/799584/5685 – Jeff Dec 30 '11 at 15:44

What you've got is pretty readable, but there are a few points to make:

1. Your sub-query is written with a different indentation style from the main query.
2. I prefer to see the keywords in upper-case.
3. Personally, I dislike spaces before commas intensely (and you aren't 100% consistent about adding them). However, I loathe even more the style where the comma is put at the start of the next line:

     item1
, item2
, item3

4. I'd use explicit AS to introduce table aliases. At least on the DBMS I use mainly, using AS gives a degree of future-proofing against aliases that become keywords in a later version of the product. That is, writing FROM SomeTable st runs into a problem later if st becomes a keyword, but writing FROM SomeTable AS st avoids that problem. Whether this helps in other DBMS is open to debate.

For your query, I'd probably write:

SELECT lp.period_id,
lp.building_id,
lp.suite_id,
lp.lease_id,
lp.suite_status,
lp.suite_name,
lp.suite_area,
lp.lease_status,
lp.lease_current_start_date,
lp.lease_current_stop_date,
lp.lease_current_term,
lp.lease_occupancy_date,
lp.lease_vacated_date
FROM property.lease_period AS lp
WHERE lp.lease_current_stop_date < @truncateddate
AND (UPPER(lp.lease_status) = 'ACTIVE' OR UPPER(lp.lease_status) = 'OVERHOLDING')
AND lp.building_id = @building_id
AND NOT EXISTS
(SELECT *
FROM lease_deal.lease AS l
WHERE lp.building_id = l.building_id
AND lp.lease_id = l.lease_id
)
AND (@section_name = 'ALL'
OR (@section_name <> 'ALL' AND UPPER(section_name) = UPPER(@section_name)))
AND lp.lease_current_stop_date BETWEEN @truncateddate AND
ORDER BY period_id DESC;


Sometimes, I'd indent the sub-query more than I did here, occasionally even placing it after the EXISTS:

   AND NOT EXISTS (SELECT *
FROM lease_deal.lease AS l
WHERE lp.building_id = l.building_id
AND lp.lease_id = l.lease_id
)


That has a tendency to run off the right-hand edge of the screen, though.

• Just wondering what DBMS you're using (in relation to declaring 'as' explicitly) – Michael A Dec 23 '11 at 7:55
• See my profile :) Informix. – Jonathan Leffler Dec 23 '11 at 15:41
• You will find the style with the comma at the start of the next line very useful if you do a lot of copying/cutting and pasting column names around. I am so tired of SQL parsers bitchslapping me just because I forgot an extra comma right before the 'FROM' keyword. – Mike Nakis Dec 23 '11 at 15:42
• Very valid point - also good when commenting lines out. Might adopt that - appreciated. Would upvote you but not enough karma yet :( – Michael A Dec 23 '11 at 15:45
• I know that people use the leading comma because it makes it easier to move code around without fogettting to to remove unwanted trailing commas; it can also reduce the number of lines changed when you add a new column at the end of the select-list (because you don't change what was the last line; you only add a new one). Neither is, IMNSHO (but it is only opinion) sufficient excuse to justify the violence to the normal use of comma after words. It is an ugly and unwarranted abuse of the syntax. But I'm an old fuddy-duddy, and you aren't on my team, so you can do as you see fit. – Jonathan Leffler Dec 23 '11 at 15:52