3

I have a SQL view, that as far as I understand, attempts to get the latest invoice date and the year and month combination of that invoice date. These values are then used to count the quantity in that month.

My SQL knowledge is basic and I'm not sure why the person would do this part:

MAX(CONVERT(varchar(3), InvoiceDate, 101) + CONVERT(varchar(4), 
                  YEAR(InvoiceDate))) AS YearMonth

The rest of the code, including the TOP(100) PERCENT part etc., I am fairly happy with.

I'm not sure if the part in question was a logic mistake or if it has some SQL or business reason to it as in some instances it is completely different from the Max Invoice Date.

The full SQL expression below:

SELECT TOP (100) PERCENT    
            MAX(InvoiceDate) AS MaxInvoiceDate, 
            StockCode, Warehouse, 
            MAX(CONVERT(varchar(3), InvoiceDate, 101) + CONVERT(varchar(4), YEAR(InvoiceDate))) AS YearMonth
FROM dbo.ArTrnDetail
GROUP BY StockCode, Warehouse
HAVING (Warehouse = 'CS') OR
      (Warehouse = 'PS') OR
      (Warehouse = 'DS') OR
      (Warehouse = 'JS') OR
      (Warehouse = 'JN')
ORDER BY MaxInvoiceDate DESC

And a potentially problematic output:

MaxInvoiceDate           | StockCode | Warehouse    | YearMonth
----------------------------------------------------------------
2013-08-21 00:00:00.000  | ACH045    | PS           | 12/2012
gofr1
  • 15,741
  • 11
  • 42
  • 52
Jak
  • 471
  • 5
  • 20
  • 2
    I have no idea why someone would want the maximum of MM/YYYY. It doesn't look correct but perhaps there is some arcane reason. – Gordon Linoff Aug 30 '16 at 11:59
  • If `InvoiceDate` is an actual `date` or `datetime` or `datetime2` type you can use the `DATEPART` sql function instead of `CONVERT`. The act of converting it to a string might be a convenience for the consumer of the view but you would have to look at that code to see why that is. – Igor Aug 30 '16 at 12:00
  • Please, get rid of the ORDER BY from the view.. It's not only useless, but could actually hurt performance. Ordering, if really must be done in SQL Server, should be part of the outermost query. – dean Aug 30 '16 at 12:03
  • I don't really care about the speed etc, as I'm recreating the output in other software. I do want to know if the difference between MaxInvoiceDate and YearMonth values are intentional for some reason, if that is something someone can answer from a SQL perspective. – Jak Aug 30 '16 at 12:08
  • No one can answer that who does not have access to all the code that makes use of that sql view. Purely technical, there is no reason or benefit to do it as the InvoiceDate is already being returned as a value so the consumer could use that directly. But I would not change it until you know what is calling it and how its being used, and we (SO users) can not answer that question for you. – Igor Aug 30 '16 at 12:26
  • 1
    I'm voting to close this question as off-topic because speculation as to reasons why some random code exists is not on topic. – Martin Smith Aug 30 '16 at 12:30
  • @MartinSmith I can see how a knowledgeable SQL user can see it as off-topic, but by the mere fact that the code in question does not look correct, to advanced SQL users, helps me. I now know that the only way to be certain is to ask the original creator, if I can find the person. – Jak Aug 30 '16 at 12:50

2 Answers2

1

That's interesting, but makes sense. the string "12" is going to be the MAX string in any month formulation, and that record goes to the previous year, which also makes sense if the actual max happened in August of 2013. The simplest way to fix it is to move the MAX inside the conversion so you are converting the same date that is in the first column:

SELECT TOP (100) PERCENT    
            MAX(InvoiceDate) AS MaxInvoiceDate, 
            StockCode, Warehouse, 
            CONVERT(varchar(3), MAX(InvoiceDate), 101) + CONVERT(varchar(4), YEAR(MAX(InvoiceDate))) AS YearMonth
FROM dbo.ArTrnDetail
GROUP BY StockCode, Warehouse
HAVING (Warehouse = 'CS') OR
      (Warehouse = 'PS') OR
      (Warehouse = 'DS') OR
      (Warehouse = 'JS') OR
      (Warehouse = 'JN')
ORDER BY MaxInvoiceDate DESC

As to why the original programmer didn't use the datepart in the first half, its pretty slick, you get the '/' character for free by getting the first 3 characters of MM/DD/YYYY. Not something I would have thought of, but it actually saves you a command or string concatenation.

Brett Law
  • 68
  • 2
  • 8
  • Do you then suggest that it's a slick approach with an error? Your output for YearMonth in my example is 08/2013... What I would expect. – Jak Aug 30 '16 at 13:12
  • @Jak: yes. That one part was slick, I actually like way LukStorms did it below better. I try to respect a way of doing something that I wouldn't have thought of. Grabbing the first three characters of MM/DD/YYYY when you want MM/YYYY is slick. Formatting it DD/MM/YYYY and grabbing the last 7 characters is also slick! – Brett Law Aug 30 '16 at 15:40
  • Turns out you were completely correct and you helped me convince the person in question. Thank you again. – Jak Sep 01 '16 at 10:57
0

The query below coverts the maximum date to a format. Which is different from the wrong idea to first convert the date parts of each date and then get the maximum of the calculated varchars.

And with a calculated column named 'YearMonth', one would expect a value that starts with the year and then the month. (It's also easier to sort a YYYY/MM format)

SELECT TOP (100) PERCENT    
    StockCode, Warehouse, 
    MAX(InvoiceDate) AS MaxInvoiceDate, 
    CONVERT(varchar(7), MAX(InvoiceDate), 111) as YearMonth
FROM dbo.ArTrnDetail
WHERE Warehouse in ('CS','PS','DS','JS','JN')
GROUP BY StockCode, Warehouse
ORDER BY MaxInvoiceDate DESC;

Gives:

MaxInvoiceDate       StockCode  Warehouse   YearMonth
21.08.2013 00:00:00  ACH045     PS          2013/08

Note that the HAVING has been switched with a WHERE clause.
This will give the same, but potentionally faster results.

If the MM/DDDD format is prefered anyway, then this will calculate it in that format.

RIGHT(CONVERT(varchar(10), MAX(InvoiceDate), 103),7) as MonthYear
LukStorms
  • 28,916
  • 5
  • 31
  • 45