GOTO 3

[NotSQL]

Can you help me with this GOTO statement



Declare @Year varchar(4) = '2010', @Quarter varchar(9) = null, @Month varchar(3) = Null

If @Quarter = null and @Month = null GOTO Step_One; Else GOTO Step_Four;



Step_One:
SELECT 'StepOne', CASE Targeted_Account
WHEN 'TRUE' THEN 'Targeted' ELSE 'Un-Targeted' END AS [Customer Type],
COUNT(DISTINCT MaginusAccount) AS [Resellers]
FROM dbo.V_TimeCard
where [YEAR] = @Year
GROUP BY Targeted_Account


Step_Four:
SELECT 'StepOne', CASE Targeted_Account
WHEN 'TRUE' THEN 'Targeted' ELSE 'Un-Targeted' END AS [Customer Type],
COUNT(DISTINCT MaginusAccount) AS [Resellers]
FROM dbo.V_TimeCard
where [YEAR] = @Year and [Quarter] = @Quarter and [MONTH] = @Month
GROUP BY Targeted_Account

Can you tell me why this executes both queries? although im only passing in one parameter...

Any help would be greatful

 

[gmmastros]

You cannot compare nulls the way you are. You need to use the IS operator instead.

[tt]
If @Quarter [!]Is[/!] null and @Month [!]Is[/!] null GOTO Step_One; Else GOTO Step_Four;
[/tt]

Also... Since execution occurs from top down, if Step_One is performed, so would Step_Four. You could add another label and go to that.

Declare @Year varchar(4) = '2010', @Quarter varchar(9) = null,  @Month varchar(3) = Null

If @Quarter [!]Is[/!] null and  @Month [!]Is[/!] null GOTO Step_One; Else GOTO Step_Four;



Step_One:
SELECT    'StepOne', CASE Targeted_Account
WHEN 'TRUE' THEN 'Targeted' ELSE 'Un-Targeted' END AS [Customer Type],
COUNT(DISTINCT MaginusAccount) AS [Resellers]
FROM dbo.V_TimeCard
where [YEAR] = @Year
GROUP BY  Targeted_Account

[!]Goto Done[/!]
Step_Four:
SELECT    'StepOne', CASE Targeted_Account
WHEN 'TRUE' THEN 'Targeted' ELSE 'Un-Targeted' END AS [Customer Type],
COUNT(DISTINCT MaginusAccount) AS [Resellers]
FROM dbo.V_TimeCard
where [YEAR] = @Year and [Quarter] = @Quarter and [MONTH] = @Month
GROUP BY  Targeted_Account

[!]Done:[/!]

You should know that writing code using GOTO is frowned upon by most developers. It leads to code that is difficult to follow and difficult to maintain. I'm not gonna sit here and preach to you about GOTO, but if you are interested in a better way, let me know and I'll be happy to show you.



-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[IanWaterman]

Not totally familiar with GOTo but are you sure you need the ; after step_one

If @Quarter = null and @Month = null GOTO Step_One; Else GOTO Step_Four;

and be

If @Quarter = null and @Month = null GOTO Step_One Else GOTO Step_Four;

 

[NotSQL]

Yeah please assist if you believe there is a better way

 

[gmmastros]

There are many different ways to do this. Each way has an advantage and a disadvantage, so I will attempt to explain this as best I can...

One query method

Looking at the 2 queries involved, I notice that the only difference is in the where clause.

[tt]
where [YEAR] = @Year

where [YEAR] = @Year and [Quarter] = @Quarter and [MONTH] = @Month[/tt]

It looks like you want to use the simplified where clause if @Quarter is NULL and @Month is NULL. There is a way to make a where clause that accommodates this condition. Like this:

SELECT    CASE Targeted_Account
WHEN 'TRUE' THEN 'Targeted' ELSE 'Un-Targeted' END AS [Customer Type],
COUNT(DISTINCT MaginusAccount) AS [Resellers]
FROM dbo.V_TimeCard
where [YEAR] = @Year 
      [!]and (@Quarter Is Null Or [Quarter] = @Quarter) 
      and (@Month Is NULL or [MONTH] = @Month)[/!]
