Ugly code fix

Question

Ugly code fix

jharvey6 76850

Old Hand

Points: 381
More actions
November 3, 2020 at 7:49 pm

#3804012
```
CASE
WHEN Phone IS NOT NULL OR MobilPhone IS NOT NULL THEN
CONCAT(CONCAT(LEFT(COALESCE(Phone,MobilPhone),3),'-'),
CONCAT(CONCT(RIGHT(LEFT(COALESCE(Phone,MobilPhone),7),3),'-'),
RIGHT(LEFT(COALESCE(Phone,MobilPhone),12),4)))
ELSE ''
END As PhoneNumber
```
I inherited this god awful code. I comment this variable out and the query executes in < 30 seconds against a sizable number of records. (~ 1MM). I add this little tidbit of insanity back into the query time goes > 5 MINUTES!!! This made my head hurt 🙂 Any suggestions on how to more efficiently extract the formatted phone number from the two possible combinations of Phone vs MobilePhone.

Thanks In Advance for any suggestions.
- This topic was modified 4 years ago by jharvey6 76850.

Viewing 6 posts - 1 through 5 (of 5 total)

You must be logged in to reply to this topic. Login to reply

ScottPletcher SSC Guru Points: 101098 More actions · Answer 1

Maybe as below? I've got a feeling CONCAT may be a bit slow, just like some of the other newer functions. FORMAT is notoriously slow.

CASE
WHEN Phone IS NULL AND MobilPhone IS NULL THEN ''
ELSE LEFT(COALESCE(Phone,MobilPhone),3) + '-' +
     RIGHT(LEFT(COALESCE(Phone,MobilPhone),7),3) + '-' +
     RIGHT(LEFT(COALESCE(Phone,MobilPhone),12),4)
END As PhoneNumber

SQL DBA,SQL Server MVP(07, 08, 09) "It's a dog-eat-dog world, and I'm wearing Milk-Bone underwear." "Norm", on "Cheers". Also from "Cheers", from "Carla": "You need to know 3 things about Tortelli men: Tortelli men draw women like flies; Tortelli men treat women like flies; Tortelli men's brains are in their flies".

Phil Parkin SSC Guru Points: 246960 More actions · Answer 2

You could create a calculated, persisted column in your table definition and SELECT that instead.

The absence of evidence is not evidence of absence
- Martin Rees
The absence of consumable DDL, sample data and desired results is, however, evidence of the absence of my response
- Phil Parkin

Jonathan AC Roberts SSCoach Points: 18354 More actions · Answer 3

jharvey6 76850 wrote:

CASE
WHEN Phone IS NOT NULL OR MobilPhone IS NOT NULL THEN
CONCAT(CONCAT(LEFT(COALESCE(Phone,MobilPhone),3),'-'),
CONCAT(CONCT(RIGHT(LEFT(COALESCE(Phone,MobilPhone),7),3),'-'),
RIGHT(LEFT(COALESCE(Phone,MobilPhone),12),4)))
ELSE ''
END As PhoneNumber

That's very odd code. It looks like whoever wrote it didn't know you can join more than two values together with one concat statement

CONCAT('a',CONCAT('b',CONCAT('c',CONCAT('b'))))

is the same as:

CONCAT('a','b','c','b')

ScottPletcher SSC Guru Points: 101098 More actions · Answer 4

Jonathan AC Roberts wrote:

jharvey6 76850 wrote:
CASE
WHEN Phone IS NOT NULL OR MobilPhone IS NOT NULL THEN
CONCAT(CONCAT(LEFT(COALESCE(Phone,MobilPhone),3),'-'),
CONCAT(CONCT(RIGHT(LEFT(COALESCE(Phone,MobilPhone),7),3),'-'),
RIGHT(LEFT(COALESCE(Phone,MobilPhone),12),4)))
ELSE ''
END As PhoneNumber
That's very odd code. It looks like whoever wrote it didn't know you can join more than two values together with one concat statement
CONCAT('a',CONCAT('b',CONCAT('c',CONCAT('b'))))
is the same as:
CONCAT('a','b','c','b')

I think Oracle's CONCAT is like that, that it accepts only two values, maybe they had an Oracle background?!

SQL DBA,SQL Server MVP(07, 08, 09) "It's a dog-eat-dog world, and I'm wearing Milk-Bone underwear." "Norm", on "Cheers". Also from "Cheers", from "Carla": "You need to know 3 things about Tortelli men: Tortelli men draw women like flies; Tortelli men treat women like flies; Tortelli men's brains are in their flies".

richardm-1037631 SSC-Addicted Points: 481 More actions · Answer 5

The intent behind this code is to produce a formatted telephone number "###-###-####" or a empty string depending on the column values of Phone and MobilPhone. You can achieve the same effect by dropping the case statement entirely and using this formula instead: ISNULL(STUFF(STUFF(COALESCE(phone,mobilPhone,''),7,0,'-'),4,0,'-'),'') as PhoneNumber.

The COALESCE portion will return the first non-null value, the two STUFF statements will insert the 2 dashes at the correct positions, and the surrounding ISNULL will ensure that if either of the STUFF statements return a null (because the insert location exceeds the length of the string), an empty string will be returned.

You'll need to test whether this will give you the increase in speed that you're looking for, but I think it should help.