16

I have this function I am trying to create. When I parse it, it works fine, but to actually create the function in the database it says my column names are invalid. That is not true, I spelled them correctly. Here is the code:

ALTER FUNCTION [dbo].[fnTally] (@SchoolId nvarchar(50))
RETURNS int

AS 

BEGIN 

DECLARE @Final nvarchar
IF EXISTS (

    SELECT 
        question, 
        yes_ans, 
        no_ans, 
        na_ans, 
        blank_ans 
    FROM dbo.qrc_maintally 
    WHERE school_id = @SchoolId 

)

    IF yes_ans > no_ans AND yes_ans > na_ans 
    BEGIN
        SET @Final = 'Yes'
    END

    ELSE IF no_ans > yes_ans AND no_ans > na_ans 
    BEGIN
        SET @Final = 'No'
    END

    ELSE IF na_ans > yes_ans AND na_ans > no_ans 
    BEGIN
        SET @Final = 'N/A'
    END

RETURN @Final

END
Improve this question
7
  • Is school_id the PK on that table? i.e. can there ever be more than one matching row? Commented Feb 17, 2011 at 16:40
  • @G_M refresh, I did that a few minutes ago. Commented Feb 17, 2011 at 16:43
  • There is no PK in this table but yes it could be used as one. And the values of yes_ans, no_ans, etc are integers so I'm trying to compare each row per schoolId and which ever has the greatest value, then output a final value. Commented Feb 17, 2011 at 16:43
  • @Salim - Well if more than 1 row matches which row should be used in the calculation? Also why is your function declared as RETURNS int when it actually returns varchar? Also you need to declare a length for all your varchar variables. e.g. DECLARE @Final VARCHAR(3) Commented Feb 17, 2011 at 16:47
  • If more than one row matches then another review will be done on our site and that reviewer will be a case breaker. Commented Feb 17, 2011 at 16:49

6 Answers 6

Reset to default
9 
ALTER FUNCTION [dbo].[fnTally] (@SchoolId nvarchar(50))
    RETURNS nvarchar(3)
AS BEGIN 

    DECLARE @Final nvarchar(3)
    SELECT @Final = CASE 
        WHEN yes_ans > no_ans  AND yes_ans > na_ans THEN 'Yes'
        WHEN no_ans  > yes_ans AND no_ans  > na_ans THEN 'No'
        WHEN na_ans  > yes_ans AND na_ans  > no_ans THEN 'N/A' END
    FROM dbo.qrc_maintally
    WHERE school_id = @SchoolId

Return @Final
End

As you can see, this simplifies the code a lot. It also makes other errors in your code more obvious: you're returning an nvarchar, but declared the function to return an int (corrected in the code above).

Improve this answer
Sign up to request clarification or add additional context in comments.

10 Comments

DECLARE @Final nvarchar needs a length or will get truncated at 1 character.
@Martin - copied that from the original question. I can't put a length there until the OP tells us what it needs to be.
@Joel varchar(3) should work fine with the string literals in the code.
@Joel Coehoom: Martin's right. But there's another problem too. You are returning an nvarchar while the function is declared to return int.
@Martin - okay, you got me, but in my defense I don't think he needs to declare a return variable at all (updated my answer to reflect this).
|
8

You'll need to create local variables for those columns, assign them during the select and use them for your conditional tests.

declare @yes_ans int,
        @no_ans int,
        @na_ans int

SELECT @yes_ans = yes_ans, @no_ans = no_ans, @na_ans = na_ans 
    from dbo.qrc_maintally 
    where school_id = @SchoolId

If @yes_ans > @no_ans and @yes_ans > @na_ans 
begin
Set @Final = 'Yes'
end
-- etc.
Improve this answer

7 Comments

On the right track, but we can do even better by assigning to @Final directly from a CASE statement
@Joel - Assuming of course that only one row can match the condition.
@Martin - you need that for the variables here, too
The comment was really about the degree of "right trackness" not specifically anything to do with the CASE statement. The OP says multiple rows can be returned for a schoolid but I'm still somewhat baffled by their explanation so +1.
Specifically each schoolId has about 18 rows since there are 18 questions its confusing explaining without visually seeing it.
|
3

No one seems to have picked that if (yes=no)>na or (no=na)>yes or (na=yes)>no, you get NULL as the result. Don't believe this is what you are after.

Here's also a more condensed form of the function, which works even if any of yes, no or na_ans is NULL.

USE [***]
GO
/****** Object:  UserDefinedFunction [dbo].[fnActionSq]    Script Date: 02/17/2011 10:21:47 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
ALTER FUNCTION [dbo].[fnTally] (@SchoolId nvarchar(50))
RETURNS nvarchar(3)
AS 
BEGIN
return (select (
       select top 1 Result from
       (select 'Yes' Result, yes_ans union all
        select 'No', no_ans union all
        select 'N/A', na_ans) [ ]
        order by yes_ans desc, Result desc)
       from dbo.qrc_maintally
       where school_id = @SchoolId)
End
Improve this answer

4 Comments

Thanks Richard I could try this as well although the webform does not allow nulls anyway
This works as a select query but not in code behind as a function, doesn't spit anything back out
A variable still needs to be declared for receiving the result of the query and for being pushed out as the result of the function.
I hadn't and now I have. Obviously I was wrong, sorry. There probably was a similar situation in a different context where it was mandatory to use a variable (or at least it was impossible to pull the result of a subquery directly), and I managed to confuse it with RETURN. Can't remember what it was, though.
2 
If yes_ans > no_ans and yes_ans > na_ans

You're using column names in a statement (outside of a query). If you want variables, you must declare and assign them.

Improve this answer

Comments

2

I think you'd be better off with a CASE statement, which works a lot more like IF/ELSEIF

DECLARE @this int, @value varchar(10)
SET @this = 200
SET @value = (
SELECT 
CASE
    WHEN @this between 5 and 10 THEN 'foo'
    WHEN @this between 10 and 15 THEN 'bar'
    WHEN @this < 0 THEN 'barfoo'
    ELSE 'foofoo'
    END
)

More info: http://technet.microsoft.com/en-us/library/ms181765.aspx

Improve this answer

Comments

0

Look at these lines:

If yes_ans > no_ans and yes_ans > na_ans

and similar. To what do "yes_ans" etc. refer? You're not using these in the context of a query; the "if exists" condition doesn't extend to the column names you're using inside.

Consider assigning those values to variables you can then use for your conditional flow below. Thus,

if exists (some record)
begin
   set @var = column, @var2 = column2, ...

   if (@var1 > @var2)
      -- do something

end

The return type is also mismatched with the declaration. It would help a lot if you indented, used ANSI-standard punctuation (terminate statements with semicolons), and left out superfluous begin/end - you don't need these for single-statement lines executed as the result of a test.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.