GROUP BY  Targeted_Account

This one query does the same thing as your 2 separate queries. Specifically... the results would always be the same. The benefit of this approach is that you only have one query to maintain. The disadvantage of this method is that the query may be ever so slightly slower because OR conditions don't perform as well.

Multiple Query Method

Instead of using GOTO's in your code, you could reorganize it a little so that the goto's are gone, but you effectively get the same execution. Like this:

Declare @Year varchar(4) = '2010', @Quarter varchar(9) = null,  @Month varchar(3) = Null

If @Quarter = null and  @Month = null
  Begin
    SELECT    'StepOne', CASE Targeted_Account
    WHEN 'TRUE' THEN 'Targeted' ELSE 'Un-Targeted' END AS [Customer Type],
    COUNT(DISTINCT MaginusAccount) AS [Resellers]
    FROM dbo.V_TimeCard
    where [YEAR] = @Year
    GROUP BY  Targeted_Account
  End
Else
  Begin
    SELECT    'StepOne', CASE Targeted_Account
    WHEN 'TRUE' THEN 'Targeted' ELSE 'Un-Targeted' END AS [Customer Type],
    COUNT(DISTINCT MaginusAccount) AS [Resellers]
    FROM dbo.V_TimeCard
    where [YEAR] = @Year and [Quarter] = @Quarter and [MONTH] = @Month
    GROUP BY  Targeted_Account
  End

The disadvantage of this query is that there are actually multiple queries here. If you wanted to modify this code to add another column, you would need to modify both queries. To make things worse, suppose you wanted to handle 3 "possibly null" parameters in any combination. You would need to write 8 different queries. This makes maintenance harder. The advantage to this method is that there are no OR's and each query will execute faster than the first method I showed.

Dynamic SQL

You can use dynamic sql to run the query. With dynamic sql, you are essentially building a string that represents your query and then you execute it.

Ex:

Declare @Year varchar(4) = '2010', @Quarter varchar(9) = null,  @Month varchar(3) = Null

Declare @SQL VarChar(8000)

Set @SQL = 'SELECT    ''StepOne'', CASE Targeted_Account
    WHEN ''TRUE'' THEN ''Targeted'' ELSE ''Un-Targeted'' END AS [Customer Type],
    COUNT(DISTINCT MaginusAccount) AS [Resellers]
    FROM dbo.V_TimeCard
    where 1 = 1'

