patternsqlModerate
SQL - How's my formatting?
Viewed 0 times
sqlformattinghow
Problem
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.tenant_trading_name,
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 'ALL' and upper(section_name) = upper(@section_name)))
and lp.lease_current_stop_date between @truncateddate
and dateadd(MONTH, @lease_expiry_period, @truncateddate)
order by period_id descSolution
What you've got is pretty readable, but there are a few points to make:
-
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:
-
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
For your query, I'd probably write:
Sometimes, I'd indent the sub-query more than I did here, occasionally even placing it after the EXISTS:
That has a tendency to run off the right-hand edge of the screen, though.
- Your sub-query is written with a different indentation style from the main query.
- I prefer to see the keywords in upper-case.
-
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-
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.tenant_trading_name,
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 'ALL' AND UPPER(section_name) = UPPER(@section_name)))
AND lp.lease_current_stop_date BETWEEN @truncateddate AND
DATEADD(MONTH, @lease_expiry_period, @truncateddate)
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.
Code Snippets
item1
, item2
, item3SELECT lp.period_id,
lp.building_id,
lp.suite_id,
lp.lease_id,
lp.tenant_trading_name,
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
DATEADD(MONTH, @lease_expiry_period, @truncateddate)
ORDER BY period_id DESC;AND NOT EXISTS (SELECT *
FROM lease_deal.lease AS l
WHERE lp.building_id = l.building_id
AND lp.lease_id = l.lease_id
)Context
StackExchange Code Review Q#7101, answer score: 11
Revisions (0)
No revisions yet.