-1

How can I get better performance with my sql query in a SP? It has a lot of memory usage.if you look at below my execution pan you will see that :

IF NOT EXISTS(SELECT * FROM Common.[CustomerxxxIds] WHERE xyzType = @xyzType AND CustomerId = @CustomerId)[/code]

has alot of memory usage. How can I reduce that?

ALTER PROCEDURE [Common].[SaveCustomerxxxIds] 
(
    @xyzType    NVARCHAR(128),
    @CustomerId INT,
    @xxxId  INT OUTPUT
)
AS
BEGIN
    SET NOCOUNT ON;

    IF NOT EXISTS(SELECT * FROM Common.[CustomerxxxIds] WHERE xxxType = @xxxType AND CustomerId = @CustomerId)
    BEGIN
        INSERT INTO Common.[CustomerxxxIds]
                    ([xxxId]
                    ,[CustomerId]
                    ,[xxxType])
                VALUES
                    (0
                    ,@CustomerId
                    ,@xxxType)
    END

    UPDATE  Common.[CustomerxxxIds]
    SET     [xxxId] = ([xxxId]) + 1
    WHERE   [xxxType] = @xxxType
            AND CustomerId = @CustomerId

    SELECT  @xxxId = xxxId
    FROM    Common.[CustomerxxxIds]
    WHERE   [xxxType] = @xxxType
            AND CustomerId = @CustomerId
END
Penguen
  • 16,836
  • 42
  • 130
  • 205
  • 1
    What do you mean it "has a lot of memory usage"? – DavidG Jun 13 '19 at 12:30
  • 1
    I get an unpleasant feeling seeing "clustered" in the execution plan, especially in combination with "insert"... – Bart Hofland Jun 13 '19 at 12:40
  • 2
    There are no memory consuming operators in that execution plan. You need an index on `EntityType,CustomerId` - probably this can be simplified too – Martin Smith Jun 13 '19 at 12:52
  • 2
    @BartHofland - you will get that unpleasant feeling a lot then. Every time you insert to the table with a clustered index you will see this. – Martin Smith Jun 13 '19 at 12:55
  • @MartinSmith OK. Thanks for the remark. I have to be honest: I have not checked an SQL Server query execution plan for more than 15 years. Sorry about that. I just got alarmed, because if the table contains an editable (non-identity) clustered key, it could result in heavy disc traffic due to physically rearranging records so that they are physically sorted correctly again. Since the OP didn't specify the index definitions, I assumed it could be a potential performance issue here... – Bart Hofland Jun 13 '19 at 13:11
  • i would suggest you also sharing the `CREATE TABLE` statements – Raymond Nijland Jun 13 '19 at 13:12
  • Ideally for these queries in the SP you want to be having a multicolumn index on `(CustomerId, EntityType)` or `(EntityType, CustomerId)` depending on the data which gives you the best index selectivity – Raymond Nijland Jun 13 '19 at 13:19
  • 2
    @BartHofland - existing records aren't physically re-arranged at insert time to get them into perfectly unfragmented clustered index key order. If there is room on the page where the record would need to go it is written there and only the slot array potentially updated to point to rows on the page in key order. If there is no room on the page a new page is allocated from anywhere in the file as a page split and linked into the linked list. This is exactly the same process as used for a non clustered index. [more details](https://stackoverflow.com/a/24470091/73226) – Martin Smith Jun 13 '19 at 13:22
  • Can you explain what you're trying to accomplish with this statement? I feel like something is missing. Why are you inserting a record, then updating the same record? – Wes H Jun 13 '19 at 13:46
  • From my understanding of the code you've posted. The `EntityId` column represents the number of entities of each type that each customer has. This code has a potential to break in a multi-user environment and either cause errors or worst - wrong data. – Zohar Peled Jun 13 '19 at 13:46

3 Answers3

2

You can do things to avoid "re-read" of the table to get the output value.

After the INSERT ( INSERT INTO Common.[CustomerxxxIds])

Use SCOPE_IDxxx() to get the newly created surrogate key.

