Rewriting the Sql code

Question

Rewriting the Sql code

VSSGeorge

SSCrazy Eights

Points: 8151
More actions
June 3, 2015 at 8:41 am

#318742

Hi
Please help me to rewrite the code in a meaning way.
It is now in a zig-zag way. Any help is appreciated.
select A.activitySqlId, A.activityIncId,
S.studyName as 'Study Name',
isnull(A.activityScheduledEndDate, A.activityLatestEndDate) as 'Scheduled End Date'
from Activities as A
inner join TypesOfActivities as ToA
on (A.typeOfActivityIncId = ToA.typeOfActivityIncId AND A.typeOfActivitySqlId = ToA.typeOfActivitySqlId AND ToA.typeOfActivityCode like 'EAS%')
Inner join Studies as S WITH(NOLOCK)
on A.studySqlId = S.studySqlId
and A.studyIncId = S.studyIncId
and S.isDeleted = 0x0
INNER Join Operators as O WITH(NOLOCK)
ON O.OperatorCode = 'EPGR'
INNER JOIN StudiesPositions AS StPo WITH(NOLOCK)
LEFT JOIN Positions AS Po ON Po.positionSqlId = StPo.positionSqlId AND Po.positionIncId = StPo.positionIncId AND Po.isDeleted = 0x0
AND Po.positionCode = 'ESM002'
ON StPo.studySqlId = S.studySqlId AND StPo.studyIncId = S.studyIncId AND StPo.isDeleted = 0x0
AND O.operatorSqlId = StPo.operatorSqlId AND O.operatorIncId = StPo.OperatorIncId AND O.isDeleted = 0x0
Where A.isDeleted=0x0
AND ((A.activityStatusSqlId = 134 and A.activityStatusIncId = 2) or (A.activityStatusSqlId = 134 and A.activityStatusIncId = 4)) --TD or IP
AND isnull(A.activityScheduledEndDate, A.activityLatestEndDate) <= DATEADD(Day, -7, GETDATE())
AND (Select count(A2.activityCode)
from activities as A2
where A2.isDeleted=0x0 AND A2.fatherActivityIncId=A.activityIncId
AND A2.fatherActivitySqlId=A.activitySqlId)=0
order by isnull(A.activityStartDate, isnull(A.activityScheduledStartDate, A.activityEarliestStartDate)), S.studyName, A.activityCode, A.activityName

Viewing 12 posts - 1 through 11 (of 11 total)

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

Gail Shaw SSC Guru Points: 1004504 More actions · Answer 1

What, other than the NOLOCK hints, is the problem?

Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability

We walk in the dark places no others will enter
We stand on the bridge and no one may pass

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

It is now in a zig-zag way.

What do you mean by this? Formatting, or logic, or something else?

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

dbodell Right there with Babe Points: 742 More actions · Answer 3

I also don't understand what you mean by "Zig-zag" or if there is an actual problem you need to solve. But I do have a couple of comments. First, unless you are sure you need to use WITH (NOLOCK), don't use it. Second, why are you comparing a column to 0x0 in your WHERE clause? Are you comparing the value to 0? If not, then this would have to be a string value and it should be in quotes. Maybe I am missing something.

Otherwise, unless you are having a specific problem, nothing else jumps out at me.

One more thing: why are you using ORDER BY? Is this data feeding something else and it has to be sorted beforehand? Otherwise, can't your presentation tool sort it (Excel, SSRS, Crystal, etc.)?

ChrisM@home SSC-Insane Points: 24260 More actions · Answer 4

GilaMonster (6/3/2015)
What, other than the NOLOCK hints, is the problem?

The OP is confused by the location of the ON clauses. Since there are only INNER joins, I don't think it matters.

[font="Arial"]^{Low-hanging fruit picker and defender of the moggies}[/font]

For better assistance in answering your questions, please read this[/url].

Understanding and using APPLY, (I)[/url] and (II)[/url] Paul White[/url]

Hidden RBAR: Triangular Joins[/url] / The "Numbers" or "Tally" Table: What it is and how it replaces a loop[/url] Jeff Moden[/url]

Alvin Ramard SSC-Forever Points: 41190 More actions · Answer 5

ChrisM@home (6/3/2015)
GilaMonster (6/3/2015)
What, other than the NOLOCK hints, is the problem?
The OP is confused by the location of the ON clauses. Since there are only INNER joins, I don't think it matters.

oops, I missed where the OP said he was confused about the ON clauses.

Alvin Ramard
Memphis PASS Chapter[/url]

All my SSC forum answers come with a money back guarantee. If you didn't like the answer then I'll gladly refund what you paid for it.

For best practices on asking questions, please read the following article: Forum Etiquette: How to post data/code on a forum to get the best help[/url]

Jeff Moden SSC Guru Points: 1004365 More actions · Answer 6

Alvin Ramard (6/3/2015)
ChrisM@home (6/3/2015)
GilaMonster (6/3/2015)
What, other than the NOLOCK hints, is the problem?
The OP is confused by the location of the ON clauses. Since there are only INNER joins, I don't think it matters.
oops, I missed where the OP said he was confused about the ON clauses.

