Why use INNER JOINS?



  • <FONT color=#0000ff>After my 3rd week on this project rewriting just about every page, I just had to post this little gem.    </FONT>

    <FONT color=#0000ff>This is a very simple search routine.    Anyway, I'm sure you can imagine what I'm going through right now!!!</FONT>

    <FONT color=#0000ff></FONT><FONT color=#0000ff></FONT> 

    <FONT color=#0000ff>[URL=http://imageshack.us][IMG]http://img505.imageshack.us/img505/2765/mydailywtf8pr.jpg[/IMG][/URL]</FONT>



  • ...Just so you know this search routine would rarely return results before it caused the server to time out.    You don't even want to know what it looked like in Profiler



  • The injection potential for this application is quite high.

    If it's all output only, I'd suggest a stored procedure or a view.



  • @Kev777 said:

    ...Just so you know this search routine would rarely return results before it caused the server to time out.    You don't even want to know what it looked like in Profiler



    LIKE '%string%' is pretty good for doing that.  Since it's totally unindexed (in mysql, and probably every other db), and all that.

    If you're doing a LIKE '%string%', you're doing something wrong.  It's either your table structure, your query, or both.



  • ... at least there is no cursor



  • @merreborn said:


    If you're doing a LIKE '%string%', you're doing something wrong.  It's either your table structure, your query, or both.


    Okay, so how are you supposed to offer an "all records where field X contains Y somewhere in it" search?



  • @emurphy said:

    @merreborn said:

    If you're doing a LIKE '%string%', you're doing something wrong.  It's either your table structure, your query, or both.


    Okay, so how are you supposed to offer an "all records where field X contains Y somewhere in it" search?



    Depends.  In the case of searching for text in an article database, for example, you use a fulltext index, and the correct match() function.

    If you're doing something stupid, like, say, storing a list of names of employees who work in a department in a single column, then you need to change your schema, and separate those employees out into a separate table, and then a simple JOIN with a WHERE employee = 'name' will do (although it'd be even better to use an employee ID instead, but that's a whole different subject)


    Again: LIKE '%string%' is totally unindexed, which is totally unacceptable for any reasonably large dataset.



  • The real problem here was that is was requesting a recordset from the database for each record returned from the first query.       A new recordset object was created for each row in the first recordset and then again each time again for the second recordset. (eg.  For each x,   then for each y,   then  for each z execute a query against the db!!! )

    Furthermore the sql string had security issues because it directly attached request variable to the query string.  

    Oh and notice how the recordset is converted into an array just to get the recordcount.  lol

     

     



  • @snoofle said:

    ... at least there is no cursor

    lol this is basicaly a triple nested cursor done with ASP code!       A cursor would actually be faster then this crap



  • @Kev777 said:

    @snoofle said:

    ... at least there is no cursor

    lol this is basicaly a triple nested cursor done with ASP code!       A cursor would actually be faster then this crap


    A stored proc returning a cursor might be the way to go, although I'd try a stored proc returning a collection first. I have a sinking feeling from the SQL syntax that the DBMS is Oracle, in which case they are violating a lot of basic principles, like minimize hard parses, etc.

    What's really scary is that I think I recognize the company from the object names. If so, they have a LOT of data. That they are not seeing fast enough.



  • This post made me redesign my database and learn about FULLTEXT & MATCH 🙂



  • <font size="2"><font face="Georgia">Ignoring the SQL for the moment, do NOT open a recordset inside another one... if you want to use this method put the 1st recordset into an array, close the recordset and use the array members instead.

    You can also code this way:

    if x=y then response.write("test")

    (no end if needed)

    and request.form("var") is marginally quicker then request("var")

    Doing the data manipulation in SQL server will be quicker than in your asp page e.g. stored procs

    And don't forget to lookup "sql injection" as well 😉
    </font></font>



  • @mhuk said:

    <FONT size=2><FONT face=Georgia>Ignoring the SQL for the moment, do NOT open a recordset inside another one... if you want to use this method put the 1st recordset into an array, close the recordset and use the array members instead.

    You can also code this way:

    if x=y then response.write("test")

    (no end if needed)

    and request.form("var") is marginally quicker then request("var")

    Doing the data manipulation in SQL server will be quicker than in your asp page e.g. stored procs

    And don't forget to lookup "sql injection" as well 😉
    </FONT></FONT>

    don't worry I totally understand all this stuff.   The original programmer opened 1000's of recordsets and basically crashed the server every time a search was requested 🙂

     In short, I only spent about 2 mins on this fix and just created a sql server view.    The LIKE search works fast enough with the view and I limited the result set to 300 records.

    Often when you encounter bad code there is only a need to make it do EXACTLY what the client asked for.  In this case the client only wanted it to work faster.     If he wants it to be more secure then he can pay extra 🙂   After all I don't even want to look at his s-hit code.   

    Trust me when a client asks you to modify sh-it code don't waste your time rewriting it.  

     

     

     


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.