The above will only work for IDxxx columns. From your question, you may not actually have an IDxxx column.

See

https://learn.microsoft.com/en-us/sql/t-sql/functions/scope-idxxx-transact-sql?view=sql-server-2017

.........

with the UPDATE and/or INSERT, you could use OUTPUT functionality to get the value.

https://learn.microsoft.com/en-us/sql/t-sql/queries/output-clause-transact-sql?view=sql-server-2017

This AVOIDS the last select statement (the "re-read" as I am calling it) to get the desired output value.

Obviously completely removing a SELECT statement will improve performance.

..

Below is a simple but complete Northwind database example of using OUTPUT for INSERT and UPDATE

SELECT 'Before' as Looksie, [ShipperID]
      ,[CompanyName]
      ,[Phone]
  FROM [Northwind].[dbo].[Shippers]

  --
  DECLARE @MyInsertAuditTable table( AuditShipperID INT,  
                           AuditCompanyName nvarchar(40),  
                           AuditPhone nvarchar(24));  
INSERT [Northwind].[dbo].[Shippers] (CompanyName , Phone )
    OUTPUT INSERTED.ShipperID, INSERTED.CompanyName, INSERTED.Phone  
        INTO @MyInsertAuditTable  (AuditShipperID, AuditCompanyName , AuditPhone )

SELECT TOP 1
    --(SELECT MAX(ShipperID) + 1 from dbo.Shippers )
     'Shipper' + LEFT(CONVERT(VARCHAR(38), NEWID()), 12)
    , '(555) 555-5555'
    FROM sys.objects

--Display the result set of the table variable.  
SELECT AuditShipperID, AuditCompanyName, AuditPhone FROM @MyInsertAuditTable;  

  DECLARE @MyUpdateAuditTable table( AuditShipperID INT,  
                           AuditCompanyName nvarchar(40),  
                             AuditOldPhone nvarchar(24),
                           AuditNewPhone nvarchar(24));  

UPDATE [Northwind].[dbo].[Shippers] 

SET Phone = '(777) 555-7777'

OUTPUT inserted.ShipperID,  inserted.CompanyName ,
       deleted.Phone,  
       inserted.Phone
INTO @MyUpdateAuditTable ( AuditShipperID, AuditCompanyName, AuditOldPhone , AuditNewPhone)
FROM [Northwind].[dbo].[Shippers]  shippers
JOIN @MyInsertAuditTable insAudit on shippers.ShipperID = insAudit.AuditShipperID

SELECT * from @MyUpdateAuditTable



SELECT 'After' as Looksie, [ShipperID]
      ,[CompanyName]
      ,[Phone]
  FROM [Northwind].[dbo].[Shippers]

  --

Results

Looksie ShipperID   CompanyName Phone
Before  1   Speedy Express  (503) 555-9831
Before  2   United Package  (503) 555-3199
Before  3   Federal Shipping    (503) 555-9931

..

AuditShipperID  AuditCompanyName    AuditPhone
9               Shipper3C062D46-EEA (555) 555-5555

...

AuditShipperID  AuditCompanyName    AuditOldPhone   AuditNewPhone
9               Shipper3C062D46-EEA (555) 555-5555  (777) 555-7777

..

Looksie ShipperID   CompanyName Phone
After   1           Speedy Express  (503) 555-9831
After   2           United Package  (503) 555-3199
After   3           Federal Shipping    (503) 555-9931
After   9           Shipper3C062D46-EEA (777) 555-7777
Penguen
  • 16,836
  • 42
  • 130
  • 205
granadaCoder
  • 26,328
  • 10
  • 113
  • 146
  • 1. You mean `scope_identity()`. It's a function, not a global variable. (what you wrote is a mixup between `scope_identity()` and `@@identity` which is a totally different thing). 2. `EntityId` is **clearly not** and identity column - as the code updates it. identity columns can't be updated. – Zohar Peled Jun 13 '19 at 13:38
  • I fixed the scope identity syntax. The original question looks a little mucked up, thus why I gave the pointers to the features to avoid the reread – granadaCoder Jun 13 '19 at 14:04
  • Even with he syntax fix, this answer is still wrong - since `EntityId` is not an identity column. – Zohar Peled Jun 13 '19 at 14:05
  • If EntityID is not an IDENTITY, then one can use OUTPUT for both cases. – granadaCoder Jun 13 '19 at 14:33
