Search code examples
sql-servert-sqlstored-proceduresssmsdrop-table

Drop table in Stored Procedure not working properly?


I have a stored procedure which drops a table if it exists, then it re-creates the table & fills it with relevant data, a friend of mine has about the same code, the only real difference is in the column headers for the table.

As an illustration, here's how mine looks (not really, just a representation).

+----+-----+-----+--------+
| ID | Foo | Bar | Number |
+----+-----+-----+--------+
|  1 | x   | x   |      0 |
|  2 | x   | x   |      1 |
+----+-----+-----+--------+

And here's what his might look like

+----+--------+--------+-----+--------+
| ID | BarFoo | FooBar | Num | Suffix |
+----+--------+--------+-----+--------+
|  1 | x      | x      |   0 | a      |
|  2 | x      | x      |   1 | b      |
+----+--------+--------+-----+--------+

Again, these are merely representations of the situation.

As this is to be a school assignment, the teacher will be creating & executing both SP's, however when creating the SP after using another, I get this error:

Msg 207, Level 16, State 1, Procedure XYZ, Line 59
Invalid column name 'Foo'.

Msg 213, Level 16, State 1, Procedure XYZ, Line 61
Column name or number of supplied values does not match table definition.

However, at the start of both stored procedures, we have this:

CREATE PROCEDURE XYZ
AS
BEGIN
    IF EXISTS (SELECT name
               FROM   sysobjects
               WHERE  name = 'TABLENAME'
                      AND xtype = 'u')
        DROP TABLE TABLENAME;

From what I understand, this should remove the entire table? Including table/column definitions & data?

The only fix I've found so far, is to either execute the DROP TABLE separately before creating the stored procedure, which won't work for us as it really has to be within the stored procedure.

Help would be much appreciated :)

EDIT: Here's my ACTUAL code, apart from comments, this is exactly how it looks in my script (excluding other code behind it).

IF EXISTS (SELECT name
           FROM   sysobjects
           WHERE  name = 'BerekenStatistiek'
                  AND xtype = 'p')
    DROP PROCEDURE BerekenStatistiek;


GO
CREATE PROCEDURE BerekenStatistiek
@jaar INT=0
AS
BEGIN
    IF EXISTS (SELECT name
               FROM   sysobjects
               WHERE  name = 'Statistiek'
                      AND xtype = 'u')
        DROP TABLE Statistiek;
    DECLARE @year AS NVARCHAR (4);
    SET @year = CONVERT (NVARCHAR (4), @jaar);
    SELECT *,
           CAST (Kost - Korting + Freight AS MONEY) AS Netto,
           '' AS Richting
    INTO   Statistiek
    FROM   (SELECT   O.Kwartaal,
                     CAST (SUM(O.Kost) AS MONEY) AS Kost,
                     CAST (SUM(O.Korting) AS MONEY) AS Korting,
                     CAST (SUM(O.Freight) AS MONEY) AS Freight
            FROM     (SELECT CASE 
WHEN CONVERT (NVARCHAR (8), OrderDate, 112) BETWEEN @year + '0101' AND @year + '0331' THEN 1 
WHEN CONVERT (NVARCHAR (8), OrderDate, 112) BETWEEN @year + '0401' AND @year + '0630' THEN 2 
WHEN CONVERT (NVARCHAR (8), OrderDate, 112) BETWEEN @year + '0701' AND @year + '0930' THEN 3 
WHEN CONVERT (NVARCHAR (8), OrderDate, 112) BETWEEN @year + '1001' AND @year + '1231' THEN 4 
END AS 'Kwartaal',
                             ROUND(UnitPrice * Quantity, 2) AS Kost,
                             Round((UnitPrice * Quantity) * Discount, 2) AS Korting,
                             Freight
                      FROM   Orders AS O
                             INNER JOIN
                             OrderDetails AS Od
                             ON O.OrderID = Od.OrderID
                      WHERE  CONVERT (NVARCHAR (4), OrderDate, 112) = @year) AS O
            GROUP BY O.Kwartaal) AS O1;
    ALTER TABLE Statistiek ALTER COLUMN Kwartaal INT NOT NULL;
    ALTER TABLE Statistiek ALTER COLUMN Richting NVARCHAR (8);
    ALTER TABLE Statistiek
        ADD PRIMARY KEY (Kwartaal);
