2

I have one stored procedure to insert into a table. I have two questions:

  1. Is it a ok?

  2. Sometimes the insert operation runs twice. Why?

Microsoft SQL Server 2016 (RTM) - 13.0.1601.5 (X64)
  Apr 29 2016 23:23:58
  Copyright (c) Microsoft Corporation
  Enterprise Edition (64-bit) on Windows 8.1 Enterprise 6.3 (Build 9600: )

CREATE PROCEDURE [dbo].[insert_newNEWS] 
-- Add the parameters for the stored procedure here
@news_owner_id   INT,
@news_owner_type BIT,
@news_cat_id     INT,
@news_producer   NVARCHAR(1000),
@news_title      NVARCHAR(1000),
@news_sutitr     NVARCHAR(1000),
@news_text       NVARCHAR(1000),
@news_date       NVARCHAR(128),
@news_time       NVARCHAR(16),
@news_link       NVARCHAR(512),
@img_uri_type    TINYINT,
@img_uri         XML,
@newsContentID   INT OUTPUT
AS
     BEGIN


         -- SET NOCOUNT ON added to prevent extra result sets from
     -- interfering with SELECT statements.
     SET NOCOUNT ON;
     BEGIN TRANSACTION;
     SET @newsContentID = dbo.Biggest_ID(13);
     INSERT INTO [Contents].[news]
     ([id_news],
      [news_owner_id],
      [news_owner_type],
      [news_cat_id],
      [news_producer],
      [news_title],
      [news_sutitr],
      [news_text],
      [news_date],
      [news_time],
      [news_link],
      [img_uri_type],
      [img_uri]
     )
     VALUES
     (@newsContentID,
      @news_owner_id,
      @news_owner_type,
      @news_cat_id,
      @news_producer,
      @news_title,
      @news_sutitr,
      @news_text,
      @news_date,
      @news_time,
      @news_link,
      @img_uri_type,
      @img_uri
     );
     IF(@@ERROR <> 0)
         BEGIN
             ROLLBACK TRAN;
             RETURN 1;
         END;
     COMMIT;
 END;

AND this is Mr Biggest_ID function

SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO
ALTER FUNCTION [dbo].[Biggest_ID](@table_id INT)
RETURNS INT
AS
     BEGIN
         DECLARE @BiggestID AS INT;
         --another table ID
         IF @table_id = 13
             BEGIN
                 SELECT @BiggestID = ISNULL(MAX(nws.id_news), 0) + 1
                 FROM [Contents].[news] AS nws;
             END;

         RETURN @BiggestID;
     END;

and this table :

SET ANSI_NULLS ON
GO

SET QUOTED_IDENTIFIER ON
GO

CREATE TABLE [Contents].[news](
     [id_news] [int] NOT NULL,
     [news_owner_id] [int] NOT NULL,
     [news_owner_type] [bit] NOT NULL,
[news_cat_id] [int] NOT NULL,
[news_producer] [dbo].[JBText] NOT NULL,
[news_title] [dbo].[JBText] NOT NULL,
[news_sutitr] [dbo].[JBText] NOT NULL,
[news_text] [dbo].[JBText] NOT NULL,
[news_date] [nvarchar](128) NULL,
[news_time] [nvarchar](16) NULL,
[news_link] [dbo].[JBLinks] NULL,
[img_uri_type] [tinyint] NULL,
[img_uri] [xml] NULL,
 CONSTRAINT [PK_news] PRIMARY KEY CLUSTERED 
(
    [id_news] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]

GO
  • Comments are not for extended discussion; this conversation has been moved to chat. – Paul White Feb 9 '17 at 2:34
  • 3
    You should change your id_news column to an IDENTITY(1,1), that will make it incremental so you dont need to handle the id yourself. – Daniel Björk Aug 14 '17 at 11:49
1

I created a test DB and table, I adjusted the schema to make it a bit easier and also commented out the set in the proc.

USE [Test]
GO

/****** Object:  Table [dbo].[news]    Script Date: 2/4/2017 12:21:07 PM ******/
SET ANSI_NULLS ON
GO

SET QUOTED_IDENTIFIER ON
GO

CREATE TABLE [dbo].[news](
    [id_news] [int] NULL,
    [news_owner_id] [int] NULL,
    [news_owner_type] [bit] NULL,
    [news_cat_id] [int] NULL,
    [news_producer] [nvarchar](1000) NULL,
    [news_title] [nvarchar](1000) NULL,
    [news_sutitr] [nvarchar](1000) NULL,
    [news_text] [nvarchar](1000) NULL,
    [news_date] [nvarchar](128) NULL,
    [news_time] [nvarchar](16) NULL,
    [news_link] [nvarchar](512) NULL,
    [img_uri_type] [tinyint] NULL,
    [img_uri] [xml] NULL,
    [newsContentID] [int] NULL
) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]

GO

I'm going to answer 2 first. I ran the proc once, it inserted one time. I looked through the execution plan and it shows a straight forward insert. You can see it here: Standard run. I then set your error handling to 0, just to see the plan with that being triggered. Also straight forward. Error handling triggered. I would run SQL Profiler when you see it insert two records or set up sp_whoisactive to catch the plan when it does multiple inserts. My guess is that it's being executed twice.

For question 1, it looks fine to me. You could put in more exact error handling, but overall I would not say it's bad. I really like this template by spaghetti dba.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.