• MainCoder (unregistered)

    32231213

  • me (unregistered)

    exec. 'nuff said.

  • Gumpy Guss (unregistered)

    I worked at a similar place. Classic old historic building, expensive office furniture, and early April-in-Paris temperature IQ's. Advice: start shopping around your resume ASAP.

  • ClaudeSuck.de (unregistered)

    CREATE PROCEDURE AdHocQuery

    AS

    DECLARE sSQL varchar(4000)

    SET sSQL = ' SELECT ''Please don''''t write ad-hoc queries'' FROM CompanyRules WHERE User = ''Boss'' OR User = ''Senior Developer'''

    Exec(sSQL)

  • Prosthetic Lips (unregistered)

    Ha! My current company doesn't have a stored procedure-only policy, but I found a stored procedure that was supposed to replace some SQL from our code, and it looked very similar. Basically:

    1. Remove the SQL from the code
    2. Convert to stored procedure with dynamic SQL
    3. ???
    4. Profit!

    All of the SQL-injection, none of the optimization of being in a stored procedure ... plus all of the pain of calling a stored procedure (not seeing the SQL in the code, so having to lookup the stored procedure in the database; not seeing the arguments directly, but passing them as parameters).

  • Anonymous Coward (unregistered)

    Its a little like doing keyhole surgery wearing hockey gloves.

  • James L (unregistered)

    Unfortunately, if the application was written in ASP.NET, then this style of coding is the only way to get SPs to work when using SqlDataConnection objects.

    If ASP.NET wasn't the front end, then I can see it's a WTF.

  • A Nonny Mouse (cs)

    looks like someone didn't know the difference between char and varchar and got a bit out of control

  • James L (unregistered)

    I'll correct myself.

    This is the only way to get SPs to work if they use Temp Tables

  • B. Grenger (unregistered)

    I don't understand what is wrong with hte code since I don't know hte language. Can someone enlighten me?

  • Joon (unregistered) in reply to James L
    James L:
    Unfortunately, if the application was written in ASP.NET, then this style of coding is the only way to get SPs to work when using SqlDataConnection objects.

    That's some good trolling. Well done!

    Let's see how many people fall for it and start saying "But to the front end this proc would look no different no matter how the back-end does it..."

  • Single User (unregistered)

    I never understood why exec was even legal in stored procedures.

    CAPTCHA: autocompleted, yet again.

  • Flipper (unregistered) in reply to B. Grenger
    B. Grenger:
    I don't understand what is wrong with hte code since I don't know hte language. Can someone enlighten me?
    And I guess you don't know hte Engrish either.
  • dpm (cs) in reply to B. Grenger
    B. Grenger:
    I don't understand what is wrong with hte code since I don't know hte language. Can someone enlighten me?
    I can explain, but I can't guarantee that enlightenment will occur.

    The command should be hard-coded, as in

       SELECT COUNT FROM USERS WHERE DATE = @STARTDATE
    

    but instead they build a string which is then passed to the EXEC function, which has to parse it and lookup all of the column names and basically do all the work, every time that procedure is called, that should have been done once when the procedure is created.

    It is similar to using an interpreted language instead of a compiled one.

  • Alan (unregistered) in reply to James L

    I never understood Microsofts insistence on providing ways to tie the front-end directly to the database.

    Write some code and use Object Data Binding instead

  • Adriano (unregistered) in reply to Gumpy Guss
    Gumpy Guss:
    Advice: start shopping around your resume ASAP.
    From the article:
    Thankfully, I don't work there anymore
    .
  • OzPeter (cs) in reply to Single User
    Single User:
    I never understood why exec was even legal in stored procedures.

    You can't dynamically specify table names in a SP, thus it makes it harder to create a generic SP that might have to extract data from different tables.

    I am dealing with this situation with multiple data logging tables generated by similar pieces of equipment.

    Either I write an SP with dynamic SQL or I write 15 separate SPs.

  • Eoin (unregistered)

    The usual justification for using dynamic sql (hence the existence of exec in the language) is that you don't know what table you're going to be checking against until runtime.

    If it's a limited set then you can take an input param and case input_param when 1 then table1 when 2 then table2

    Otherwise you have to accept the user input and validate

  • Oliver (unregistered) in reply to B. Grenger

    The exec is used to run a dynamically built SQL string, so effectively you can do an SQL injection by passing appropriate parameters, eg setting dbname to "; delete from "

  • Smash King (cs) in reply to Single User

    If exec wasn't bad enough...

    1- As A Nonny Mouse noticed, the coder used char when varchar would make much more sense. 2- What's with Ltrim and Rtrim? I don't know all RDBMS out there but I'm pretty sure the most widely used accept Trim or a similar function. And probably the idiot had to trim the string because of reason 1 above. 3- Why would you join the tables UserRelationship and Batch and not use them in your where or select clauses? (Yes I know it is possible to filter this way, but someone as clueless as this coder wouldn't know it)

    Of course, exec still is their worst problem... no, wait. Forget that. Their worst problem is a total cluelessness. The rest is consequence.

  • James L (unregistered) in reply to Joon
    Comment held for moderation.
  • ClaudeSuck.de (unregistered) in reply to OzPeter
    OzPeter:
    Single User:
    I never understood why exec was even legal in stored procedures.

    You can't dynamically specify table names in a SP, thus it makes it harder to create a generic SP that might have to extract data from different tables.

    I am dealing with this situation with multiple data logging tables generated by similar pieces of equipment.

    Either I write an SP with dynamic SQL or I write 15 separate SPs.

    In case of an authentication (that's what the sp does) it doesn't make much sense to query different tables for different logons. I'd say there is a terrible design problem somewhere.

  • dabcabc (unregistered) in reply to Joon

    Slap SET NOCOUNT ON at the start of the procedure when using with temporary tables and it'll work fine. :)

  • TopGooner (unregistered)
    Comment held for moderation.
  • ClaudeSuck.de (unregistered) in reply to Smash King
    Smash King:
    If exec wasn't bad enough...

    1- As A Nonny Mouse noticed, the coder used char when varchar would make much more sense. 2- What's with Ltrim and Rtrim? I don't know all RDBMS out there but I'm pretty sure the most widely used accept Trim or a similar function. And probably the idiot had to trim the string because of reason 1 above. 3- Why would you join the tables UserRelationship and Batch and not use them in your where or select clauses? (Yes I know it is possible to filter this way, but someone as clueless as this coder wouldn't know it)

    Of course, exec still is their worst problem... no, wait. Forget that. Their worst problem is a total cluelessness. The rest is consequence.

    Hmmm, no clue? Point 1) See before last paragraph. Point 2 is wrong because SQL Server does not have a TRIM function, no, no. That's pure old SQL Server. Point 3) you still have to use the table names in your FROM clause (and for some DBs you even need to put the WHERE fields in the SELECT clause first). That's pure old SQL.

    I doubt that it's the front-end developer's fault. It's more the database designer who went gaga.

    Hmmm, who has clues and who has not?

  • Protector one (unregistered) in reply to dpm

    Thanks for that. Exercises for the reader piss me off to no (all?) extent.

  • Kefer (cs)

    TRWTF is not being able to explain the problem to the senior developer. A little SQL injection example should do the trick. And if an innocent injection using a select doesn't cut it, show him (/her) a more aggressive variant.

    Showing the lack of performance improvement should be easy as well.

    And trimming datetime fields? WTF?

  • Zapp Brannigan (unregistered) in reply to Protector one

    A minor improvement, you could get rid of the ltrim/rtrim and just use trim.

  • dpm (cs) in reply to OzPeter
    OzPeter:
    Either I write an SP with dynamic SQL or I write 15 separate SPs.
    Or you write one piece of code and run it through a preprocessor fifteen times to create fifteen SPs for you.

    #if defined TARGET_TABLE_ALPHA

    define THENAME WhackAlpha

    define THETABLE AlphaUser

    #elif defined TARGET_TABLE_BRAVO

    define THENAME WhackBravo

    define THETABLE BravoUser

    [...] #endif

    CREATE PROCEDURE [dbo.][THENAME] AS SELECT WHATEVER FROM THETABLE

  • TopGooner (unregistered)

    I agree that use of exec would have been better served using sp_executesql and passing in the parameters as parameters rather than inlining them in the sql.

  • Tim (unregistered)

    I realise this isn't a database forum but does anyone have a feel for the quantitative performance improvement from using stored procedures? my asp.net application uses direct SQL because this seemed to be the best way to support 4 different database platforms without a lot of re-coding and re-testing

    we mainly use simple sql statements with primary keys and indexed lookups, and we have never had performance problems in a real life situation.

  • campkev (cs) in reply to Protector one
    Protector one:
    Thanks for that. Exercises for the reader piss me off to no (all?) extent.

    Thanks for that. Mangled expressions piss me off to no end.

  • Kefer (cs)
    ... + '' and c.UserID = '' + ltrim(rtrim(str(@UserID))) + “and c.Password = “ + ltrim(rtrim(str(@Password)))

    Stored procedure as shown above won't work anyway:

    • Missing whitespace between the value for c.UserID and the keyword "and"
    • Missing quotes around the value for c.Password
  • brazzy (cs) in reply to TopGooner
    TopGooner:
    Apart from the use of char, rtrim, ltrim, and bearing in mind I nothing about the overlaying application, then there is actually not a whole heap wrong with this SQL.

    Except that it is wide open to SQL injection, when protection against SQL injection was one of the two stated goals of using stored procedures in the first place - and most likely the SPs that could have been non-dynamic still were, defeating the other stated goal (performance) as well.

  • Mr B (cs) in reply to OzPeter
    OzPeter:
    Single User:
    I never understood why exec was even legal in stored procedures.

    You can't dynamically specify table names in a SP, thus it makes it harder to create a generic SP that might have to extract data from different tables.

    I am dealing with this situation with multiple data logging tables generated by similar pieces of equipment.

    Either I write an SP with dynamic SQL or I write 15 separate SPs.

    Or you use a better schema.

  • dpm (cs) in reply to Mr B
    Mr B:
    OzPeter:
    Either I write an SP with dynamic SQL or I write 15 separate SPs.
    Or you use a better schema.
    What a wonderful dream, that the person responsible for writing the SPs is the same person with the authority to change the schema.

    Not in my world, though.

  • ClaudeSuck.de (unregistered) in reply to Tim
    Tim:
    I realise this isn't a database forum but does anyone have a feel for the quantitative performance improvement from using stored procedures? my asp.net application uses direct SQL because this seemed to be the best way to support 4 different database platforms without a lot of re-coding and re-testing

    we mainly use simple sql statements with primary keys and indexed lookups, and we have never had performance problems in a real life situation.

    After some 10 years with sps I can say that for me there has never been a performance improvement. But you get a nice separation of data and presentation layer (apart from other advantages mentioned in other posts). I used to use ad-hoc SQL in the code but gave it up in the end. The problem with today's IDEs is that you can put SQL in many places (like data objects), not only the code. Have a nice time finding all the places where you used a given fieldname or table or so. You will always miss some of them. Using sps you may query system tables (syscomments for MSSQL, something similar in Oracle) or ideally you already have them in a script for deployment where you can use Search or Search+Replace to your advantage. I agree that developers often suffer from that approach but they should also understand that there are only some 10% or less developers who can actually write better queries than

    SELECT * FROM MyTable

    While this is not a big sin when "programming" your Access address book it can (and will) create a big performance hit on a datamart or similar, let me say tables above some 100.000 lines.

  • Muppet (unregistered) in reply to Tim

    One main benefit of using Stored Procedures, other than the previously indicated speed benefits of compile once, run many and the ability to carry out complex procedures all on the server rather than sending data back to the client to do data processing - potentially huge amounts - doesn't necessarily go with the whole client / server paradigm is that they separate out the data access logic from the business logic of your application.

  • Smash King (cs) in reply to ClaudeSuck.de
    ClaudeSuck.de:
    Smash King:
    If exec wasn't bad enough...

    1- As A Nonny Mouse noticed, the coder used char when varchar would make much more sense. 2- What's with Ltrim and Rtrim? I don't know all RDBMS out there but I'm pretty sure the most widely used accept Trim or a similar function. And probably the idiot had to trim the string because of reason 1 above. 3- Why would you join the tables UserRelationship and Batch and not use them in your where or select clauses? (Yes I know it is possible to filter this way, but someone as clueless as this coder wouldn't know it)

    Of course, exec still is their worst problem... no, wait. Forget that. Their worst problem is a total cluelessness. The rest is consequence.

    Hmmm, no clue? Point 1) See before last paragraph. Point 2 is wrong because SQL Server does not have a TRIM function, no, no. That's pure old SQL Server. Point 3) you still have to use the table names in your FROM clause (and for some DBs you even need to put the WHERE fields in the SELECT clause first). That's pure old SQL.

    I doubt that it's the front-end developer's fault. It's more the database designer who went gaga.

    Hmmm, who has clues and who has not?

    1) Where? In the CodeSOD? I have no idea what you meant here. 2) Ok, my mistake. SQL Server doesn't have Trim. But as someone above noted, trimming a datetime field is a WTF in itself and provides another detail as to why it's not absurd to say this guy is clueless. 3) You did not understand my point at all. Please go back and re-check. Try again later.

    You're probably right that the DB schema must be a mess, but it's no excuse for this set of stupidities.

  • OzPeter (cs) in reply to Mr B
    Mr B:
    OzPeter:
    Either I write an SP with dynamic SQL or I write 15 separate SPs.

    Or you use a better schema.

    Except in the real world there are somethings you cannot choose nor can you change.

  • Madox (unregistered) in reply to James L
    James L:
    I'll correct myself.

    This is the only way to get SPs to work if they use Temp Tables

    What kind of crack are you smoking? Since when can't you use a temp table within a regular sproc?

    What are you on, Sybase 6?

  • Madoxx (unregistered) in reply to Joon
    Joon:
    James L:
    Unfortunately, if the application was written in ASP.NET, then this style of coding is the only way to get SPs to work when using SqlDataConnection objects.

    That's some good trolling. Well done!

    Let's see how many people fall for it and start saying "But to the front end this proc would look no different no matter how the back-end does it..."

    For the uninitiated, the db calls go into the Data Access Layer. That's assuming a standard 3 tier app structure (presentation, business logic, data access). So for the love of the spaghetti monster, stop calling sprocs in your front end.

  • pbo (unregistered) in reply to ClaudeSuck.de
    ClaudeSuck.de:
    After some 10 years with sps [...] let me say tables above some 100.000 lines.

    Shouldn't you be calling those things 'tuples' after ten years?

  • ubersoldat (cs)

    hehehe... exec()... that's funny

  • redbeard0x0a (unregistered) in reply to Smash King
    Smash King:
    If exec wasn't bad enough... 2- What's with Ltrim and Rtrim? I don't know all RDBMS out there but I'm pretty sure the most widely used accept Trim or a similar function. And probably the idiot had to trim the string because of reason 1 above.

    The wonderful world of Microsoft SQL Server is where you find only RTRIM and LTRIM with no TRIM in sight. Don't forget they didn't have DATE or TIME datatypes until SQL Server 2008, you had to make due with DATETIME...

  • Mr B (cs) in reply to dpm
    dpm:
    Mr B:
    OzPeter:
    Either I write an SP with dynamic SQL or I write 15 separate SPs.
    Or you use a better schema.
    What a wonderful dream, that the person responsible for writing the SPs is the same person with the authority to change the schema.

    Not in my world, though.

    Get a new world then, in my world the schema and the queries are treated as a single unit, under the control of the database scrum, which has representatives from ops, developers and dbas.

    Lack of organisational structure is the first wtf, compounded by simply going along with it for the sake of an easy life. Typical wtf'er really, complaining about the state of things and yet unwilling to do anything about it.

    :)

  • Mr B (cs) in reply to OzPeter
    OzPeter:
    Mr B:
    OzPeter:
    Either I write an SP with dynamic SQL or I write 15 separate SPs.

    Or you use a better schema.

    Except in the real world there are somethings you cannot choose nor can you change.

    Of course you can change it. Either you re-write the schema, or you have a shoddy proprietary schema trigger-linked into a sensible schema, of your design, which you can report on.

    Sheesh, you lot are such defeatists at times!

    :)

  • threecheese (unregistered) in reply to OzPeter
    OzPeter:
    Single User:
    I never understood why exec was even legal in stored procedures.

    You can't dynamically specify table names in a SP, thus it makes it harder to create a generic SP that might have to extract data from different tables.

    I am dealing with this situation with multiple data logging tables generated by similar pieces of equipment.

    Either I write an SP with dynamic SQL or I write 15 separate SPs.

    IMO you could use a trigger (if tsql) to write that data to a common table.

  • aysunmoon (unregistered) in reply to Adriano

    This has grown to concern me more than a little bit over the years -- Why is our standard response to incompetence within our workplaces an automatic "Run, Forrest! Run!"? Where are the efforts to actually do something about the rampant unprofessionalism within our trade? Other professions seem to have found ways to enforce at least some semblance of discipline upon their respective practitioners, so why do we continue to throw our hands up and say "What are you gonna do?" when faced with the messes left behind by the countless clueless hacks infesting our ranks? Is it that we are having trouble letting go of the hobbyist spirit that drew so many of us to this career to begin with? (And yes, I'm aware of some small irony in asking this on a site founded on our twisted fascination in watching it all go so wrong). So really... WTF?

  • SoonerMatt (unregistered)

    Does this actually prevent SQL injection attacks either?

    What if the password was "UNION DELETE FROM USERS"?

Leave a comment on “All Pain, No Gain”

Log In or post as a guest

Replying to comment #:

« Return to Article