July 1, 2011 at 9:24 am
Hello SQL gods, 😉
I've created a scalar-valued function that returns a figure. I pass in a variable @assetRegion_id. Dependant on the value of this variable the where clause will change.
if 3 then psf.assetRegionID > 0
else if 2 then psf.assetRegionID > 1
else psf.assetRegionID = @assetRegion_ID
The problem is caused by the fact that the sign has to change too, from > to =
I know I can do this if I build the tsql string and then execute at the end of the function. But this doesn't seem to be an option as the function doesn't seem to like exec(string)
here is an example of the function:
RETURN
(SELECT SUM(ISNULL(psf.figure1,0)) *
CASEWHEN psf.currency_id1 = 1 OR psf.figuretype_id1 <> 1
THEN 1
ELSE (SELECT uk_rate FROM pfo_currency WHERE currency_id = psf.currency_id1)
ENDAS [figure]
FROMpfo_schemefigures psf
WHEREpsf.dataset_id = @dataset_id
ANDpsf.figureType_id1 = 1
ANDpsf.categoryType_id = @categoryType_ID
ANDpsf.assetRegion_ID = @assetregion_id
GROUP BY psf.currency_id1,psf.figuretype_id1)
I've bolded the part where I would like to add a case statment. I know it sounds very simple but again the problem is it will need to be > or = depending on the value.
Any comments/suggestions would be very much appreciated!
July 1, 2011 at 9:39 am
I would try someting like this --
SELECT 1
WHERE 1 = CASE
WHEN @x = @y THEN 1
ELSE 0
END
@x = @y is of course your condition, or one of your conditions.
Good luck!
July 1, 2011 at 9:39 am
You could change the query to to
AND psf.assetRegion_ID >= @assetregion_id_min
AND psf.assetRegion_ID <= @assetregion_id_max
set @assetregion_id_min to 1, 2 or @assetRegion_ID and
@assetregion_id_max to 2147483647 (max value for an INT data type) or @assetRegion_ID
(assuming assetRegion_ID is an integer data type)
July 1, 2011 at 2:41 pm
I'm not sure I understand waht you are tryign to do, but myabe this will help? SELECT
...
WHERE
1 = CASE
WHEN @assetRegion_ID = 3 AND psf.assetRegionID > 0
THEN 1
WHEN @assetRegion_ID = 2 AND psf.assetRegionID > 1
THEN 1
WHEN @assetRegion_ID <> 2 AND @assetRegion_ID <> 3 AND psf.assetRegionID = @assetRegion_ID
THEN 1
ELSE 0
END
July 1, 2011 at 4:33 pm
Instead of using EXEC, take a look at books online here for examples of using sp_executeSQL to do a "parameterized" dynamic SQL query, passing variables from the calling procedure into the dynamic SQL.
__________________________________________________
Against stupidity the gods themselves contend in vain. -- Friedrich Schiller
Stop, children, what's that sound? Everybody look what's going down. -- Stephen Stills
July 1, 2011 at 8:58 pm
First, instead of posting a piece of the function try posting the entire function for us to see.
Second, please tell me how this fragment is supposed to work,as I am unfortunately confused:
if 3 then psf.assetRegionID > 0
else if 2 then psf.assetRegionID > 1
else psf.assetRegionID = @assetRegion_ID
Here is where I am confused:
IF 3 -- if what is 3
else IF 2 -- again if what is 2
July 4, 2011 at 4:04 am
Thanks for you replys.
LutzM - That sounds like it might just work I will give it a go now and see.
Dixie - Unfortunately I cannot use Dynamic Sql within a function.
Lynn - I had pasted the function at the bottom. But I can understand how my post was confusing. Ultimately this is what i'm trying to achieve:
ALTER FUNCTION [dbo].[udf_getDropDownValSumAll] (@dataset_id INT,@category VARCHAR(10),@accesslevel INT,@currency_ID INT, @assetregion_id INT)
RETURNS DECIMAL(20,2)
AS
BEGIN
DECLARE @categoryType_ID INT
SELECT @categoryType_ID = categorytype_id FROM PFO_categories WHERE category = @category AND book_id IN (select bookid from dbo.uf_GetAccesLevel(@accesslevel))
RETURN
(SELECT SUM(ISNULL(COALESCE(psf.figure0,psf.figure1),0)) *
CASEWHEN COALESCE(psf.figure0,psf.currency_id1) = 1 OR COALESCE(psf.figuretype_id0,psf.figuretype_id1) <> 1
THEN 1
ELSE (SELECT uk_rate FROM pfo_currency WHERE currency_id = COALESCE(psf.currency_id0,psf.currency_id1))
ENDAS [figure]
FROMpfo_schemefigures psf
WHEREpsf.dataset_id = @dataset_id
ANDCOALESCE(psf.figureType_id0,psf.figureType_id1) = 1
ANDpsf.categoryType_id = @categoryType_ID
ANDpsf.assetRegion_ID =
CASEWHEN ISNULL(@assetregion_id,psf.assetregion_id) = 3
THEN AND psf.assetRegion_id > 0
WHEN ISNULL(@assetregion_id,psf.assetregion_id) = 2
THEN AND psf.assetRegion_id > 1
ELSE
THEN AND psf.assetRegion_id = ISNULL(@assetregion_id,psf.assetregion_id)
END
GROUP BY psf.currency_id0,psf.currency_id1,psf.figuretype_id0,psf.figuretype_id1,figure0)
END
The part I have bolded will not work, but I would like to create a case statement that will. I will try LutzM's suggestion and let you know how I get on...
July 4, 2011 at 4:23 am
LutzM (7/1/2011)
You could change the query to toAND psf.assetRegion_ID >= @assetregion_id_min
AND psf.assetRegion_ID <= @assetregion_id_max
set @assetregion_id_min to 1, 2 or @assetRegion_ID and
@assetregion_id_max to 2147483647 (max value for an INT data type) or @assetRegion_ID
(assuming assetRegion_ID is an integer data type)
Thank you VERY much LutzM, this seems to have done the job.
Here is the final function:
ALTER FUNCTION [dbo].[udf_getDropDownValSumAll] (@dataset_id INT,@category VARCHAR(10),@accesslevel INT,@currency_ID INT, @assetregion_id INT)
RETURNS DECIMAL(20,2)
AS
BEGIN
DECLARE @categoryType_ID INT
SELECT @categoryType_ID = categorytype_id FROM PFO_categories WHERE category = @category AND book_id IN (select bookid from dbo.uf_GetAccesLevel(@accesslevel))
DECLARE @assetRegion_ID_Min AS INT
DECLARE @assetRegion_ID_Max AS INT = 2147483647 -- max value for int
IF@assetregion_id = 3 --Global
SET@assetRegion_ID_Min = 1
ELSE IF@assetregion_id = 2 --Overseas
SET@assetRegion_ID_Min = 2
ELSE
SET@assetRegion_ID_Min = 0
RETURN
(SELECT SUM(ISNULL(COALESCE(psf.figure0,psf.figure1),0)) *
CASEWHEN COALESCE(psf.figure0,psf.currency_id1) = 1 OR COALESCE(psf.figuretype_id0,psf.figuretype_id1) <> 1
THEN 1
ELSE (SELECT uk_rate FROM pfo_currency WHERE currency_id = COALESCE(psf.currency_id0,psf.currency_id1))
ENDAS [figure]
FROMpfo_schemefigures psf
WHEREpsf.dataset_id = @dataset_id
ANDCOALESCE(psf.figureType_id0,psf.figureType_id1) = 1
ANDpsf.categoryType_id = @categoryType_ID
--ANDpsf.assetRegion_ID = ISNULL(@assetregion_id,psf.assetregion_id)
ANDpsf.assetRegion_id between@assetRegion_ID_Min and @assetRegion_ID_Max
GROUP BY psf.currency_id0,psf.currency_id1,psf.figuretype_id0,psf.figuretype_id1,figure0)
END
July 5, 2011 at 5:01 am
Thank you for your advice Celko, much appreciated. I will definately read up on ISO-8601 and ISO-11179.
Your data element names are insane. Take deep breath and think about “category_id”; an attribute can be a “<something>_category” or a “<something>_id” but not that silly string of adjectives looking for a noun.
But you carried this design flaw even further to give us “category_type_id”; this is really funny to me because when I teach basic data modeling and RDBMS design, I have used that exact as an exaggerated example in class! You cannot write lampoons and farce any more; someone is exactly doing it!
Unfortunately i'm working here as a c# web developer. I have no control over the design of this particular database. I agree there are many flaws but we are not in a position to re-design.
There is no such things as a “book_id” in the trade; we have an ISBN (International Standard Book Number). Likewise there is an ISO currency_code CHAR(3), etc. You never did any basic research for this mess, did you?
Again, I did not design this database. We are a directory publishing company with an online presence. The book_id is an internal identifier to represent our directory products.
We do put “udf” prefixes and we are the database, where we don't carry if this is for a drop down or anything else in the front end. Actually, good SQL programmers would never use UDFs more than a few times in their entire career; they cannot be optimized and push the engine to row-by-row processing. The worst is table valued functions like your “dbo.GetAccesLevel(@in_access_lvl)”.
I was taught to prefix all my functions with udf. We have a webpage that dynamically builds controls from the database. Therefore the idea behind the control in the name is so that we can identify the function to the group of controls it works with. As there are only 2 developers here it works ok. But I understand future developers may find it fairly difficult to understand. Although we do document everything.
We do not use local variables very much. We put the3 expression that assigns the value in the code, so the optimizer can see it. We use the ANSI/ISO Standard COALESCE and not the old Sybase COALESCE () legacy syntax. We use declarative CASE expressions and not procedural IF-THEN-ELSE control flow. We do not use repeated group to fake an array like pr4ocedrual languages, so your (foobar_0, foobar_1) is a major design flaw and misunderstanding of RDBMS basics.
Noted.
Did you know that COALESCE can take a list of parameters?
COALESCE (COALESCE(PSF.figure_0, PSF.figure_1), 0)
is simply:
COALESCE(PSF.figure_0, PSF.figure_1, 0)
Yes I did. Not sure what you mean by COALESCE (COALESCE(PSF.figure_0, PSF.figure_1), 0) as it is not used like this in the code?
What kind of attribute is a “dataset_id”? What is a dataset? It sounds too generic to be RDBMS; we follow the law of logic called the Law of Identity – “To be is to be something in particular; to be nothing in particular or anything in general is to be nothing at all”
In 25 words or less, you are still writing OO and 1950's procedural code, but you are using SQL. This is like driving screws with a rock.
A dataset_id seems to be a seperate scheme for a company, one company can have many schemes. But again this is from a database that was designed over 20 years ago, it has been redesigned but seems to have had a lot of the old "ideas" carried across. Unfortunately I was not involved in the design or re-design of this database.
To take a simple example of what you wrote:
IF @local_access_region = 3 --Global
SET @local_access_region_min = 1
ELSE IF @local_access_region = 2 --Overseas
SET @local_access_region_min = 2
ELSE
SET @local_access_region_min = 0;
Becomes the more compact optimization CASE expression:
SET @local_access_region_min
= CASE @local_access_region
WHEN 3 THEN 1
WHEN 2 THEN 2
ELSE 0 END;
But where did you use this local variable? One place, a predicate:
AND PSF.access_region
BETWEEN @local_access_region_min
AND @local_access_region_max
take it out and use the expression.
AND PSF.access_region
BETWEEN CASE @local_access_region
WHEN 3 THEN 1
WHEN 2 THEN 2
ELSE 0 END
AND @local_access_region_max
Looking at the local variable, @local_access_region_max, it is only assigned in its declaration to the largest possible integer value. Get rid of it:
AND PSF.access_region
>= CASE @local_access_region
WHEN 3 THEN 1
WHEN 2 THEN 2
ELSE 0 END
This is easy to read, to optimized, saves local variables and should run 1-2 orders of magnitude faster for large data sets. Since we have no specs or DDL and the rest of your procedure is unreadable, I cannot go any further.
Based on a few decades (yes, decades) of cleaning up bad SQL, you ought to be able to get 3-4 orders of magnitude performance improvement by re-do your DDL and throwing out this awful code. But the real payoff will be in maintaining the code.
Here is my raw notes or a re-write from just the code, without specs or DDL.
CREATE FUNCTION Compute_Foobar_Tot – needs valid name
(@dataset_id INTEGER, @in_foobar_category VARCHAR(10), @in_access_lvl INTEGER, @in_currency_code CHAR(3), @in_access_region INTEGER)
RETURNS DECIMAL(20,2)
AS
RETURN
(SELECT SUM(COALESCE(PSF.figure_0, PSF.figure_1, 0)) *
CASE WHEN COALESCE(PSF.figure_0, PSF.currency_code_1)
= <<ISO currency_code>>
OR COALESCE(PSF.figure_type_0, PSF.figure_type_1) <> 1
THEN 1
ELSE (SELECT uk_rate
FROM PFO_Currencies
WHERE currency_code
= COALESCE(PSF.curency_code_0, PSF.currency_code_1))
END AS something_figure
FROM PFO_Schemefigures AS PSF
WHERE PSF.dataset_id = @in_dataset_id
AND COALESCE(PSF.figure_type_0, PSF.figure_type_1) = 1
AND PSF.foobar_category
= (SELECT foobar_category
FROM PFO_Categories
WHERE foobar_category = @in_foobar_category
AND isbn
IN (SELECT isbn
FROM dbo.GetAccesLevel(@in_access_lvl)))--- UDF??
--AND PSF.access_region = COALESCE (@local_access_region, PSF.access_region)
AND APSF.access_region
>= CASE @local_access_region
WHEN 3 THEN 1
WHEN 2 THEN 2
ELSE 0 END
GROUP BY PSF.curency_code_0, PSF.curency_code_1, PSF.figure_type_0, PSF.figure_type_1, figure_0)
END;
Agreed, thank you for your invaluable advice.
Viewing 9 posts - 1 through 8 (of 8 total)
You must be logged in to reply to this topic. Login to reply