¿Este procedimiento almacenado es seguro para subprocesos? (o lo que sea que el equivalente esté en SQL Server)

Question

Con la ayuda de otros en SO, esta mañana he desbaratado un par de Tablas y Procedimientos almacenados, ya que estoy lejos de ser un programador de DB.

¿Alguien le importaría echar un vistazo a esto y decirme si es seguro para subprocesos? Supongo que probablemente no sea el término que usan los desarrolladores de bases de datos/bases de datos, pero espero que entiendas la idea: básicamente, ¿qué sucede si esta sp se está ejecutando y otra aparece al mismo tiempo? ¿Podría uno interferir con el otro? ¿Esto es incluso un problema en SQL/SP?

CREATE PROCEDURE [dbo].[usp_NewTicketNumber] 
    @ticketNumber int OUTPUT 
AS 
BEGIN 
    SET NOCOUNT ON; 
    INSERT INTO [TEST_Db42].[dbo].[TicketNumber] 
       ([CreatedDateTime], [CreatedBy]) 
     VALUES 
       (GETDATE(), SUSER_SNAME()) 
    SELECT @ticketNumber = IDENT_CURRENT('[dbo].[TicketNumber]'); 
    RETURN 0; 
END 

Answer 1

Es probable que no quiere estar usando IDENT_CURRENT - esto devuelve la última de identidad generado en la tabla en cuestión, en cualquier sesión y cualquier ámbito. Si alguien más hace una inserción en el momento equivocado, ¡obtendrá su identificación en su lugar!

Si desea obtener la identidad generada por la inserción que acaba de realizar, entonces es mejor usar la cláusula OUTPUT para recuperarla. Solía ​​ser habitual utilizar SCOPE_IDENTITY() para esto, pero hay problemas con los planes de ejecución paralelos.

El equivalente SQL principal de seguridad de subprocesos es cuando se ejecutan varias sentencias que provocan un comportamiento inesperado o indeseable. Los dos tipos principales de comportamiento que puedo pensar son bloqueo (en particular, bloqueos) y problemas de concurrencia.

Los problemas de bloqueo ocurren cuando una instrucción detiene el acceso de otras declaraciones a las filas con las que está trabajando. Esto puede afectar el rendimiento y, en el peor de los casos, dos declaraciones realizan cambios que no se pueden conciliar y se produce un interbloqueo, lo que hace que una instrucción finalice.

Sin embargo, un simple inserto como el que tiene no debe causar bloqueos a menos que esté involucrado algo más (como transacciones de base de datos).

Los problemas de concurrencia (describiéndolos muy deficientemente) son causados ​​por un conjunto de cambios en los registros de la base de datos que sobrescriben otros cambios en los mismos registros. De nuevo, esto no debería ser un problema al insertar un registro.

Answer 2

En primer lugar, ¿por qué no devuelve el nuevo número de ticket en lugar de 0 todo el tiempo? ¿Alguna razón en particular para eso?

En segundo lugar, para estar absolutamente seguro, debe envolver su instrucción INSERT y SELECT en una TRANSACCIÓN para que nada desde el exterior pueda intervenir.

En tercer lugar, con SQL Server 2005 en adelante, envolvería mis declaraciones en un TRY .... CATCH bloqueará y retrotraerá la transacción si falla.

A continuación, trataría de evitar especificar el servidor de la base de datos (TestDB42) en mis procedimientos siempre que sea posible. ¿Qué ocurre si desea implementar ese proceso en un nuevo servidor (TestDB43)?

Y, por último, nunca utilizaría un SET NOCOUNT en un procedimiento almacenado - puede hacer que la persona que llama crea erróneamente que el proceso almacenado falló (vea mi comentario a gbn a continuación). Este es un problema potencial si está usando Solo objetos ADO.NET SqlDataAdapter, vea MSDN docs sobre cómo modificar datos ADO.NET con SqlDataAdapter para más explicaciones).

Así que mi sugerencia para su proc almacenado sería:

