Count records and pull DISTINCT

Question

I have a table that reads every minute, but the data on one meter only changes once every 60 minutes. It will then repeat that value for 60 minutes before changing again.

I need to take the sum of these values and add them up for the day. But if I grab it by the hour, I will get duplicates. So what I decided to do was count the number of records, and if is over 30 minutes use that record.

This is currently working good and is not a big issue, but I am curious if there is an easier way to do this.

WITH A AS 
(SELECT DISTINCT 
  CONVERT(CHAR(12), TIME, 101) AS [ DAY ],
  DatePart (HOUR, TIME) AS [ HOUR ],
  [ B100 Pulse LAST HOUR ],
  [ RESET Counter ],
  [ B700 Pulse LAST HOUR ],
  [ B800 Pulse LAST HOUR ],
  [ B1300 Pulse LAST HOUR ],
  [ B900 Pulse LAST HOUR ],
  COUNT([ RESET Counter ]) AS Records 
FROM
  (SELECT 
    TOP (100) PERCENT KPI $.[ B100 Pulse LAST HOUR ],
    Main $.Time,
    KPI $.[ RESET Counter ],
    KPI $.[ B700 Pulse LAST HOUR ],
    KPI $.[ B800 Pulse LAST HOUR ],
    KPI $.[ B900 Pulse LAST HOUR ],
    KPI $.[ B1300 Pulse LAST HOUR ] 
  FROM
    Main $ 
    INNER JOIN KPI $ 
      ON Main $.[LOOP INDEX ] = KPI $.[LOOP INDEX ] 
  ORDER BY Main $.Time DESC) AS t1 
GROUP BY CONVERT(CHAR(12), TIME, 101),
  DatePart (HOUR, TIME),
  [ B100 Pulse LAST HOUR ],
  [ RESET Counter ],
  [ B700 Pulse LAST HOUR ],
  [ B800 Pulse LAST HOUR ],
  [ B1300 Pulse LAST HOUR ],
  [ B900 Pulse LAST HOUR ]) 
SELECT 
  Top 120 [ DAY ],
  COUNT(*) AS [ # of Records], SUM([B100 Pulse Last Hour]) * 100 AS [Building 100 Gal Discharged], SUM([B700 Pulse Last Hour]) * 100 AS [Building 700 Gal Discharged], (SUM([B800 Pulse Last Hour])
  - SUM([ B1300 Pulse LAST HOUR ])) * 100 AS [ Building 800 Gal Discharged ],
  SUM([ B900 Pulse LAST HOUR ]) * 100 AS [ Building 900 Gal Discharged ],
  SUM([ B1300 Pulse LAST HOUR ]) * 100 AS [ Building 1300 Gal Discharged ],
  SUM(
    (
      [ B100 Pulse LAST HOUR ] + [ B700 Pulse LAST HOUR ] + [ B800 Pulse LAST HOUR ] + [ B900 Pulse LAST HOUR ]
    ) * 100
  ) AS [ Daily Total ],
  [ STARTING LEVEL ] AS [ LEVEL IN B1800 AT Midnight ],
  [ Difference ] 
FROM
  (SELECT 
    *,
    ROW_NUMBER () Over (
      PARTITION BY Records 
  ORDER BY Records ASC
  ) AS ROW 
  FROM
    A 
    INNER JOIN B1800_Totals 
      ON A.[ DAY ] = B1800_Totals.[ DATE ]) AS A2 
WHERE Records >= 30 
  AND ([ DAY ] = @date) 
GROUP BY DAY,
  [ STARTING LEVEL ],
  [ Difference ] 
ORDER BY DAY ASC 

Answer 1

Well done on getting a query that works: that is the hardest part!

I do agree that you have overcomplicated this query: I had to spend quite a long time playing around with it to figure out what it was intended to do.

I also suspect it might not be the best way to achieve what you want. So well done also for seeking feedback here - I'm sorry mine is coming quite late. I'm going to dump a tonne of feedback in this answer: it might seem fairly blunt but don't take it personally, we all learn from experience. :)

I struggled to decide what order to present my feedback in, so it might not be in the most logical order for you. I apologise if so.

How to tackle complex queries

There are two ways one can go about a complex query:

1. Dive in and try to get something that works by examination of your results, then refine it subsequently, or
2. Spend time up-front designing the query without actually writing it, and go straight to a simpler (perhaps simplest) solution. Usually this means you can "prove" the query to be correct by reasoning about the design logic, without having to examine the results so much.

I'm a big fan of (2) and believe that it is generally viewed as the "best" option across all schools of programming - but of course it's not always practical.

It seems to me that the biggest problem with this query is that you took approach (1). The query screams "unclear thinking" to me; it is messy and hard to read, with quite a lot of unnecessary bits. Learning to design code well before you write it is a skill of its own, but one well worth practising if you are able to.