If @Year Is Not NULL
	Set @SQL = @SQL + ' and [Year] = ''' + @Year + ''''

If @Quarter Is Not NULL
	Set @SQL = @SQL + ' and [Quarter] = ''' + @Quarter + ''''

If @Month Is Not NULL
	Set @SQL = @SQL + ' and [Month] = ''' + @Month + ''''

Set @SQL = @SQL + ' GROUP BY  Targeted_Account'

Exec (@SQL)

The advantage with this approach is that there is only one query to maintain, and it executes faster. The disadvantage is that you need to be careful about SQL injection, and EXEC is disabled on the server by default when using SQL2005 or newer. The query I show above is susceptible to SQL Injection, but there are thing you can do to help protect yourself, too. If you decide to use dynamic sql, make sure you read up on [google]Dynamic SQL[/google] and [google]SQL Injection[/google].



-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[gmmastros]

In the Multiple Query Method I mistakenly copy/pasted the original code without changing the NULL condition to use the IS operator. The corrected code should be:

Declare @Year varchar(4) = '2010', @Quarter varchar(9) = null,  @Month varchar(3) = Null

If @Quarter Is null and  @Month Is null
  Begin
    SELECT    'StepOne', CASE Targeted_Account
    WHEN 'TRUE' THEN 'Targeted' ELSE 'Un-Targeted' END AS [Customer Type],
    COUNT(DISTINCT MaginusAccount) AS [Resellers]
    FROM dbo.V_TimeCard
    where [YEAR] = @Year
    GROUP BY  Targeted_Account
  End
Else
  Begin
    SELECT    'StepOne', CASE Targeted_Account
    WHEN 'TRUE' THEN 'Targeted' ELSE 'Un-Targeted' END AS [Customer Type],
    COUNT(DISTINCT MaginusAccount) AS [Resellers]
    FROM dbo.V_TimeCard
    where [YEAR] = @Year and [Quarter] = @Quarter and [MONTH] = @Month
    GROUP BY  Targeted_Account
  End

-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[SQLSister]

Yet another wonderful explanation from the master.

"NOTHING is more important in a database than integrity." ESquared

 

[NotSQL]

Thanks, that is realy useful information

So Would you recommend the fist option as the best option...?

 

[gmmastros]

It's hard to say what I recommend, which is why I presented all 3 options.

If the first option performs well and you expect the table to remain relatively small, then the first option is probably best. I would probably recommend AGAINST option 2 (the multiple query approach) because it become a maintenance nightmare when you have many options.

Dynamic SQL can be a good option if your DBA allows you to use dynamic SQL, and you can control the user inputs. By "control the user inputs", I am NOT talking about your front end stuff, because that cannot always be trusted. In your case, it appears as though Year, Quarter, and Month are very well known values. So, if you write code in to your procedure that validates the input, the Dynamic SQL method maybe be appropriate for you. Ex:

If not (IsNumeric(@Year) = 1 and Convert(Int, @Year) Between 1950 and 2050)
	Set @Year = NULL

If Not (IsNumeric(@Month) = 1 And Convert(Int, @Month) Between 1 and 12)
	Set @Month = NULL

By making sure you have numeric inputs and the values are within a particular range, you can prevent problems with SQL injection. Better yet would be to declare your inputs (assuming this is a stored procedure) to be integers. Then, if someone hacks your system and attempts to pass bad arguments in to your procedure it will fail even before it attempts to execute the dynamic SQL. I'm not sure what your Quarter data looks like, but you should either compare it to a list of known values, or make sure it is numeric.

Make sense?

-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[NotSQL]

I've created a store procedure using this information however when i try and run the procedure in the form of EXEC Storeprocedure '2010'. It states it requires the @Quarter parameter

Any ideas?

ALTER PROCEDURE [dbo].[Report_1_Visits_by_Targeted_and_Un-Targeted]
-- Add the parameters for the stored procedure here

@Year varchar(4), @Quarter varchar(9), @Month varchar(3)
AS
BEGIN
-- SET NOCOUNT ON added to prevent extra result sets from
-- interfering with SELECT statements.
SET NOCOUNT ON;

-- Insert statements for procedure here

SELECT CASE Targeted_Account
WHEN 'TRUE' THEN 'Targeted' ELSE 'Un-Targeted' END AS [Customer Type],
COUNT(DISTINCT MaginusAccount) AS [Resellers]
FROM dbo.V_TimeCard
where [YEAR] = @Year
and (@Quarter Is Null Or [Quarter] = @Quarter)
and (@Month Is NULL or [MONTH] = @Month)
GROUP BY Targeted_Account


END

 

[gmmastros]

That's right. You have 3 parameters to the stored procedure, but you only supplied one parameter in your call.

There are 2 ways to fix this.

1. Add the parameters to the stored procedure call.
[tt]EXEC Storeprocedure '2010', NULL, NULL[/tt]

2. Modify the stored procedure to make the parameters optional.
[tt]
ALTER PROCEDURE [dbo].[Report_1_Visits_by_Targeted_and_Un-Targeted]
-- Add the parameters for the stored procedure here

@Year varchar(4), @Quarter varchar(9) [!]=NULL[/!], @Month varchar(3) [!]=NULL[/!]
AS
[/tt]

-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom

 

[NotSQL]

Thanks for all this very useful information gmmastros...

 

[gmmastros]

You're welcome.


-George

"The great things about standards is that there are so many to choose from." - Fortune Cookie Wisdom