Comment On SELECTing The Hard Way

Why do you need a found flag in the search results. Isn't it found if it's in the results?

My eyeses! It burns us!

Maybe he used to be an Oracle programmer? No SANE person would do that.

Anonymous:

Why do you need a found flag in the search results. Isn't it found if it's in the results?

Not if the results contain all of the rows in the table w/o applying any criteria at all, as this genius has done.

Hey!  That's my code!  My beautiful code!



Just kidding.

Now that type of thing just makes me sick!!!

Hey, at least he's using WHERE's!  Now granted theyre used in the most retarded way possible...BUT THEY'RE THERE!

That made my eyes hur. Question for the the people who do know this SQL
procedure dialect: doesn't this also have a giant race-condition 
if two calls to sp_SearchProjects are made simultaniously?

I can find three nice things to say about this procedure:

1. It is nicely formatted so that it is very easy to see what is being done.


2. In some hardware configurations it may well run faster than selecting all rows and filtering them on the client side.


3. It is encapsulated such that someone who actually understands how
   to write a WHERE clause can replace this stored procedure with a decent
   one without affecting any other code.

Of course, the whole procedure body could (and should) consist of a
single SELECT statement, the format of which is so obvious that it's
not even worth typing it in here.  I'd suggest that the programmer
here might be very competent in other types of coding, but shouldn't be
left to generate SQL with some guidance.

 

A programmer capable of this who claims any SQL expertise should be
hung by their toenails and forced to watch episodes of "The Nanny"
until they recant.  Otherwise, this seems like a management WTF to
have put this person on the task.

I think Alex's have run out of real WTFs and he is just making up crap. There is no way a human being can make code as heinous as this one. We have hit a new low, gentlemen.

/The goggles they do nothing

But, boss:  I wanted to make sure the code followed the user requirements as closely to their original written form as possible...

This isn't so bad.  At least it completes in O(N) time and memory usage.



A real WTF would have to use
at least two tables to be O(N*N) in either time or memory. 
Possibly in both if you really bugger it up.

Can you really declare a table with @Table syntax?

joodie:

doesn't this also have a giant race-condition  if two calls to sp_SearchProjects are made simultaniously?

The Torvaldian programmer has heroically avoided a race condition by using all local variables. 

Wow, this is the first time I literally had the words "WTF?" pop out of
my mouth while reading code.  Perhaps because I do T-SQL by
trade.  Good thing nobody overheard me; I deem this Not Safe For
Work!

joodie:

That made my eyes hur. Question for the the people who do know this SQL
procedure dialect: doesn't this also have a giant race-condition 
if two calls to sp_SearchProjects are made simultaniously?

This seems to be Microsoft SQL Server's T-SQL, and all of the rows are
selected into a "table variable" scoped for the stored procedure. 
There probably won't be any lock contention or anything since it
doesn't go back to the original table.  If it's a big table, there
might be a problem with all the reads it has to do, especially if the
table sees a lot of write activity as well...  But if it's a big
table, and the stored proecdure is called a lot, memory will probably
be a big problem too.

Maurits:

Can you really declare a table with @Table syntax?

If this is MSSQL then definitely:

DECLARE    @TT TABLE (

    Account varchar(40),

    BOOKDATE INT

)

Maurits:

Can you really declare a table with @Table syntax?

I don't think it's ANSI compliant, but SQL Server lets you use in-memory "table variables" local to a batch or stored procedure.

Maurits:

Can you really declare a table with @Table syntax?

If this is MSSQL then definitely:


DECLARE    @TT TABLE (


    Account varchar(40),


    BOOKDATE INT


)

Maurits:

Can you really declare a table with @Table syntax?

Yes.  You get an in-memory local-to-the-procedure table rather than a temporary table which actually resides the database.

joodie:

That made my eyes hur. Question for the the people who do know this SQL procedure dialect: doesn't this also have a giant race-condition  if two calls to sp_SearchProjects are made simultaniously?

No.  Because of the above, every call to the SP works with its own copy of the data.

Anonymous:

Of course, the whole procedure body could (and should) consist of a single SELECT statement, the format of which is so obvious that it's not even worth typing it in here.  I'd suggest that the programmer here might be very competent in other types of coding, but shouldn't be left to generate SQL with some guidance.

