-1
\$\begingroup\$

My QA told me that I should not use IF SELECT before doing an UPDATE statement because it will breaks the atomicity of a transaction. I'm doing some research about this but cant find any article saying IF SELECT is not a good practice for TRANSACTION.

--setup example tables
DECLARE @tblCustomer TABLE(
CustomerId int identity(1,1),
FullName varchar(255),
CustomerStatus varchar(100)
)

DECLARE @tblRestaurant TABLE(
RestaurantId int identity(1,1),
RestaurantName varchar(255),
RestaurantStatus varchar(100)
)

INSERT INTO @tblCustomer (FullName) values ('Tom Hanks')
INSERT INTO @tblCustomer (FullName) values ('Maria')
INSERT INTO @tblCustomer (FullName) values ('Darwin Slack')

INSERT INTO @tblRestaurant (RestaurantName, RestaurantStatus) values ('MC Donalds', 'Closed')
INSERT INTO @tblRestaurant (RestaurantName, RestaurantStatus) values ('Burger King', 'Closed')
INSERT INTO @tblRestaurant (RestaurantName, RestaurantStatus) values ('KFC', 'Closed')

--update transaction begin
BEGIN TRANSACTION;
BEGIN TRY

--Edit: I add insert statement here to add complexity in query
INSERT INTO @tblCustomer (FullName) values ('Dora Explorer')

IF EXISTS(SELECT 1 FROM @tblRestaurant WHERE RestaurantStatus = 'Open')
BEGIN
    UPDATE @tblCustomer SET
    CustomerStatus = 'Full'
END

SELECT * FROM @tblCustomer
COMMIT TRANSACTION;
RETURN;
END TRY
BEGIN CATCH
    IF @@TRANCOUNT > 0  
ROLLBACK TRANSACTION;
END CATCH;

\$\endgroup\$
5
  • 1
    \$\begingroup\$ By if select are you talking about the if exists()? \$\endgroup\$
    – Reinderien
    Jul 6 '21 at 13:22
  • \$\begingroup\$ yes, it's pointed to if exists () / if is null () / if != (select) \$\endgroup\$
    – vantian
    Jul 6 '21 at 13:23
  • \$\begingroup\$ What version of SQL server do you use? \$\endgroup\$
    – Reinderien
    Jul 6 '21 at 14:56
  • \$\begingroup\$ I'm using Azure SQL \$\endgroup\$
    – vantian
    Jul 8 '21 at 0:14
  • 3
    \$\begingroup\$ We need to know what the code is intended to achieve. To help reviewers give you better answers, please add sufficient context to your question, including a title that summarises the purpose of the code. We want to know why much more than how. The more you tell us about what your code is for, the easier it will be for reviewers to help you. The title needs an edit to simply state the task, rather than your concerns about the code. \$\endgroup\$ Jul 9 '21 at 13:28
2
\$\begingroup\$

I should not use IF SELECT before doing an UPDATE statement because it will breaks the atomicity of a transaction

This is wrong. The if exists / update is itself a non-atomic operation, but since it exists within the context of a transaction, ACID will be preserved. If there were no transaction, no isolation level would be applied and integrity would not be guaranteed.

Other points:

  • your code doesn't run. So far as I can tell that's because you can't use a dynamic, table-typed variable without also constructing dynamic SQL in a string to reference it. I've worked around this by creating normal tables instead of table-typed variables.
  • (1, 1) is the default for identity and can be made implicit.
  • Hungarian notation is these days considered awkward, non-legible and fairly out-of-fashion. Rather than prefixing tbl, just pluralize your table names.
  • Prefixing the name of the table before every one of its columns - e.g. CustomerStatus - is redundant.
  • Transact SQL being broken and not adhering to the SQL standard, it supports neither enumerations nor boolean columns, so for your status use either a constrained string or a bit.
  • Combine your values() terms.
  • Indent your blocks.
  • Transact SQL again being broken, it does not consistently respect semicolons as it should. In almost all cases you should omit them.

Suggested

create table Customers(
    -- The default for identity is (1, 1) per
    -- https://docs.microsoft.com/en-us/sql/t-sql/statements/create-table-transact-sql?view=sql-server-ver17#arguments
    Id int identity primary key,

    -- Unless you're on SQL Server 2019 or later with UTF-8
    -- collation, you should be using nvarchar instead
    FullName varchar(255) not null,

    -- Status varchar(100) not null
    -- check (Status in ('Hungry', 'Full'))
    IsHungry bit not null default 1
)
  
create table Restaurants(
    Id int identity primary key,
    Name varchar(255) not null,

    -- Status varchar(100) not null
    -- check (status in ('Open', 'Closed'))
    IsOpen bit not null
)
  
insert into Customers(FullName) values
    ('Tom Hanks'),
    ('Maria'),
    ('Darwin Slack')
    
insert into Restaurants(Name, IsOpen) values
    ('McDonalds', 0),
    ('Burger King', 1),
    ('KFC', 0)

begin transaction
begin try
    insert into Customers(FullName) values
    ('Dora Explorer')

    if exists(
        select 1 from Restaurants where IsOpen=1
    )
    begin
        update Customers set IsHungry=0
    end

    select * from Customers
    commit transaction
    return
end try
begin catch
    rollback transaction
end catch

Fiddle

\$\endgroup\$
2
  • \$\begingroup\$ I found MERGE statement while looking alternatively replacing IF EXISTS, do you think it's a best practice? stackoverflow.com/questions/2273815/… \$\endgroup\$
    – vantian
    Jul 8 '21 at 0:16
  • \$\begingroup\$ and I loved when he said Developers-turned-DBAs often naïvely write it row-by-row because it's the truth..lol \$\endgroup\$
    – vantian
    Jul 8 '21 at 0:18

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