How to improve while loop insert performance in sq

Question

Here is my SQL Query. It's insert almost 6500+ row from temp table. But its takes 15+ mins! . How can i improve this ? Thanks

ALTER proc [dbo].[Process_bill]
@userid varchar(10),
@remark nvarchar(500),
@tdate date ,
@pdate date
as

BEGIN 


IF OBJECT_ID('tempdb.dbo..#temptbl_bill', 'U') IS NOT NULL
    DROP TABLE #temptbl_bill; 

CREATE TABLE #temptbl_bill (
   RowID int IDENTITY(1, 1), 
   ------------

)

// instert into temp table

DECLARE @NumberRecords int, @RowCounter int
DECLARE @batch INT
SET @batch   = 300
SET @NumberRecords = (SELECT COUNT(*) FROM #temptbl_bill)
SET @RowCounter = 1

SET NOCOUNT ON  

BEGIN TRANSACTION

WHILE @RowCounter <= @NumberRecords
BEGIN

declare @clid int
declare @hlid int
declare @holdinNo nvarchar(150)
declare @clientid nvarchar(100)
declare @clientName nvarchar(50)
declare @floor int
declare @radius nvarchar(50)
declare @bill money 
declare @others money
declare @frate int
declare @due money
DECLARE @fine money
DECLARE @rebate money

IF @RowCounter > 0 AND ((@RowCounter % @batch = 0) OR (@RowCounter = @NumberRecords))           
BEGIN                   
    COMMIT TRANSACTION
    PRINT CONCAT('Transaction #', CEILING(@RowCounter/ CAST(@batch AS FLOAT)), ' committed (', @RowCounter,' rows)');
    BEGIN TRANSACTION
END;

 // multiple select 


// insert to destination  table

Print 'RowCount -' +cast(@RowCounter as varchar(20))  + 'batch -' + cast(@batch as varchar(20))

SET @RowCounter = @RowCounter + 1;

END

COMMIT TRANSACTION

PRINT CONCAT('Transaction #', CEILING(@RowCounter/ CAST(@batch AS FLOAT)), ' committed (',
@RowCounter,' rows)');

SET NOCOUNT OFF 

DROP TABLE #temptbl_bill

END

GO

Answer 1

As has been said in comments, the loop is completely unnecessary. The way to improve the performance of any loop is to remove it completely. Loops are a last resort in SQL.

As far as I can tell your insert can be written with a single statement:

INSERT tbl_bill(clid, hlid, holdingNo,ClientID, ClientName, billno, date_month, unit, others, fine, due, bill, rebate, remark, payment_date, inserted_by, inserted_date) 
SELECT  clid = c.id, 
        hlid = h.id,
        h.holdinNo ,
        c.cliendID, 
        clientName = CAST(c.clientName AS NVARCHAR(50)), 
        BillNo = CONCAT(h.holdinNo, MONTH(@tdate), YEAR(@tdate)),
        date_month = @tDate,
        unit = 0,
        others = CASE WHEN h.hfloor = 0 THEN rs.frate * (h.hfloor - 1) ELSE 0 END, 
        fine = bs.FineRate * b.Due / 100, 
        due  = b.Due, 
        bill = @bill,   -- This is declared but never assigned
        rebate = bs.rebate,
        remark = @remark,
        payment_date = @pdate,
        inserted_by = @userid, 
        inserted_date = GETDATE()
FROM    (   SELECT  id, clientdID, ClientName
            FROM    tbl_client 
            WHERE   status = 1
        ) AS c
        INNER JOIN 
        (   SELECT  id, holdinNo, [floor], connect_radius
            FROM    tx_holding 
            WHERE   status = 1 
            AND     connect_radius <> '0' 
            AND     type = 'Residential'
        ) AS h
            ON c.id = h.clid
        LEFT JOIN tbl_radius_setting AS rs
            ON rs.radius= CONVERT(real,h.connect_radius) 
            AND rs.status = 1 
            AND rs.type = 'Non-Govt.'
        LEFT JOIN tbl_bill_setting AS bs
            ON bs.Status = 1
        LEFT JOIN 
        (   SELECT  hlid,
                    SUM(netbill) AS Due
            FROM    tbl_bill AS b
            WHERE   date_month < @tdate  
            AND     (b.ispay = 0 OR b.ispay IS NULL) 
            GROUP BY hlid
        ) AS b
            ON b.hlid = h.id
WHERE   NOT EXISTS 
        (   SELECT  1 
            FROM    tbl_bill AS tb 
            WHERE   EOMONTH(@tdate) = EOMONTH(date_month)       
            AND     tb.holdingNo = h.holdinNo
            AND     (tb.update_by IS NOT NULL OR tb.ispay=1)
        );

Please take this with a pinch of salt, it was quite hard work trying to piece together the logic, so it may need some minor tweaks and corrections

As well as adapting this to work as a single statement, I have made a number of modifications to your existing code:

• Swapped NOT IN for NOT EXISTS to avoid any issues with null records. If holdingNo is nullable, they are equivalent, if holdingNo is nullable, NOT EXISTS is safer - Not Exists Vs Not IN
• The join syntax you are using was replaced 27 years ago, so I switched from ANSI-89 join syntax to ANSI-92. - Bad habits to kick : using old-style JOINs
• Changed predicates of YEAR(date_month) = YEAR(@tDate) AND MONTH(date_month) = MONTH(@tDate) to become EOMONTH(@tdate) = EOMONTH(date_month). These are syntactically the same, but EOMONTH is Sargable, whereas MONTH and YEAR are not.

Then a few further links/suggestions that are directly related to changes I have made

• Although I removed the while lopp, don't fall into the trap of thinking this is better than a cursor. A properly declared cursor will out perform a while loop like yours - Bad Habits to Kick : Thinking a WHILE loop isn't a CURSOR
• The general consensus is that prefixing object names is not a good idea. It should either be obvious from the context if an object is a table/view or function/procedure, or it should be irrelevant - i.e. There is no need to distinguish between a table or a view, and in fact, we may wish to change from one to the other, so having the prefix makes things worse, not better.
• The average ratio of time spent reading code to time spent writing code is around 10:1 - It is therefore worth the effort to format your code when you are writing it so that it is easy to read. This is hugely subjective with SQL, and I would not recommend any particular conventions, but I cannot believe for a second you find your original code free flowing and easy to read. It took me about 10 minutes just unravel the first insert statement.

---

EDIT

The above is not correct, EOMONTH() is not sargable, so does not perform any better than YEAR(x) = YEAR(y) AND MONTH(x) = MONTH(y), although it is still a bit simpler. If you want a truly sargable predicate you will need to create a start and end date using @tdate, so you can use:

DATEADD(MONTH, DATEDIFF(MONTH, '19000101', @tdate), '19000101') 

to get the first day of the month for @tdate, then almost the same forumla, but add months to 1st February 1900 rather than 1st January to get the start of the next month:

DATEADD(MONTH, DATEDIFF(MONTH, '19000201', @tdate), '19000201') 

So the following:

DECLARE @Tdate DATE = '2019-10-11';
SELECT  DATEADD(MONTH, DATEDIFF(MONTH, '19000101', @tdate), '19000101'),
        DATEADD(MONTH, DATEDIFF(MONTH, '19000201', @tdate), '19000201');

Will return 1st October and 1st November respectively. Putting this back in your original query would give:

WHERE   NOT EXISTS 
        (   SELECT  1 
            FROM    tbl_bill AS tb 
            WHERE   date_month >= DATEADD(MONTH, DATEDIFF(MONTH, '19000101', @tdate), '19000101'),
            AND     date_month < DATEADD(MONTH, DATEDIFF(MONTH, '19000201', @tdate), '19000201')
            AND     tb.holdingNo = h.holdinNo
            AND     (tb.update_by IS NOT NULL OR tb.ispay=1)
        );