High-level algorithm choice

Without any sample data and clear information on the source table structures, it is hard for me to critique your overall approach.

It sounds like what you have is a fairly typical scenario, that the data warehousing world at least refers to as "semi-additive facts". That is, data that are numeric and can in principle be added to each other, but for which adding them only makes sense in certain limited scenarios. Semi-additive facts aren't that uncommon and you might find that a useful search term if you want to read more about dealing with them.

Certainly, a common approach to semi-additive facts is "two pass" cubing (or grouping/aggregation), which is essentially all this query is doing (but it took me far too long to understand that). So your general approach might be just fine.

However I have a niggling feeling that you don't need to bother with the "count the number of records and if it's over 30, use that record" bit. I can't say for sure without seeing some of your data but it feels like you could just DISTINCT your core data set to reduce it to one row per meaningful set of measurements, and then do only one GROUPing operation.

I suspect that if you work on simplifying and clarifying your query, you will be able to see for yourself if this second grouping is necessary or not. When a query is as simple as it can be, wrong logic is usually easy to spot.

Comments, magic numbers and code that won't run

I'm also totally unclear where the magic number 30 comes into things. If you get a measurement per minute, which changes once per hour, I'd have expected the number 60 to feature, not 30.

Your code would benefit from clear comments explaining why constant numbers like this are what they are - more generally, your code would benefit from any comments at all.

The Top 120 bit falls into the same category. Is that 120 days (~4 months)? I'm not sure it is, because of the mysterious @date variable hidden uncommented in your code.

This isn't declared anywhere so as far as I'm concerned, your code is broken and does not run. I presume you forgot to include the declaration, or this code exists within a stored procedure - but you haven't mentioned. If a single date is being passed in, I can't understand how your query will return more than one day of results, so the Top 120 doesn't make sense. Neither does the final ORDER BY DAY ASC - it seems to me that a result set will only ever contain one value of DAY.

CTEs vs sub-queries and incremental problem solving

This bit is both SQL-specific stylistic advice and very general programming advice.

Break any programming problem down into a series of steps, then solve those steps one-by-one. The clearer it is to follow in your code which steps are which, and what order they happen in, the better your code will be.

T-SQL provides a really handy construct for step-by-step problem solving, the common table expression (CTE); you use one of them, named A. The big problem is that you only use one of them. Apart from A you use a variety of subqueries (SELECT query results enclosed in brackets, and treated like tables in their own right).

A bit of history: in the international definition of the SQL language, subqueries have been around a long time, and CTEs are a more recent addition. CTEs were introduced mainly as an improvement on subqueries: they are clearer, more flexible, and reusable (within one query).

In the context of "production" SQL code (code to be written once and used again and again - as against ad-hoc one-off analysis) I do not personally believe there is any situation in which it is better to use a subquery. CTEs are not always the best choice for performance, but in that case other constructs such as temporary tables and table variables are the better choice.

Conclusion: rework your query to use a linear series of CTEs, each of which builds on the one before towards the end result.

Use as few constructs / steps as necessary

Once you do a CTE-focussed overhaul of your code, you will see that multiple steps can be combined into one to simplify (rather than complicate) the overall query.

You have used far more steps than you need to but it's very hard to identify that currently, because you have to skip around the query to follow it.

In a general sense, you need to know/remember that what SQL does best is join tables together. Unless there's a good reason not to (for example to do with cardinality, the number of records in the relationship), do all your joins in one query set: this gives SQL Server more options for optimising how it does them.

You also need to know that GROUP BY clauses work fine in the same query as multiple joins. There's no need to do joins in one set (CTE or subquery) then group the results in a separate set.

Style and layout

Consistency is crucial in any programming task I've come across. Much of the following relates to consistency.

• I can't see any patterns of when you start new lines, why/how much you indent your code, where you put commas, and so on. This reduces readability a lot. Choose a style then follow it consistently.
• In general, use more indentation and spacing, not less. Bytes are not precious in this day and age. Judicious use of blank lines and reasonably deep indentation make code much easier on the eye.
• Capitalise consistently. This applies to table and column names wherever you have control over them (see below for more on that), and to function names and SQL keywords. If you are capitalising keywords like SELECT, Top should also be capitalised.
• Unnecessary brackets () in expressions are distracting. Instead of AND ([ DAY ] = @date), just AND [ DAY ] = @date.
• Your code doesn't convince me that you understand the basic rules of arithmetic as they apply to SQL: for example, that sum(column*100) is the same as sum(column)*100, or that sum(column1) - sum(column2) is the same as sum(column1-column2). If you aren't rock solid on these (which is OK!) I recommend brushing up - it will really help over time. If you are, use one form or the other consistently; it will give any reader of the code additional trust in your ability.
• In a production-quality SQL query, never list columns that don't need to be there. It is distracting and can (sometimes) affect performance. For example [ RESET Counter ] is never used directly (only within a COUNT()) so shouldn't be listed. You also generate a ROW_NUMBER() but never refer to or show it, so it shouldn't be there - and removing it reduces the number of sets/steps you need overall.
• The SQL * is a useful shorthand when developing queries but it has no place in a final query that will be used in production. It is far better to list explicitly the columns you want to see. This makes the query more robust against changes to underlying table definitions.
• Where you list the same/similar set of columns in multiple places (for example in a SELECT then again in GROUP BY, or in a CTE then again in the main SELECT), list them in the same order each time. It's much easier to see if columns have been omitted, deliberately or accidentally.