I suspect this is what happened--the code is just too structured to be a newbie programmer mistake.  It screams "I'm new to SQL" not "I have no clue how to code".  We all have our moments--I've just recently become more in tune with how to use creative joins to get the results I want rather than resorting to a cursor/loop structure.  There's some less-than-efficient stored procedures in our SQL server right now thanks to me, but I've learned from my mistakes and I'm correcting my code as I come back across it.

Maurits:

Can you really declare a table with @Table syntax?

Sure! This is MS SQL Server dialect, and all that declaration does is create a variable of type Table. It is not actually creating a table in the database. The closest description is that it is an in-memory table (that is not completely accurate, but close enough).

Not that it matters, of course, since this is one of the most painful ways I have seen to filter data![:S]

Anonymous pretty much said it all, but in addition I'd like to offer a humble plea to all MS SQL Server developers to stop giving their stored procedure names an sp_ prefix. When you call a procedure that begins with "sp_", SQL Server assumes it's a system stored procedure, and looks for it in the master database first. If it doesn't find it there, then it looks in the current database. So by calling it sp_blahblah, you're adding lots of useless overhead.

Of course, this is yet another reason why Hungarian notation is getting slowly killed off. Developers think, "Hey, this is a stored procedure, so I better name it 'sp' for 'stored procedure'! I know that the object is in a Stored Procedures folder in Query Analyzer/Enterprise Manager, and the script begins with 'create procedure', but I better tack on this useless naming convention, just in case!" 

Geez. Keep it simple, people: "create procedure dbo.ProjectSearch ..."

 

 

Wizard thank you so much for the comment to MS SQL devs.



Hungarian notation drives me nuts.....

OK, so in a stored procedure,

    DECLARE @Table TABLE (...)

is effectively the same as

    CREATE TABLE #Table (...)

where the # means "temporary to connection"



Learn something new every day...



As regards sp_:

sp_ is only useful if the following conditions both hold:

1) You're creating the procedure INSIDE the "master" database

2) You want to call the procedure OUTSIDE of the "master" database



I've only been tempted to create my own sp_ procedure twice.  The
first time was before I knew any better.  The second time I
created an sp_send_cdosys_mail stored procedure for sending
HTML-formatted email. It's probably the single-most-called procedure on
my database server.

...I
created an sp_send_cdosys_mail stored procedure for sending
HTML-formatted email.

Why would you use the database to send e-mails?



I'm curious, not trying to flame :)

When your DBA revokes everyone's rights to creating objects in the database, this is the reason why.  Don't complain when it happens, just walk away and say, "Yes, I see why things must be this way..."

A million reasons.



A select few:



Online work-ticket system - when a ticket is added or modified, an
email goes to all the relevant people containing details of the
addition/modification and a link to the full history of the ticket.



Online advertising management system - when an ad reaches it's
end-of-run, an email goes out to the interested parties including a
summary of the ad's run and a link to performance stats



Shipping of physical products - when somebody buys something that
requires physical fulfillment, an email goes to the department whose
job it is to stick the thing in a box and send it out



etc.

Let me offer a couple of possibilites as to what this coder might have
been thinking. Please note that I'm not justifying this code -- just
trying to understand what the person was thinking (if anything).



1) Perhaps the database was set to auto-generate temporary indexes when
SQL Server thinks it needs them. I can't remember the actual name of
the setting. Anyway, perhaps the person was trying to avoid having the
temporary indexes generated, as I doubt SQL Server generates temp
indexes for an in-memory table. ?



2) Perhaps the person believes that having "OR" conditions in a WHERE
clause causes SQL Server to do things too slowly. I've been told this
is sometimes true, although I haven't seen it. Of course, I doubt it
could ever be slower than this solution...



Just some thoughts.

Maurits:

OK, so in a stored procedure,

    DECLARE @Table TABLE (...)

is effectively the same as

    CREATE TABLE #Table (...)

where the # means "temporary to connection"

Not *quite*.  They behave a little differently.



http://support.microsoft.com/default.aspx?scid=kb;en-us;305977

No one.  NO ONE, could be that stupid.  I call shenanigans!

Holy gaaaawd!



    dZ.

A million reasons.



A select few:



Online work-ticket system - when a ticket is added or modified, an
email goes to all the relevant people containing details of the
addition/modification and a link to the full history of the ticket.



Online advertising management system - when an ad reaches it's
end-of-run, an email goes out to the interested parties including a
summary of the ad's run and a link to performance stats



Shipping of physical products - when somebody buys something that
requires physical fulfillment, an email goes to the department whose
job it is to stick the thing in a box and send it out