I also see at least one outer join, not to mention the correlated subquery in the WHERE clause.

--Jeff Moden

RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

Change is inevitable... Change for the better is not.

Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)

Jeff Moden SSC Guru Points: 1004365 More actions · Answer 7

Junglee_George (6/3/2015)
Hi
Please help me to rewrite the code in a meaning way.
It is now in a zig-zag way. Any help is appreciated.
select A.activitySqlId, A.activityIncId,
S.studyName as 'Study Name',
isnull(A.activityScheduledEndDate, A.activityLatestEndDate) as 'Scheduled End Date'
from Activities as A
inner join TypesOfActivities as ToA
on (A.typeOfActivityIncId = ToA.typeOfActivityIncId AND A.typeOfActivitySqlId = ToA.typeOfActivitySqlId AND ToA.typeOfActivityCode like 'EAS%')
Inner join Studies as S WITH(NOLOCK)
on A.studySqlId = S.studySqlId
and A.studyIncId = S.studyIncId
and S.isDeleted = 0x0
INNER Join Operators as O WITH(NOLOCK)
ON O.OperatorCode = 'EPGR'
INNER JOIN StudiesPositions AS StPo WITH(NOLOCK)
LEFT JOIN Positions AS Po ON Po.positionSqlId = StPo.positionSqlId AND Po.positionIncId = StPo.positionIncId AND Po.isDeleted = 0x0
AND Po.positionCode = 'ESM002'
ON StPo.studySqlId = S.studySqlId AND StPo.studyIncId = S.studyIncId AND StPo.isDeleted = 0x0
AND O.operatorSqlId = StPo.operatorSqlId AND O.operatorIncId = StPo.OperatorIncId AND O.isDeleted = 0x0
Where A.isDeleted=0x0
AND ((A.activityStatusSqlId = 134 and A.activityStatusIncId = 2) or (A.activityStatusSqlId = 134 and A.activityStatusIncId = 4)) --TD or IP
AND isnull(A.activityScheduledEndDate, A.activityLatestEndDate) <= DATEADD(Day, -7, GETDATE())
AND (Select count(A2.activityCode)
from activities as A2
where A2.isDeleted=0x0 AND A2.fatherActivityIncId=A.activityIncId
AND A2.fatherActivitySqlId=A.activitySqlId)=0
order by isnull(A.activityStartDate, isnull(A.activityScheduledStartDate, A.activityEarliestStartDate)), S.studyName, A.activityCode, A.activityName

Going quite literally by what you said, copy and paste the code above into the website at PoorSQL.com and it will almost instantly reformat it for you to make it more readable. It won't be perfect (look for ON and WHERE that didn't get wrapped properly) but it does do most of the work for you.

And, yeah... the double ON for the Left Join does actually work.

--Jeff Moden

RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
First step towards the paradigm shift of writing Set Based code:
________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

Change is inevitable... Change for the better is not.

Helpful Links:
How to post code problems
How to Post Performance Problems
Create a Tally Function (fnTally)

VSSGeorge SSCrazy Eights Points: 8151 More actions · Answer 8

Thank you all for the replies.

Actually my need is to rearrange the JOINs and ON conditions in a meaningful readable way.

There are so many JOINs written in a very difficult way to understand.

Phil Parkin SSC Guru Points: 246965 More actions · Answer 9

Junglee_George (6/4/2015)
Thank you all for the replies.
Actually my need is to rearrange the JOINs and ON conditions in a meaningful readable way.
There are so many JOINs written in a very difficult way to understand.

That's a subjective opinion. It's really not that bad, compared to some of the queries I've seen.

The NOLOCKs need to be removed if possible, you need to check that isDeleted really is varbinary (sounds like it should be bit to me).

The hard-coded Ids are difficult to interpret & would be better (IMO) as literals from the underlying lookup table.

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

ChrisM@Work SSC Guru Points: 186127 More actions · Answer 10

Jeff Moden (6/3/2015)
Alvin Ramard (6/3/2015)
ChrisM@home (6/3/2015)
GilaMonster (6/3/2015)
What, other than the NOLOCK hints, is the problem?
The OP is confused by the location of the ON clauses. Since there are only INNER joins, I don't think it matters.
oops, I missed where the OP said he was confused about the ON clauses.
I also see at least one outer join, not to mention the correlated subquery in the WHERE clause.

The left join isn't referenced by the wayward ON clauses, I don't think it will be influenced by the proposed changes below.

-- change the FROM list from this

from Activities as A

inner join TypesOfActivities as ToA

on A.typeOfActivityIncId = ToA.typeOfActivityIncId

AND A.typeOfActivitySqlId = ToA.typeOfActivitySqlId

AND ToA.typeOfActivityCode like 'EAS%'

Inner join Studies as S WITH(NOLOCK)

on A.studySqlId = S.studySqlId

and A.studyIncId = S.studyIncId

and S.isDeleted = 0x0

INNER Join Operators as O WITH(NOLOCK)

ON O.OperatorCode = 'EPGR'

INNER JOIN StudiesPositions AS StPo WITH(NOLOCK)