Object naming

• You should qualify table names with which schema they belong to. This makes your code more robust if multiple schemas are used in the future.
• You need to familiarise yourself with the rules of regular database identifiers in SQL Server. As you're clearly aware, non-regular identifiers can be used if enclosed in square brackets, but this is bad practice. Wherever possible use regular identifiers, which means no spaces. Use underscores or CamelCase instead. This (and all subsequent advice in this section) applies to aliases as well as actual object names.
• While # and $ are valid in general, you shouldn't be using # at the start of anything (e.g. a column alias) because it denotes a temporary table. I would recommend avoiding # and $ altogether if possible: there really isn't any need for them that I'm aware of.
• Your underlying table names Main $ and KPI $ (I think that's what they are?) are not enclosed in square brackets: I'm surprised this hasn't caused an error.
• You should also be avoiding using SQL Server reserved words for object names. That link is a good one to familiarise yourself with.
• If you cannot control the presence of spaces, $ signs, reserved words etc in your underlying table/column names, consider creating some simple views that mask this by renaming all columns to SQL-compliant values that won't need square brackets. This would make your query (and all other queries on the same dataset) much, much more readable.
• Always alias every table in a multi-table query and use the aliases consistently to qualify every column reference. This is clearer than having to type out the full table name to qualify columns. By qualifying every column with its table name, it's really obvious which columns come from which tables (without having to refer to an ERD or specification).
• Use sensible, meaningful aliases. A is almost always a terrible name for a column, table, CTE or anything else, because it doesn't tell you anything about what it does or what it is for. I always prefix CTE names with cte so I know that's what they are when I refer to them later.
• CTE names can afford to be reasonably long (same kind of length as tables) to describe their contents.
• Table aliases are best short - I stick to 2 or 3 characters. One is usually too short to be clear. I'd alias Main as MN and KPI as KPI (but still alias it so it's clear!).

A few points related specifically to spacing and choice of column names and aliases:

• If at all possible avoid the fancy spaced column aliases in your final results (and throughout). I think B100_Pulse_Last_Hour is as clear as [ B100 Pulse LAST HOUR ] in a result set - give your data users some credit.
• Or, it's possible you're using the wrong technology here - if you really need "pretty" column names perhaps you should be using SSRS or some other technology downstream of the query?
• If you really, really need pretty column names at the end of this query, consider the Alias = Expression syntax rather than Expression AS Alias. This makes it easier to align your aliases at the start of the query and let the differing-length expressions run off to the right.

For example (if you've not seen it before)

select [ Building 100 Gal Discharged ] = 100*SUM(B100_Pulse_LAST_HOUR)
from    table

Potential performance / behaviour effects

• If I've understood the purpose of your @date variable, you should use it to limit the number of records as early as possible in the query, not as late as possible. Cutting out unnecessary rows early on will speed up later parts of the query.
• In CTE A: the DISTINCT keyword is useless because you're also grouping by all the non-aggregate columns.
• TOP (100) PERCENT is a useless construct: that means all rows, which is the default behaviour if you remove that bit altogether.
• Where you use CONVERT to get [ DAY ]: format 101 outputs 10-character results (mm/dd/yyyy) but you use a CHAR(12) which will cause trailing spaces.
• If you're on 2008+ you can use the date data type instead: CAST(Time as date), confusing though that reads, will do essentially the same thing as your CONVERT, but keep it in actual date format.
• Although I've said above your ROW_NUMBER() is useless, you should understand that if you do need it, it is pointless to ORDER BY the same expression you PARTITION BY. The partition means that the numbering will restart from 1 for every distinct value of Records. Within a partition, all rows will have the same value of Records, so you cannot useful order on that column. This means the ordering is essentially random - not a good thing.
• Instead of getting COUNT([ RESET Counter ]) in one row set and filtering on it in a later row set using WHERE, you can filter directly on the result of a COUNT() function using the HAVING clause. It works the same as WHERE but is intended for working on aggregate functions like COUNT(), SUM() etc. You put it after the GROUP BY clause and before an ORDER BY if you have one. Again, this reduces the number of sets you need.