Need Help with Stored Procedure

  • So, I'm writing a stored procedure for my Database Programming class in college. This one's kind of a convoluted problem, so I'll only tell you what I need help with. Here's my stored procedure:

    CREATE PROC krEagleLetter

    @customercount int,

    @state varchar(2),

    @custname varchar(40),

    @compname varchar(40),

    @currentdate smalldatetime,

    @futuredate smalldatetime,

    @chiefofficer varchar(40),

    @jobtitle varchar(35)

    AS

    SET @currentdate = GETDATE()

    SET @futuredate = DATEADD(ww, 2, GETDATE())

    SELECT @chiefofficer = FirstName + ' ' + LastName, @jobtitle = Jobtitle

    FROM Employee

    WHERE JobTitle = 'Chief Sales Officer'

    SELECT @customercount = COUNT(*)

    FROM Customer

    WHERE State = @state

    IF @customercount > 0

    BEGIN

    WHILE @customercount > 0

    BEGIN

    SELECT @custname = CustFirstName + ' ' + CustLastName,

    @compname = ISNULL(CompanyName, ' ')

    FROM Customer

    WHERE State = @state

    PRINT ' '

    PRINT ' '

    PRINT ' '

    PRINT ' '

    PRINT ' ' + @currentdate

    PRINT 'Dear ' + @custname + ' ,' + @compname

    PRINT ' '

    PRINT ' Eagle is pleased to offer you a 20% discount on any purchase you make prior to'

    PRINT ' 11:55pm ' + @futuredate + '. This limited time offer is our best offer of the year! You can'

    PRINT ' view our entire product line and place your order at Eagle20Deal.com. Make sure to'

    PRINT ' place your order by' + @futuredate + 'because this offer expires at 11:55pm on that day.'

    PRINT ' '

    PRINT ' '

    PRINT ' Sincerely, ' + @chiefofficer

    PRINT @jobtitle

    PRINT ' Eagle Corporation'

    SET @customercount = @customercount - 1

    END

    END

    ELSE

    BEGIN

    PRINT ' '

    PRINT ' There are no customers in this state.'

    END

    It creates the procedure successfully when I run this. But when I try to enter a value and execute it, I get this error:

    Msg 8114, Level 16, State 5, Procedure krEagleLetter, Line 0

    Error converting data type varchar to int.

    I'm not sure what the problem is. If you could help me, I would appreciate it.

  • Sincerely appreciate you being honest about this is for a class. You'd be surprised.

    So, with that in mind, what you're running into is a topic called "Implicit Conversion". You'll want to google those keywords to get up and running.

    Where your problem is coming in is in your huge text concatonation. It's trying to get your text string to be an int for addition, instead of turning your int into text. As an example, your dates will run into similar issues, check out the following:

    DECLARE @blah SMALLDATETIME

    SET @blah = GETDATE()

    DECLARE @whatever VARCHAR(1000)

    -- Comment the next line to remove the error

    SET @whatever = 'abcdef' + @blah

    PRINT @whatever

    SET @whatever = 'abcdef ' + CONVERT( VARCHAR(20), @blah, 101)

    PRINT @whatever

    Hopefully that helps. Let us know if you get stuck on something in your research, we'll try to get you in the right direction again.

    Welcome to the site.


    - Craig Farrell

    Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.

    For better assistance in answering your questions[/url] | Forum Netiquette
    For index/tuning help, follow these directions.[/url] |Tally Tables[/url]

    Twitter: @AnyWayDBA

  • The error is kind of misleading. You would think that is a problem with an int value when it's actually with a smalldatetime and a string which SQL Server tries to convert to int and fails. If you convert your @futuredate variable, you'll get rid of that error, eg. CONVERT( char(12), @futuredate, 107).

    Other than that, I feel sorry for you if this is what you're learning in a DB Programming class. This is procedural code (also called RBAR around here) and you should be learning set-based programming which is the base to good DB Programming. Try to read about RBAR and how to avoid it, it should give you extra points. 😉

    Luis C.
    General Disclaimer:
    Are you seriously taking the advice and code from someone from the internet without testing it? Do you at least understand it? Or can it easily kill your server?

    How to post data/code on a forum to get the best help: Option 1 / Option 2
  • Thank you for your help, both of you. But while I think I understand what you're getting at, I guess I was kind of foggy on how to fix it:

    CREATE PROC krEagleLetter

    @customercount int,

    @state char(2),

    @custname varchar(40),

    @compname varchar(40),

    @currentdate smalldatetime,

    @futuredate smalldatetime,

    @chiefofficer varchar(40),

    @jobtitle varchar(35)

    AS

    SET @currentdate = CONVERT(char(12), GETDATE(), 107)

    SET @futuredate = CONVERT(char(12), (DATEADD(ww, 2, GETDATE())), 107)

    SELECT @chiefofficer = CONVERT(varchar(15), FirstName, 107) + ' ' + CONVERT(varchar(20), LastName, 107), @jobtitle = Jobtitle

    FROM Employee

    WHERE JobTitle = 'Chief Sales Officer'

    SELECT @customercount = COUNT(*)

    FROM Customer

    WHERE State = @state

    IF @customercount > 0

    BEGIN

    WHILE @customercount > 0

    BEGIN

    SELECT @custname = CONVERT(varchar(15), CustFirstName, 107) + ' ' + CONVERT(varchar(20), CustLastName, 107),

    @compname = ISNULL(CompanyName, ' ')

    FROM Customer

    WHERE State = @state

    PRINT ' '

    PRINT ' '

    PRINT ' '

    PRINT ' '

    PRINT ' ' + CONVERT(char(12), @currentdate, 107)

    PRINT 'Dear ' + CONVERT(varchar(40), @custname, 107) + ' ,' + CONVERT(varchar(40), @compname, 107)

    PRINT ' '

    PRINT ' Eagle is pleased to offer you a 20% discount on any purchase you make prior to'

    PRINT ' 11:55pm ' + CONVERT(char(12), @futuredate, 107) + '. This limited time offer is our best offer of the year! You can'

    PRINT ' view our entire product line and place your order at Eagle20Deal.com. Make sure to'

    PRINT ' place your order by' + CONVERT(char(12), @futuredate, 107) + 'because this offer expires at 11:55pm on that day.'

    PRINT ' '

    PRINT ' '

    PRINT ' Sincerely, ' + CONVERT(varchar(40), @chiefofficer, 107)

    PRINT @jobtitle

    PRINT ' Eagle Corporation'

    SET @customercount = @customercount - 1

    END

    END

    ELSE

    BEGIN

    PRINT ' '

    PRINT ' There are no customers in this state.'

    END

    I literally converted every one of those variables (tried just the date variables first, didn't work), and still got the same error. I'm still not sure what I'm supposed to do here.

  • You don't have to convert every single value and you certainly don't have to use the format code on the values. My guess is that your problem is in one of your SELECTs. Either JobTitle or State could be integers. If so, you might need to add further logic.

    Otherwise, the procedure must run just fine.

    CREATE TABLE Employee(

    FirstName varchar(20),

    LastName varchar(20),

    Jobtitle varchar(35))

    INSERT INTO Employee VALUES ('Luis', 'Cazares', 'Chief Sales Officer')

    CREATE TABLE Customer(

    CustFirstName varchar(20),

    CustLastName varchar(20),

    CompanyName varchar(35),

    State char(2)

    )

    INSERT INTO Customer VALUES ('Mike', 'Wazowski', 'Monsters Inc.', 'TX')

    GO

    CREATE PROC krEagleLetter

    @customercount int,

    @state varchar(2),

    @custname varchar(40),

    @compname varchar(40),

    @currentdate smalldatetime,

    @futuredate smalldatetime,

    @chiefofficer varchar(40),

    @jobtitle varchar(35)

    AS

    SET @currentdate = GETDATE()

    SET @futuredate = DATEADD(ww, 2, GETDATE())

    SELECT @chiefofficer = FirstName + ' ' + LastName, @jobtitle = Jobtitle

    FROM Employee

    WHERE JobTitle = 'Chief Sales Officer'

    SELECT @customercount = COUNT(*)

    FROM Customer

    WHERE State = @state

    IF @customercount > 0

    BEGIN

    WHILE @customercount > 0

    BEGIN

    SELECT @custname = CustFirstName + ' ' + CustLastName,

    @compname = ISNULL(CompanyName, ' ')

    FROM Customer

    WHERE State = @state

    PRINT ' '

    PRINT ' '

    PRINT ' '

    PRINT ' '

    PRINT ' ' + @currentdate

    PRINT 'Dear ' + @custname + ' ,' + @compname

    PRINT ' '

    PRINT ' Eagle is pleased to offer you a 20% discount on any purchase you make prior to'

    PRINT ' 11:55pm ' + CONVERT( char(12), @futuredate, 107) + '. This limited time offer is our best offer of the year! You can'

    PRINT ' view our entire product line and place your order at Eagle20Deal.com. Make sure to'

    PRINT ' place your order by ' + CONVERT( char(12), @futuredate, 107) + ' because this offer expires at 11:55pm on that day.'

    PRINT ' '

    PRINT ' '

    PRINT ' Sincerely, ' + @chiefofficer

    PRINT @jobtitle

    PRINT ' Eagle Corporation'

    SET @customercount = @customercount - 1

    END

    END

    ELSE

    BEGIN

    PRINT ' '

    PRINT ' There are no customers in this state.'

    END

    GO

    EXEC krEagleLetter 0, 'TX', NULL, NULL, NULL, NULL, NULL, NULL

    GO

    DROP TABLE Employee

    DROP TABLE Customer

    Another problem is that you have parameters that should be variables.

    REFERENCE: http://msdn.microsoft.com/en-us/library/ms187928.aspx

    Luis C.
    General Disclaimer:
    Are you seriously taking the advice and code from someone from the internet without testing it? Do you at least understand it? Or can it easily kill your server?

    How to post data/code on a forum to get the best help: Option 1 / Option 2
  • It seemed that the main issue was actually the State variable. Apparently, it had been confused for an integer; after using CONVERT that one, I stopped receiving the error.

  • Nice work.

    And well done on asking for help the best way possible. Keep coming back and asking for help like that and I know you'll receive it on this site.

    "The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
    - Theodore Roosevelt

    Author of:
    SQL Server Execution Plans
    SQL Server Query Performance Tuning

  • Hmm, seems to be that code would list the same customer, the "first" one, over and over rather than listing every individual customer in the state. Are you actually getting a list with different customers?

    SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.

  • ScottPletcher (10/8/2014)


    Hmm, seems to be that code would list the same customer, the "first" one, over and over rather than listing every individual customer in the state. Are you actually getting a list with different customers?

    Not precisely the first. As there's no ORDER BY, it'd be more accurate to say "a random customer".

    Luis C.
    General Disclaimer:
    Are you seriously taking the advice and code from someone from the internet without testing it? Do you at least understand it? Or can it easily kill your server?

    How to post data/code on a forum to get the best help: Option 1 / Option 2
  • Luis Cazares (10/8/2014)


    ScottPletcher (10/8/2014)


    Hmm, seems to be that code would list the same customer, the "first" one, over and over rather than listing every individual customer in the state. Are you actually getting a list with different customers?

    Not precisely the first. As there's no ORDER BY, it'd be more accurate to say "a random customer".

    That's why I put "first" in quotes. And actually, upon further reflection, it would return the "last" read row every time.

    But "random" is not accurate either: it's not entirely random, as SQL won't stop in the middle of a page to pull a row.

    SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.

  • You're right, I just wanted to point out that there's no guarantee of the order of the results.

    Luis C.
    General Disclaimer:
    Are you seriously taking the advice and code from someone from the internet without testing it? Do you at least understand it? Or can it easily kill your server?

    How to post data/code on a forum to get the best help: Option 1 / Option 2
  • Would have been better if you gave the values (some sample execution values) you were getting error for..

    That would have helped in pin pointing the error..

    Thanks

    "The price of anything is the amount of life you exchange for it" - Henry David Thoreau
  • CELKO (10/9/2014)

    1. You have many ISO-11179 violations. Do you have only one Employee, as you said? The correct name is “Personnel”; an abstract set of elements.

    2. A procedure is named “<verb>_<object>”;

    3. We have a DATE data type.

    4. The USPS standard for a mailing label is CHAR(35).

    8. You do not understand that in declarative languages, we do not use local variables, scratch files, etc.

    CREATE PROCEDURE Build_Eagle_Letter

    (... CHAR(2),

    ... INTEGER,

    ... VARCHAR(2000))

    ...

    1. I really don't have the free hours you do to go over ISO regs before choosing every table name. Besides, those types of names will be unnatural and uncommon to future employees, i.e. developers, who have to use those tables. Everyone is used to seeing "Employee", not "Personnel".

    2. Is there actually an ANSI standard that proclaims that too? What do you do for more than one action taken or more than one object affected?

    3. SQL 2005 does not have a "date" type, ergo we don't use it if we have to support such dbs.

    4. Who cares? I'm not developing for USPS, and I don't have their scrubbed data. Besides, I wouldn't used fix-length space for an address column in most cases anyway.

    8. Really? Can you name one major RDBMS that doesn't allow variables of some type?

    Finally, we use lowercase for data types, because in SQL Server type names are case sensitive in many contexts. We do not want to cause unexpected future errors when code needs to be run on a case-sensitive instance or against a remote instance.

    SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.

  • CELKO (10/9/2014)

    A competent professional in any trade would do it right without any special effort. Does a doctor think about stitches, or does he just do them? He would also see continued education as part of the job. How many thousands of $$ do you spend on books, training classes, conferences or other professional development?

    Huge amount on books. But I can't afford to waste time on zero-payback things. I hope my doctor is studying actual medicines more than the latest way to name every staff job in his office.

    So you still use six letter variable names because of FORTRAN I? Hey, everybody is used to seeing that, aren't they?

    Absolutely not. You're many decades behind. Most SQL developers don't even know what COBOL and FORTRAN are. I have to get people hirable now up to speed quickly, and I can't waste their time on such nonsense.

    2. Is there actually an ANSI standard that proclaims that too? What do you do for more than one action taken or more than one object affected?

    INCITS L8 Metadata Standards notes. Yes, it is hard to name a module that stacks wood, comforts old people and computes the square root. While you were not reading the standards of your profession, I will bet you never learned basic software engineering. Look up cohesion and coupling. The principles are that a module of code (any language) should have high cohesion – that means it does one and only one clear task. Coupling means that we want modules that are independent of each other (think math functions).

    The number of schema objects used has nothing to do with this concept. Your mindset is still in tape files, not RDBMS!

    Again, who the hell has used tape for anything but backups in 20 years?! At least join the 1990s if you can't all the way to 2014!

    You're the one trying to dictate a single naming approach for all procs. You just can't always effectively use large code bases that way. Trying to navigate system procs would be a nightmare if MS tried to always name procs that way. Sometimes it really helps to put a functional area first in the name, such as "sp_syscollector_*" or "sp_syspolicy_*" procs in SQL Server. We need to use that some in our business too.

    3. SQL 2005 does not have a "date" type, ergo we don't use it if we have to support such databases

    This is 2014; why are you a decade behind in upgrading?

    Budgets and time. Again, not everyone else has the unlimited time you do for non-payback (directly) items.

    4. Who cares? I'm not developing for USPS, ...

    Don't know what you're even ranting about here. You were the one that noted the USPS "standard", as if we should care about or align to it.

    8. Really? Can you name one major RDBMS that doesn't allow variables of some type?

    Not inside the DML; try to put on[e] inside a SELECT. ... We also have cursors; do you use them? In SQL, the loss is 2-3 order of magnitude in performance!

    Variables are used in Oracle and SQL all the time inside selects. Parameters to a proc are variables. Yes, I do use cursors for some things. They are often better than the alternatives, particularly with low volumes of data.

    Finally, we use lowercase for data types, because in SQL Server type names are case sensitive in many contexts. We do not want to cause unexpected future errors when code needs to be run on a case-sensitive instance or against a remote instance.

    The reason we prefers uppercase is a visual phenomena called a Bouma. Keywords are read as a single unit, not as letters. We found that pretty printing code could save 8-12% of the maintenance time when I did research for AIRMICS.

    But the code abends if you do that! I won't willingly write code that fails sometimes and works other times. Anyone pedantic and supercilious enough to allow production data failures because of "Bouma" should be fired. And in the real world, they would be.

    SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.

  • The one thing that you missed in this is your database tables. It's easy to just focus on the code, as "that's where the problem is, right"? But as you gain experience with SQL you will find you are only looking at half the problem. You should always "script out" your tables and indexes when you are troubleshooting, SSMS will generate create/alter/etc scripts for your tables. This way you know the datatypes of all of the fields you are working with and you will have all of info you need to solve the problem. You can also use sp_help('TableName') and it will also show you the table structure, indexes, etc. too. These are valuable "tricks" you should keep in mind as you learn to use SQL.

    Welcome to SQL Development! As a long time developer and IT manager I have come to love the raw power of the SQL language. It seems very simple at initial glance, but you can really do some amazing things with it if you learn the nuances.

    I hope you enjoy your class!!!

Viewing 15 posts - 1 through 15 (of 20 total)

You must be logged in to reply to this topic. Login to reply