3
\$\begingroup\$

I have a large Sql Server view with this schema:

  • [archive_ID] (int, not null)
  • [archive_date] (datetime, not null)
  • [archdata_path_ID] (varchar(50), not null)
  • [archdata_value] (int not null)

I need to group the records by the Date, and I need to extract just the first record for each group.

This is the current query:

WITH cte
AS (
        SELECT *
               ,CAST(archive_date AS DATE) AS C
               ,ROW_NUMBER() OVER (
                       PARTITION BY CAST(archive_date AS DATE) ORDER BY CAST(archive_date AS DATE) ASC
                       ) AS ad
        FROM ArchiveData
        WHERE archdata_path_ID = @PathID
        )
SELECT DISTINCT C
        ,archdata_value AS val
FROM cte
WHERE ad = 1
ORDER BY C ASC

The main problem is to improve the readability. Would be great to optimize also the performance, but it's not mandatory.

\$\endgroup\$
1
  • \$\begingroup\$ Can you include the version of SQL Server this is for in the question? \$\endgroup\$
    – nhgrif
    Aug 4 '14 at 17:29
4
\$\begingroup\$

I believe that DISTINCT is redundant, since the CTE should produce only one row for each date whose ROW_NUMBER() is 1.

Avoid selecting * in the CTE, and list the columns you want explicitly.

Your naming is poor: CTE, C, val, ad. Please find more descriptive names.

If you are using any SQL Server ≥ 2012, then FIRST_VALUE() is the function you want.

SELECT CAST(archive_date AS DATE) AS C
     , FIRST_VALUE(archdata_value) OVER (
           PARTITION BY CAST(archive_date AS DATE)
               ORDER BY CAST(archive_date AS DATE)
       ) AS val
    FROM ArchiveData
    WHERE archdata_path_ID = @PathID
    ORDER BY C;
\$\endgroup\$
2
\$\begingroup\$

Okay, you firstly, in your CTE you bring back SELECT * even though you don't use most of the columns. That Select * can be removed and replaced with the actual columns you need.

The CTE, while arguably helping readability a little is not very quick, I would recommend avoiding CTE's, and using subqueries or temporary tables. Table Variables are okay if you only have a handful of rows.

From what I can see, the CTE in this query is not needed anyway, so removing it should help you to understand what is actually happening.

Here is my suggestion

--return archive_date and archdata_value
SELECT DISTINCT CAST(archive_date AS DATE) AS date,
archdata_value AS val
FROM ArchiveData
WHERE archdata_path_ID = @PathID
--this is in the having clause because aggregate functions can not be in the where clause 
HAVING ROW_NUMBER() OVER (
       PARTITION BY CAST(archive_date AS DATE) ORDER BY CAST(archive_date AS DATE) ASC) = 1
--I changed from C to date to make it clearer what was being returned
ORDER BY date ASC

This should do exactly the same as the query you provided but in a single step.

\$\endgroup\$
4
  • \$\begingroup\$ Thanks for your help. How can I translate the "HAVING ROW_NUMBER() OVER ( PARTITION BY ... )" part into Linq? \$\endgroup\$
    – simoneL
    Aug 4 '14 at 14:30
  • \$\begingroup\$ You may find this page and this page helpful. They both have what look to be pretty good examples of how to do a having clause in Linq. \$\endgroup\$ Aug 4 '14 at 14:38
  • 1
    \$\begingroup\$ I'm not sure I like the idea of blindly avoiding CTEs in favor of subqueries, temp tables, and table variables. Use the right tool for the right job, and frequently CTEs will be the right tool. Also, simple (non-recursive) CTEs are not slower than subqueries, but they do tend to make queries more readable. Meanwhile, recursive CTEs do stuff that subqueries can't do. In this specific example however, I agree that the CTE is probably unnecessary. \$\endgroup\$
    – nhgrif
    Aug 4 '14 at 17:35
  • \$\begingroup\$ It might just be the queries I've had to deal with at work. But removing the CTEs has almost always improved performance. Almost always, I realise they still have a place, I just found that using them as more of a last resort worked better. \$\endgroup\$ Aug 4 '14 at 18:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.