I should have phrased my question differently:  Why would you have
the database send the e-mails rather than in the middle-tier of your
application?  It just seems that it's a business role that you
wouldn't want the database to be performing.

Ytram:

Why would you use the database to send e-mails?



I'm curious, not trying to flame :)

Maurits:

A million reasons.





A select few:





Online work-ticket system - when a ticket is added or modified, an
email goes to all the relevant people containing details of the
addition/modification and a link to the full history of the ticket.





Online advertising management system - when an ad reaches it's
end-of-run, an email goes out to the interested parties including a
summary of the ad's run and a link to performance stats





Shipping of physical products - when somebody buys something that
requires physical fulfillment, an email goes to the department whose
job it is to stick the thing in a box and send it out





etc.

I can only think of one reason why you would want all of this in the
database: Lack of a middle-tier.  Why everyone wants to implement
programs wearing the shackles of T-SQL when rich languages like C# or
even VB6 (by comparison) exist is beyond me.

I can only think of one reason why you would want all of this in the
database: Lack of a middle-tier.

That could be the flaw in my thinking - I'm just assuming that there is
a middle-tier.  Of course, some smaller projects may not require
one, but the examples given seem to indicate a scope that would need at
least one middle-tier.

I guess you're against xp_sendmail too, then?

Mmmmmm.....WTF....*drool*

Thanks Alex, this was one of the best yet![:|]

Chris F:

I can only think of one reason why you would want all of this in the
database: Lack of a middle-tier.  Why everyone wants to implement
programs wearing the shackles of T-SQL when rich languages like C# or
even VB6 (by comparison) exist is beyond me.

This violates the principle of "right tool for the right job."



A database is a good place for keeping data.  It is not a good
place for anything else, including sending e-mail, generating HTML
pages, number crunching, filtering network traffic, scanning for
viruses, processing 3D images, or message queuing.

Ytram:

I can only think of one reason why you would want all of this in the
database: Lack of a middle-tier.

That could be the flaw in my thinking - I'm just assuming that there is
a middle-tier.  Of course, some smaller projects may not require
one, but the examples given seem to indicate a scope that would need at
least one middle-tier.

Need is such a strong word =).  I've seen some pretty large
systems implemented with the absolute thinnest of middle tiers. 
Even a bank...

Ytram:

I should have phrased my question differently:  Why would you have
the database send the e-mails rather than in the middle-tier of your
application?  It just seems that it's a business role that you
wouldn't want the database to be performing.

Some people run batch jobs on their data.  Sql server has a
built-in scheduler and the easiest thing you can have it run is a
sproc.  Sometimes you want your batch script to send an
email.  People work with the tools they know, and there are a lot
of people who know SQL.  Especially the type of people who would
be asked to write such a script.



I don't believe in putting much logic in the Sql server any more, but it's a religious bias.

Chris F:

I can only think of one reason why you would want all of this in the
database: Lack of a middle-tier.  Why everyone wants to implement
programs wearing the shackles of T-SQL when rich languages like C# or
even VB6 (by comparison) exist is beyond me.

I have heard again and again the argument that the database will likely
outlive the API paradigm that is used to interface with it.  I
believe it's true.  For me that's not a good enough argument not
to use C# for the middle-tier, but I have a hard enough time convincing
LOTS of people that a middle tier is a good thing.  Many people
(DBAs, older analysts, managers) know and trust the database, but don't
get OO.

Many projects are small projects and it's harder to justify the benefits of OO.

mizhi:

No one.  NO ONE, could be that stupid.  I call shenanigans!

I don't think this person is stupid.  They know how to construct a
valid stored procedure, and a moderately complicated one at that. 
They do know how to use a where clause and construct temporary
tables.  I think they had another reason for making this code.



Maybe they have some form of Autism or OCD that drives them to write
code like this.  Maybe they were incented to take a long time
making the code.  Maybe they were angling for a more powerful
database server, so they made an awful pig of a sproc.





You can't deny that the code is well formatted and easy to read. 
It is as if they wanted to control every step of the process despite
the obvious impact on the performance.



It's hard to explain why the developer not only avoided selecting by
where clause, but also avoided deleting by where clause and used the
bit flag to determine deletes.  It's pretty paranoid behavior.





I wish the original poster could give some clue as to why this convoluted approach was used.

Chris F:

I can only think of one reason why you would want all of this in the
database: Lack of a middle-tier.  Why everyone wants to implement
programs wearing the shackles of T-SQL when rich languages like C# or
even VB6 (by comparison) exist is beyond me.

