HiveBrain v1.2.0
Get Started
← Back to all entries
patternsqlModerate

SQL - How's my formatting?

Submitted by: @import:stackexchange-codereview··
0
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 desc

Solution

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

  • 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
   , item3
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 < @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.