CREATE PROCEDURE [dbo].[usp_NewTicketNumber] 
AS 
BEGIN 
    DECLARE @NewID INT 

    BEGIN TRANSACTION 
    BEGIN TRY 
    INSERT INTO 
     [dbo].[TicketNumber]([CreatedDateTime], [CreatedBy]) 
    VALUES 
     (GETDATE(), SUSER_SNAME()) 

    SET @NewID = SCOPE_IDENTITY() 

    COMMIT TRANSACTION 
    END TRY 
    BEGIN CATCH 
    ROLLBACK TRANSACTION 
    SET @NewID = -1 
    END CATCH 

    RETURN @NewID 
END 

Marc

Answer 3

CREATE PROCEDURE [dbo].[usp_NewTicketNumber] 
    @NewID int OUTPUT 
AS 
BEGIN 
    SET NOCOUNT ON; 
    BEGIN TRY 
     BEGIN TRANSACTION 
     INSERT INTO 
      [dbo].[TicketNumber] ([CreatedDateTime], [CreatedBy]) 
     VALUES 
      (GETDATE(), SUSER_SNAME()) 

     SET @NewID = SCOPE_IDENTITY() 

     COMMIT TRANSACTION; 
    END TRY 
    BEGIN CATCH 
     IF XACT_STATE() <> 0 
      ROLLBACK TRANSACTION; 
     SET @NewID = NULL; 
    END CATCH 
END 

yo no usaría RETURN para los datos de uso significativo: ya sea de registros o parámetro de salida. RETORNO normalmente se utiliza para los estados de error (como procs almacenados del sistema hacen en la mayoría de los casos):

EXEC @rtn = EXEC dbo.uspFoo 
IF @rtn <> 0 
    --do error stuff 

También puede utilizar la cláusula OUTPUT para devolver un conjunto de registros en su lugar.

Esto es "thread safe", es decir, se puede ejecutar al mismo tiempo.

Answer 4

Estoy de acuerdo con la respuesta de David Hall, solo quiero ampliar un poco sobre por qué ident_corrent es absolutamente incorrecto de usar en esta situación.

Tenemos un desarrollador aquí que lo usó. La inserción de la aplicación cliente sucedió al mismo tiempo que la base de datos estaba importando millones de registros a través de una importación automatizada. La identificación que le devolvieron pertenecía a uno de los registros que importó mi proceso. Usó esta identificación para crear registros para algunas tablas secundarias que ahora estaban adjuntas al registro incorrecto. Peor aún, ahora no tenemos idea de cuántas veces sucedió esto antes de que alguien no pudiera encontrar la información que debería haber estado en las tablas secundarias (su cambio había estado en prod durante varios meses). Mi importación automática no solo ha interferido con su código, sino que otro usuario que inserta un registro en el momento podría haber hecho lo mismo. Ident_current nunca debe usarse para devolver la identidad de un registro recién insertado, ya que no está limitado al proceso que lo llama.

Answer 5

La forma más segura de hacerlo aquí sería utilizar la cláusula Output, ya que existe un error conocido en scope_idendity en determinadas circunstancias (procesamiento múltiple/paralelo).

CREATE PROCEDURE [dbo].[usp_NewTicketNumber] 
AS 
BEGIN 
    DECLARE @NewID INT 

    BEGIN TRANSACTION 
    BEGIN TRY 
    declare @ttIdTable TABLE (ID INT) 
    INSERT INTO 
     [dbo].[TicketNumber]([CreatedDateTime], [CreatedBy]) 
    output inserted.id into @ttIdTable(ID) 
    VALUES 
     (GETDATE(), SUSER_SNAME()) 

    SET @NewID = (SELECT id FROM @ttIdTable) 

    COMMIT TRANSACTION 
    END TRY 
    BEGIN CATCH 
    ROLLBACK TRANSACTION 
    SET @NewID = -1 
    END CATCH 

    RETURN @NewID 
END 

De esta manera usted debe ser seguro para subprocesos, ya que la cláusula de salida utiliza los datos que el insert inserta en realidad, y no tendrá problemas a través de ámbitos o sesiones.