Others offered "lack of a middle tier in simpler systems."



There is seperation of duties.  If you have an expert DBA who
might want to change table structures, by encapsulating logic in a
stored procedure he does not have to have the expert middle tier
programmers change all of their code.  The expert middle tier
programmers don't need to make decisions about how to write queries
(and writing them as efficiently as possible can require some
expertise).



There is avoiding database roundtrips.  Tree-structured data can
require recursive database calls that might be better handled on the
database side.



Specific to the email question: if you have multiple front ends, or
different ways that a ticket/order/etc. can be indirectly changed, it
might make sense to have notification emails fire based on a trigger on
a base table.  This is perhaps indicitve of poor planning of the
system, but anyone who's worked in the "real world" for a while will
tell you that systems grow and mutate unexpectedly, or you inherit
other people's "WTF's?" and re-architecting everything is not cost
effective.



Don't get me wrong, T-SQL is not a rich language by a long shot. 
But if you have to pound in nails sometimes, you'll appreciate having a
$10 hammer in addition to a $200 ratchet set.

FWIW, I've never yet found it necessary to use a trigger (triggers
scare me.)  I have a wide variety of middle-tier languages in use
- VBScript, VB.NET, and Perl.  And I send mail from all of them,
using various methods appropriate to the tier in question.  But I
still find it best, in certain occasions, to use sp_send_cdosys_mail in
a stored procedure.



The stored procedure creates a CDO.Message object using sp_OACreate, sets various properties, and finally calls the Send method.

It looks like what the author was trying to accomplish was to allow one procedure to handle zero or more selection criteria (the IF LEN(@sProjectName) > 0 allows the caller to include or leave out the project name criterion, for example). Of course, this could still be accomplished with a single SELECT (using ISNULL in the WHERE clause for each criterion...)

There's a bit of debate on this thread on whether email, HTML, business logic, etc should exist on the database tier.  My question to those that promote the "absence" of a middle tier is: what DON'T you put into the database, other than the presentation layer?  My last job put all kinds of business logic in stored procs: sending & creating HTML emails using T-SQL cursors, randomly generating passwords, etc.  It was a maintenance nightmare...

Also, keep in mind that in some cases, its the database gets replaced, rather than the client programs written in C#, VB, etc.

I think this is why we really need business object servers, or OOP databases or something.  T-SQL just sucks at these kind of things.  The next version of sql server is going to have the .NET runtime in it, but i guess thats a two edged sword.

Anonymous:

what DON'T you put into the database, other than the presentation layer?

You only put in the database what "belongs" in the database.  This varies from app to app.



A good rule of thumb - for me - is to treat stored procedures as if
they were method calls in an OO solution.  They should be agnostic
of how they are called, and do what their names imply.

I think the problem stems from the "6 optional parameters for a search" requirement. I've seen various standard solutions proposed before:

1. isnull in the where clause: where isnull(@city,city) = city AND isnull(@customer, customer) = customer AND...
2. OR in the where clause: where (@city is null OR @city = city) AND (@customer is null or @customer = customer) AND...
3.using if statments to break it out into 2^n cases where n = number of optional parameters.

#1 has poor performance but is highly maintainable and readable
#2 has better performance and remains highly maintainable and readable
#3 has the best performance but is the most difficult to read and maintain

My favorite is #2 but people tell me to use #3 because of the performance boost (I ignore them). Notice that every solution has drawbacks and so any standard solution you pick could easily be forbidden because of the drawbacks. So after reading a whole bunch of "don't do this ever" advice, you could easily fall into the trap of avoiding the known pitfalls and ending up with a monstrosity which is worse than any standard solution.

I've noticed that people have not volunteered some "better solutions" to the "6 optional parameters" requirement. Perhaps because every solution stinks so far. Though this has by far got to be the worst I've seen. Thanks for another great WTF Alex.

Anonymous:

I think the problem stems from the "6 optional parameters for a search" requirement.

Another option is to (shudder) DYNAMICALLY BUILD a sql string WITHIN the stored procedure, and then run it...



DECLARE @sql nvarchar(4000)



SELECT

    @sql = '...' +

'WHERE 1 = 1 ' +

