Awkward phrase

We have code from a manufacturer that we have not changed since the code was
purchased. In the code is some code that appears awkward. (See below) The
suggestion is to use a clause if exists (select blah blah -see below) then.
Given that the manufacturer wrote it this way (three to four years ago), is
there any reason why it may be a bad idea to change this proc... If so would
you use "If Exists..." or something else.
SELECT TOP 1
@.PickTicketID = AMU.PICK_TICKET_ID,
@.PTLine = AMU.LINE_NUMBER, @.SOI = TUID.SOI, @.INVKey = tUID.INV_Key,
@.ItemKey = tUID.ITEM_KEY
FROM
tIU WITH(NOLOCK)
INNER JOIN
tUID WITH(NOLOCK) ON tIU.UID_Key = tUID.UID_Key
INNER JOIN
ASSIGNED_MU AMU WITH(NOLOCK) ON AMU.MU_Key = tIU.MU_Key
WHERE
tIU.MU_Key = @.MUKey
IF ISNULL(@.PickTicketID,'') <> ''
--do stuff
--
Regards,
JamieReady for this one? Here's your answer, and you're sure to hate it: ;-)
"It depends."
It all depends on how often this procedure is created, and how the
underlying talbes are indexed. The question is this: What problem are
you trying to solve here? Performance? If so, it would be really
strange (and probably a mark of bad indexes) if this procedure performed
poorly. If it's cleanliness and data integrity issues, clearly NOLOCK
would have to go. But doing that could kill the concurrency of the
database -- a lot of developers use NOLOCK as a crutch to avoid dealing
with locks at the expense of potentially munging your data.
Other than that, the code wouldn't be that bad, if there were an ORDER
BY. The strange thing here is the "TOP 1". Do you expect there to be
more than one row? If so, which row should it be? You'd need an ORDER
BY there to tell SQL Server, or you'll get something random. You could
GROUP BY all of the columns, but if you *knew* that there would only be
one row coming off the query, the TOP 1 *could* get you better performance.
HTH
-Dave
thejamie wrote:
> We have code from a manufacturer that we have not changed since the code was
> purchased. In the code is some code that appears awkward. (See below) The
> suggestion is to use a clause if exists (select blah blah -see below) then.
> Given that the manufacturer wrote it this way (three to four years ago), is
> there any reason why it may be a bad idea to change this proc... If so would
> you use "If Exists..." or something else.
> SELECT TOP 1
> @.PickTicketID = AMU.PICK_TICKET_ID,
> @.PTLine = AMU.LINE_NUMBER, @.SOI = TUID.SOI, @.INVKey = tUID.INV_Key,
> @.ItemKey = tUID.ITEM_KEY
> FROM
> tIU WITH(NOLOCK)
> INNER JOIN
> tUID WITH(NOLOCK) ON tIU.UID_Key = tUID.UID_Key
> INNER JOIN
> ASSIGNED_MU AMU WITH(NOLOCK) ON AMU.MU_Key = tIU.MU_Key
> WHERE
> tIU.MU_Key = @.MUKey
> IF ISNULL(@.PickTicketID,'') <> ''
> --do stuff
>|||Top 1 is just to show the item exists. Thanks for the reply David. There is
a problem with this particular database - it was inherited and the nolock is
a crutch. It would be nice to know exactly where to put in different locking
hints and where to eliminate them completely, but my mere four months here
don't allow me that perspective just yet. In my mind, establishing any
pipelined process and fitting the locking hints to perform based on the
pipelines would be an improvement. Pipeline example might be 1- picking an
item in the warehouse, 2- moving it into production, 3- preparing the
material and 4- packing and shipping it. This is too wide a scope however;
maybe just picking is the single pipeline and maybe just packing and shipping
is another one.
--
Regards,
Jamie
"David Markle" wrote:
> Ready for this one? Here's your answer, and you're sure to hate it: ;-)
> "It depends."
> It all depends on how often this procedure is created, and how the
> underlying talbes are indexed. The question is this: What problem are
> you trying to solve here? Performance? If so, it would be really
> strange (and probably a mark of bad indexes) if this procedure performed
> poorly. If it's cleanliness and data integrity issues, clearly NOLOCK
> would have to go. But doing that could kill the concurrency of the
> database -- a lot of developers use NOLOCK as a crutch to avoid dealing
> with locks at the expense of potentially munging your data.
> Other than that, the code wouldn't be that bad, if there were an ORDER
> BY. The strange thing here is the "TOP 1". Do you expect there to be
> more than one row? If so, which row should it be? You'd need an ORDER
> BY there to tell SQL Server, or you'll get something random. You could
> GROUP BY all of the columns, but if you *knew* that there would only be
> one row coming off the query, the TOP 1 *could* get you better performance.
> HTH
> -Dave
> thejamie wrote:
> > We have code from a manufacturer that we have not changed since the code was
> > purchased. In the code is some code that appears awkward. (See below) The
> > suggestion is to use a clause if exists (select blah blah -see below) then.
> > Given that the manufacturer wrote it this way (three to four years ago), is
> > there any reason why it may be a bad idea to change this proc... If so would
> > you use "If Exists..." or something else.
> >
> > SELECT TOP 1
> > @.PickTicketID = AMU.PICK_TICKET_ID,
> > @.PTLine = AMU.LINE_NUMBER, @.SOI = TUID.SOI, @.INVKey = tUID.INV_Key,
> > @.ItemKey = tUID.ITEM_KEY
> > FROM
> > tIU WITH(NOLOCK)
> > INNER JOIN
> > tUID WITH(NOLOCK) ON tIU.UID_Key = tUID.UID_Key
> > INNER JOIN
> > ASSIGNED_MU AMU WITH(NOLOCK) ON AMU.MU_Key = tIU.MU_Key
> > WHERE
> > tIU.MU_Key = @.MUKey
> >
> > IF ISNULL(@.PickTicketID,'') <> ''
> > --do stuff
> >
>