...

And here's his code (the insertion of values in the variables are excluded just for readability (his code is a bit more bulky):

IF EXISTS (SELECT name
           FROM   sysobjects
           WHERE  name = 'BerekenStatistiek'
                  AND xtype = 'p')
    BEGIN
        DROP PROCEDURE BerekenStatistiek;
    END


GO
CREATE PROCEDURE BerekenStatistiek
@jaartal INT
AS
BEGIN
    DECLARE @huidigkwartaal AS INT = 1;
    DECLARE @beginmaand AS INT;
    DECLARE @eindmaand AS INT;
    DECLARE @vorige_netto_ontvangsten AS MONEY;
    IF EXISTS (SELECT *
               FROM   sysobjects
               WHERE  name = 'Statistiek'
                      AND xtype = 'U')
        BEGIN
            DROP TABLE Statistiek;
        END
    CREATE TABLE Statistiek
    (
        kwartaalnummer         INT          ,
        beginmaand             INT          ,
        eindmaand              INT          ,
        orderbedrag            MONEY        ,
        korting                MONEY        ,
        vervoerskost           MONEY        ,
        netto_ontvangsten      MONEY        ,
        stijgend_dalend_gelijk NVARCHAR (10)
    );

    --Variables get their data here.

    INSERT  INTO Statistiek (kwartaalnummer, beginmaand, eindmaand, orderbedrag, korting, vervoerskost, netto_ontvangsten, stijgend_dalend_gelijk)
    VALUES                 (@huidigkwartaal, @beginmaand, @eindmaand, @orderbedrag, @korting, @vervoerskost, @netto_ontvangsten, @stijgend_dalend_gelijk);

Solution

  • If you can use a different table name, start with that. And, if the table has to exist only for a moment after the proc is executed so that it can be selected from, then create a global temporary table (i.e. table name starts with ## as in ##MyTable).

    However, if it is a requirement to use the same table name as your classmate, then the teacher is probably trying to get you to learn about deferred object resolution (i.e. @Shannon's answer) and how to get around it, because outside of learning this, the scenario makes no sense since one would never do such a thing in reality.

    Sub-processes (i.e. EXEC and sp_executesql) do not resolve immediately since they aren't executed when creating the stored procedure. So, simplistically, just declare a new NVARCHAR(MAX) variable to hold some Dynamic SQL and put your SELECT statement in there. Use sp_executesql to pass in the @year variable. You are creating a real table so it will survive beyond the subprocess ending and then the ALTER TABLE statement will work.

    Additional notes:

    • You don't really need the ALTER statement to set the datatype of the [Richting] field. Just tell SQL Server what the type is in your SELECT statement:

      CONVERT(NVARCHAR(8), '') AS [Richting]
      
    • You don't really want to do CONVERT(NVARCHAR(8), OrderDate, 112) to compare to a value as it invalidates the use of any indexes that might be on [OrderDate]. Instead, construct a date value from the strings and convert that to a DATETIME or DATE (i.e. CONVERT(DATETIME, @year + '0101')).

      To better understand this issue, please read Sargability: Why %string% Is Slow, and at least the first link at the bottom, which is: What makes a SQL statement sargable?

    • You don't really want to convert the OrderDate field to NVARCHAR(4) just to compare the year, for the same reason as just mentioned in the above point. At the very least using the YEAR() function would be more direct. But if you want to make sure indexes can be used, you can't put a function on the field. But you only want the year. So isn't the year the same as BETWEEN @Year + '0101' AND @Year + '1231'? ;-)

      Interestingly enough, the first example in the accepted answer in the "What makes a SQL statement sargable?" S.O. question linked in the previous bullet is exactly what I am recommending here :).