I've made a stored procedure to get practice with cursors, I've a problem with special characters, for instance, if last_name contains a single quote, I've got an error, I need to escape it in some way, how could I do that? I don't know which special characters are contained in these fields, I've tried with QUOTENAME(d.last_name) but it didn't work

CREATE OR alter PROCEDURE list_employees
AS
BEGIN 
    DECLARE cursore CURSOR FAST_FORWARD FOR SELECT TOP(20) d.id, d.first_name, d.last_name, cd.contact
          FROM employees d 
          JOIN contacts cd ON cd.fk_employee= d.id
          ORDER BY d.id;
    DECLARE @id_employee VARCHAR(36);
    DECLARE @first_name VARCHAR(50);
    DECLARE @last_name VARCHAR(50);
    DECLARE @contact VARCHAR(255);
    DECLARE @insert_statement varchar(1000);
     IF OBJECT_ID('dbo.list_employees', 'U') IS NOT NULL 
       BEGIN
          DROP TABLE dbo.list_employees; 
       END
    OPEN cursore;
    FETCH NEXT FROM cursore INTO @id_employee , @first_name , @cognome, @contatto ;
    if(@@FETCH_STATUS = 0)
       BEGIN
         CREATE TABLE dbo.list_employees(id_employee VARCHAR(36), first_name VARCHAR(50), last_name VARCHAR(50), contact VARCHAR(255))
       END

    WHILE @@FETCH_STATUS = 0
    BEGIN 
      SET @insert_statement = 'INSERT INTO list_employees SELECT ''' @id_employee  ''', ''' @first_name  ''', ''' @last_name  ''','''  @contact  ''''
     exec(@insert_statement ) 

        FETCH NEXT FROM cursore INTO @id_employee , @first_name , @last_name , @contact ;
        END 
        CLOSE cursore;
        DEALLOCATE cursore;
    END;

CodePudding user response：

Since your code drops an existing table and then recreates it I suspect this procedure is an odd way of getting the "current top 20". Instead of using a cursor and all sorts of hassle this would be massively simplified to use a view. There is no need to constantly drop a table and repopulate it.

Here is what your view might look like.

create or alter view list_employees as
    SELECT TOP(20) d.id
        , d.first_name
        , d.last_name
        , cd.contact
    FROM employees d 
    JOIN contacts cd ON cd.fk_employee = d.id
    ORDER BY d.id;

CodePudding user response：

Firstly, let's cover off why what you have isn't working; this is because you are injecting values that should be parameters. Specifically these 2 lines are the cause:

SET @insert_statement = 'INSERT INTO list_employees SELECT ''' @id_employee  ''', ''' @first_name  ''', ''' @last_name  ''','''  @contact  ''''
exec(@insert_statement ) 

There are, in truth, 2 bad habits here:

1. injection of non-sanitised values (a huge security vulnerability)
2. Use of EXEC(@SQL) syntax, rather than sys.sp_executesql, meaning you can't parametrise your statement.

If you parametrise your statement, then the problem you have goes away:

SET @insert_statement = 'INSERT INTO dbo.list_employees (id_employee,first_name,last_name,contact) VALUES (@id_employee,@first_name,@last_name,@contact);';
EXEC sys.sp_executesql @insert_statement, N'@id_employee VARCHAR(36),@first_name VARCHAR(50),@last_name VARCHAR(50),@contact VARCHAR(255)', @id_employee,@first_name,@last_name,@contact;

Of course, this poses the question of why use dynamic SQL at all, there's nothing dynamic about the statement. It's not that the table might not exist prior to the execution, as if a table doesn't exist the validation can be deferred by the engine, and you CREATE the table in the same scope. Perhaps the definition of the table is changing? I hope not.

As also mentioned, however, the CURSOR isn't really required here. Although you state you are practicing them, there are very few times that they are ever needed, and changing something like this to use an RBAR CURSOR will be terrible for performance. You really should be using a set based INSERT:

INSERT INTO dbo.list_employees (id_employee,first_name,last_name,contact)
SELECT TOP (20)
       e.id,
       e.first_name,
       e.last_name,
       c.contact
FROM dbo.employees e
     JOIN dbo.contacts c ON c.fk_employee= e.id
ORDER BY e.id;

Or, better yet, use a VIEW as Sean demonstrates in their answer.