LEFT JOIN Positions AS Po

ON Po.positionSqlId = StPo.positionSqlId

AND Po.positionIncId = StPo.positionIncId

AND Po.isDeleted = 0x0

AND Po.positionCode = 'ESM002'

ON StPo.studySqlId = S.studySqlId

AND StPo.studyIncId = S.studyIncId

AND StPo.isDeleted = 0x0

AND O.operatorSqlId = StPo.operatorSqlId

AND O.operatorIncId = StPo.OperatorIncId

AND O.isDeleted = 0x0

--------------------------------------------------------------

-- to this

FROM Activities as A

INNER JOIN TypesOfActivities as ToA

on A.typeOfActivityIncId = ToA.typeOfActivityIncId

AND A.typeOfActivitySqlId = ToA.typeOfActivitySqlId

AND ToA.typeOfActivityCode like 'EAS%'

INNER JOIN Studies as S --WITH(NOLOCK)

on A.studySqlId = S.studySqlId

and A.studyIncId = S.studyIncId

and S.isDeleted = 0x0

INNER JOIN StudiesPositions AS StPo --WITH(NOLOCK)

ON StPo.studySqlId = S.studySqlId

AND StPo.studyIncId = S.studyIncId

AND StPo.isDeleted = 0x0

INNER JOIN Operators as O --WITH(NOLOCK)

ON O.operatorSqlId = StPo.operatorSqlId

AND O.operatorIncId = StPo.OperatorIncId

AND O.OperatorCode = 'EPGR'

AND O.isDeleted = 0x0

LEFT JOIN Positions AS Po

ON Po.positionSqlId = StPo.positionSqlId

AND Po.positionIncId = StPo.positionIncId

AND Po.positionCode = 'ESM002'

AND Po.isDeleted = 0x0

--------------------------------------------------------------

^{“Write the query the simplest way. If through testing it becomes clear that the performance is inadequate, consider alternative query forms.” - Gail Shaw}

For fast, accurate and documented assistance in answering your questions, please read this article.
Understanding and using APPLY, (I) and (II) Paul White
Hidden RBAR: Triangular Joins / The "Numbers" or "Tally" Table: What it is and how it replaces a loop Jeff Moden

sgmunson SSC Guru Points: 110640 More actions · Answer 11

Junglee_George (6/4/2015)
Thank you all for the replies.
Actually my need is to rearrange the JOINs and ON conditions in a meaningful readable way.
There are so many JOINs written in a very difficult way to understand.

I'll echo the desire to remove the NOLOCK hints, but for the sake of time, here's a quick re-write that at least formats the query in a much more readable way:

SELECT A.activitySqlId, A.activityIncId, S.studyName AS 'Study Name',

ISNULL(A.activityScheduledEndDate, A.activityLatestEndDate) AS 'Scheduled End Date'

FROM Activities AS A

INNER JOIN TypesOfActivities AS ToA

ON A.typeOfActivityIncId = ToA.typeOfActivityIncId

AND A.typeOfActivitySqlId = ToA.typeOfActivitySqlId

AND ToA.typeOfActivityCode like 'EAS%'

INNER JOIN Studies AS S WITH(NOLOCK)

ON A.studySqlId = S.studySqlId

AND A.studyIncId = S.studyIncId

AND S.isDeleted = 0x0

INNER JOIN Operators AS O WITH(NOLOCK)

ON O.OperatorCode = 'EPGR'

INNER JOIN

StudiesPositions AS StPo WITH(NOLOCK)

LEFT JOIN Positions AS Po

ON Po.positionSqlId = StPo.positionSqlId

AND Po.positionIncId = StPo.positionIncId

AND Po.isDeleted = 0x0

AND Po.positionCode = 'ESM002'

ON StPo.studySqlId = S.studySqlId

AND StPo.studyIncId = S.studyIncId

AND StPo.isDeleted = 0x0

AND O.operatorSqlId = StPo.operatorSqlId

AND O.operatorIncId = StPo.OperatorIncId

AND O.isDeleted = 0x0

WHERE A.isDeleted = 0x0

AND A.activityStatusSqlId = 134

AND A.activityStatusIncId IN (2, 4) --TD or IP

AND ISNULL(A.activityScheduledEndDate, A.activityLatestEndDate) <= DATEADD(Day, -7, GETDATE())

AND NOT EXISTS

(

SELECT 1--COUNT(A2.activityCode)

FROM Activities AS A2

WHERE A2.isDeleted = 0x0

AND A2.fatherActivityIncId = A.activityIncId

AND A2.fatherActivitySqlId = A.activitySqlId

)

ORDER BY COALESCE(A.activityStartDate, A.activityScheduledStartDate, A.activityEarliestStartDate), S.studyName, A.activityCode, A.activityName

I changed the last part of the WHERE clause to NOT exists and eliminated the COUNT, as in theory, now it only needs to detect that 1 disqualifying record exists instead of having to count all such records. I don't know if the optimizer is good enough to figure that out, but why give it the chance? In any case, let me know if this helps.

Steve (aka sgmunson) 🙂 🙂 🙂
Rent Servers for Income (picks and shovels strategy)