CASE WHEN @ProductID IS NULL THEN ''

    WHEN @ProductID = '' THEN ''

    ELSE 'AND ProductID = ''' + Replace(@ProductID, '''', '''''') + ''' '

END +

CASE WHEN @ProductName IS NULL THEN ''


    WHEN @ProductName = '' THEN ''


    ELSE 'AND ProductName = ''' + Replace(@ProductName, '''', '''''') + ''' '


END



ew... slow and hard to maintain.

Anonymous:

There's a bit of debate on this thread on
whether email, HTML, business logic, etc should exist on the database
tier.  My question to those that promote the "absence" of a middle
tier is: what DON'T you put into the database, other than the
presentation layer?  My last job put all kinds of business logic
in stored procs: sending & creating HTML emails using T-SQL
cursors, randomly generating passwords, etc.  It was a maintenance
nightmare...

Also, keep in mind that in some cases, its the database gets replaced, rather than the client programs written in C#, VB, etc.

I think this is why we really need business object servers, or OOP
databases or something.  T-SQL just sucks at these kind of
things.  The next version of sql server is going to have the .NET
runtime in it, but i guess thats a two edged sword.

I don't know if anyone is "promoting" the absence of a middle tier,
just pointing out that there is a cost to implementing a more complex
system.  For a smaller system, the efficiencies of a three-tier
system might not make up for the cost of an expert programmer's
hours.  Some of us are implementing systems alone, or as part of a
very small team.  That doesn't mean that the database is the best
place to generate a random password; but when choosing between two
tiers, it might be the best place to send an email.

Sometimes the database gets replaced, but in practice I believe that
happens less often then replacing a client program.  Often I think
you'll see several database clients.  One makes the best
predictions one can based on what one knows.

Implementing anything object-oriented does indeed suck in T-SQL, or
really, in any relational database.  I've seen more than one
person try to use a system (with tables like "ObjectTypes",
"ObjectAttributes", and "Objects") to try to fake it.  There are
object oriented databases (hit Google) and I'm curious whether any will
gain significant market share.

.NET in Yukon
SQL Server 2005 will open the door to all kinds of great WTF's, I'm
sure.  Things that should be done in T-SQL gratuitously being
written in .NET, code that should NOT be on the database layer being
done there... but the inclusion of an XML column type will be even
worse, I bet.  How many tables-inside-a-column will we see?

OneFactor:

I think the problem stems from the "6
optional parameters for a search" requirement. I've seen various
standard solutions proposed before:

1. isnull in the where clause: where isnull(@city,city) = city AND isnull(@customer, customer) = customer AND...
2. OR in the where clause: where (@city is null OR @city = city) AND (@customer is null or @customer = customer) AND...
3.using if statments to break it out into 2^n cases where n = number of optional parameters.

#1 has poor performance but is highly maintainable and readable
#2 has better performance and remains highly maintainable and readable
#3 has the best performance but is the most difficult to read and maintain

As an aside: Now I'm no DB expert, but I tend to default to #2 until
the performance kills me.  I don't know what it is about such
queries, but MS SQL Server seems unable to parse out the expression
(null is null OR null = city).  When the performance becomes
unbearable I simply switch over to dynamic SQL.  It sucks, it's
hard to maintain, but I don't know any better way.

There's got to be something I'm missing.  Perhaps it's the fact
that @city can change from row to row and the query planner can't
automatically recognize that certain variables are immutable for the
duration of the query.  Is there a way to declare them immutable
so that #2 is as performant as dynamic SQL?

Maurits:

Another option is to (shudder) DYNAMICALLY BUILD a sql string WITHIN the stored procedure, and then run it...



DECLARE @sql nvarchar(4000)



SELECT

    @sql = '...' +

'WHERE 1 = 1 ' +

CASE WHEN @ProductID IS NULL THEN ''

    WHEN @ProductID = '' THEN ''

    ELSE 'AND ProductID = ''' + Replace(@ProductID, '''', '''''') + ''' '

END +

CASE WHEN @ProductName IS NULL THEN ''


    WHEN @ProductName = '' THEN ''


    ELSE 'AND ProductName = ''' + Replace(@ProductName, '''', '''''') + ''' '


END



ew... slow and hard to maintain.

In my experience, the dynamic SQL method by far the most performant for non-trivial search methods.  What would you do?

1. and 2. are the same as each other and the following:

NULLIF(@ProductID, ProductID) IS NULL AND

NULLIF(@ProductName, ProductName) IS NULL AND...



3. I have never attempted



For small tables I do 1. or 2.

For large tables I do 3.

That is to say - for large tables I do dynamic SQL

Damn f*ing priceless!