0

You can achieve this by changing SELECT * to SELECT 1. it might help

IF NOT EXISTS (SELECT 1 FROM Common.[CustomerxxxIds] 
               WHERE xyzType = @xyzType AND CustomerId = @CustomerId)
Penguen
  • 16,836
  • 42
  • 130
  • 205
Farhan
  • 47
  • 7
  • 2
    If it has effect, it is probably just marginal. https://dba.stackexchange.com/questions/159413/exists-select-1-vs-exists-select-one-or-the-other – Bart Hofland Jun 13 '19 at 12:33
  • This is wrong. Whatever is in the select clause for a query that is used in an `exist` condition is ignored by SQL Server. You can even do `if not exists(select 1/0 from....)` and get results, where as when you do `select 1/0 from...` you get a division by zero error. – Zohar Peled Jun 13 '19 at 13:36
-1

Try this

    ALTER PROCEDURE [Common].[SaveCustomerxxxIds] 
(
    @xyz NVARCHAR(128),
    @CustomerId INT,
    @xxxId   INT OUTPUT
)
AS
BEGIN
    set @xxxId=null
    --Get xxxId
    SELECT @xxxId=[xxxId] FROM Common.[CustomerxxxIds] WHERE xyz = @xyz AND CustomerId = @CustomerId
    --If @xxxId means no record we should insert 
    if (@xxxId is null)
        begin 
            --When insert we always insert xxxId as 0 then update to one then we collect the  value (one) from db and return it.
            --Better set value directly as one insert it to DB and return it as one. Instead of insert, update, select
            --just insert
            set @xxxId = 1
           INSERT INTO Common.[CustomerxxxIds]
                            ([xxxId]
                            ,[CustomerId]
                            ,[xyz])
                        VALUES
                            (@xxxId
                            ,@CustomerId
                            ,@xyz)
        end 
    else
        begin
            --If we have the value we add one to it update the record and return it.
            --better than update table then select. 
            --We already have the value we add one to it then update table and return the value we have 
            set @xxxId+=1
            UPDATE  Common.[CustomerxxxIds]   SET     [xxxId] = @xxxId
            WHERE   [xyz] = @xyz       AND CustomerId = @CustomerId
        END
end
Penguen
  • 16,836
  • 42
  • 130
  • 205
jamil tero
  • 77
  • 1
  • 9
  • Please don't write large blocks of code with no description of what it does. In what way is this code better/faster/more performant? What have you changed? Why did you change it? – DavidG Jun 13 '19 at 12:52
  • Since you need entityid anyway so we collect from start to reduce number of operations. if there is no record it will be null we insert with entityid =1 becuase in your code you insert as 0 then add one. Here we insert directly one and save update operation. Now if there is a record I add one then update and save the last select which is not necessary – jamil tero Jun 13 '19 at 12:59
  • This way instead of 4 operations examin then insert then update then select we use only two examine then insert. In case of update it will be examine then update – jamil tero Jun 13 '19 at 13:01
  • 1
    This doesn't quite have the same semantics as the original query. The original query will instantly update the `EntityId` that was just inserted - so now it needs to be inserted as `1`. Also the `UPDATE` branch is wrong. It needs to increment the `EntityId` in the row by `1` - there is no reason to think this is the same in all rows – Martin Smith Jun 13 '19 at 13:01
  • @ Martin Smith the original insert [EntityId] as zero the update it to one. There is no idintity here – jamil tero Jun 13 '19 at 13:03
  • 1
    You don't insert it as `1` - you have a hardcoded constant of `0` in your `VALUES`. So your rewrite does not behave the same. And I didn't mention anything about identity – Martin Smith Jun 13 '19 at 13:04
  • And don't add explanation in comments, [edit] the post to include it. – DavidG Jun 13 '19 at 13:08