I hate parameterized queries


  • Garbage Person

    Really. I fucking hate them. Yes, it's the right way, but that doesn't change the fact that one line of literal SQL in the early dev version of the app I'm building exploded into 130 lines of utter insanity.

    Yes, a lot of that bulk came from adding features like proper handling of nulls, two lines to call out to a function that parses date input, etc.  - but the vast majority of it was just going through the motions of instantiating each parameter, setting it's value (WHY THE FUCK DOESN'T NPGSQL LET ME DO BOTH OF THESE AT ONCE!?!??), etc.

     And it doesn't help that the idiots on my team failed to follow the fucking ER Diagram when they were building the database and FORGOT HALF THE FUCKING FIELDS.

    Come to think of it, maybe my hatred is misplaced. Maybe I shouldn't be hating the parameterized queries, maybe I should be hating the twat who wrote the original code in a completely bogus manner.



  • @Weng said:

    Really. I fucking hate them. Yes, it's the right way, but that doesn't change the fact that one line of literal SQL in the early dev version of the app I'm building exploded into 130 lines of utter insanity.

    Yes, a lot of that bulk came from adding features like proper handling of nulls, two lines to call out to a function that parses date input, etc.  - but the vast majority of it was just going through the motions of instantiating each parameter, setting it's value (WHY THE FUCK DOESN'T NPGSQL LET ME DO BOTH OF THESE AT ONCE!?!??), etc.

    And it doesn't help that the idiots on my team failed to follow the fucking ER Diagram when they were building the database and FORGOT HALF THE FUCKING FIELDS.

    Come to think of it, maybe my hatred is misplaced. Maybe I shouldn't be hating the parameterized queries, maybe I should be hating the twat who wrote the original code in a completely bogus manner.


    Can't say that I care for them much either.  I generally just do regular queries with the escaping handled by the DAL.


  •  I normally use stored procs for everything.



  • Why on earth wouldn't you just use sprocs if this is so hard for you?



  • @tster said:

     I normally use stored procs for everything.

    yeah me too, seems to take a lot of the pain out



  • SPs wont necessarily save you if you have any dynamic SQL creating the call. If you don't then you're essentially using prepared statements anyway.
    Prepared statements also give you a performance and concurrency boost, as they can bypass a lot of the parsing, saving CPU and locking of crucial system resources the parser has to use.
    They're not that painful in day to day stuff and the benefits are well worth it.
    You're smart guys though and I'm sure you already know all this really.


  • Garbage Person

    It's not hard. It's just fucking tedious.

    And frankly, in I'm-not-pissed-because-I-just-burned-a-ton-of-time-doing-fuckall retrospect, the majority of this code needed to be written anyway because '' != null and full on half of the code in there was fixing that.

     TRWTF is the machinations that I'm going to have to deal with to pry data out of the database and shove it into asp.net form elements.



  • @Weng said:

    It's not hard. It's just fucking tedious.

    And frankly, in I'm-not-pissed-because-I-just-burned-a-ton-of-time-doing-fuckall retrospect, the majority of this code needed to be written anyway because '' != null and full on half of the code in there was fixing that.

     TRWTF is the machinations that I'm going to have to deal with to pry data out of the database and shove it into asp.net form elements.


    It definitely sounds like you are doing something very wrong.

    Have you thought about sharing some code here and asking for some help?



  • @Weng said:

    ... because '' != null ...

    I take it you've never been put through the agony of working with Oracle. 


  • Garbage Person

    @Farmer Brown said:

    It definitely sounds like you are doing something very wrong.
    Have you thought about sharing some code here and asking for some help?
     

    If there's a terser way that isn't unmaintainable gobblygook, I'd LOVE to know.  The layer above this passes through direct HTTP GET/POST values, so everything's stringified.

    DB connection and transaction is opened in constructor, closed in destructor.

    Heavy anonymization of the prime-mover method in question follows:

             public Boolean insertStuff(String foo, String bar, String baz ,String quux, string mumble, string bazola, string ztesch, string thud, string grunt, string bletch, string wombat)
            {
                //begin senseless grinding where I build *ONE* SQL statement in several hundred lines of code!!!
                string sql = "insert into stuff (foo, bar, baz, " +
                    "quux, mumble, bazola, ztesch, thud, grunt, bletch, wombat) " +
                    " values (:foo, :bar, :baz, :quux, :mumble, :bazola, :ztesch," +
                    " :thud, :grunt, :bletch, :wombat)";
                NpgsqlCommand dbcmd = new NpgsqlCommand(sql, dbcon);

                NpgsqlParameter fooP = new NpgsqlParameter("foo", NpgsqlDbType.Date);
                NpgsqlParameter barP = new NpgsqlParameter("bar", NpgsqlDbType.Char, 50);
                //omitted for brevity

                dbcmd.Parameters.Add(fooP);
                dbcmd.Parameters.Add(barP);
                //omitted for brevity

                //MIDDLE ENDIAN DATES ARE HATEFUL THINGS AND SHOULD BE DESTROYED
                String[] dateParts = blowUpDate(startDate);   //Yes, this method works in an intelligent manner. For sanity's sake someone should probably make it return an int[] instead, though
                foo.Value = new NpgsqlDate(Int16.Parse(dateParts[2]), Int16.Parse(dateParts[0]), Int16.Parse(dateParts[1]));           

               //I may pay for this professionally, but I want oracle-like null vs. null-string! But as like, an option that I can turn on per-query.
                if (bar.Equals(""))
                    barP.Value = null;
                else
                    barP.Value = physicalDescription;

                // omitted for brevity

               try
                {
                    int rows = dbcmd.ExecuteNonQuery();
                    if (rows == 0)
                        return false; // if nothing was inserted, then we're doin it wrong
                }
                catch (Npgsql.NpgsqlException ne)
                {
                    throw ne;   //todo: This is not sane and is included for debugging purposes. If you see it, tail the server log. Should return false instead
                }
                return true;
            }

     Offhand I can think of one or two refactors that should be done (tommorrow sounds like a great day for that), but aside from working with the NpgsqlStatement.Parameters data structure directly (which results in longer line-lengths, piss poor readability, and not much shorter of a method) can't come up with much.



  • Are you really writing that for every single call???


  • Garbage Person

    If by "call" you mean "form that submits to the database", yes, I am... Because this is the only one. Yes, this should be a one-day/one-dev project, but internal politics require fucking overblown user and usability testing for even the most rudimentary commonsense piece of software that faces a user (it used to be ALL software... and then we bored a user panel to death watching a log daemon)....

    So I have two devs, one of whom is myself (both of us also have other responsibilities), three designers who write code and submit it to SVN because they think we developers are slacking, and a PROJECT MANAGER (!!! IT'S FOUR ASPX FILES AND AN HTML FORM!) who won't let us scrap their garbage and start from scratch so we can be done in half an hour. Plus we have to go into a code freeze for more user testing at every milestone (which occurs approximately every 20 minutes of dev time), so it's absolutely ridiculous.

     

    If, on the other hand, you mean "call" as in every argument to the function, yes. They all get exactly the same treatment as bar (foo is a bit different because it's a date). Looking at it in retrospect, I could have streamlined it a bit, but maintainability might suffer. In addition, the real code is a lot harder to look at because the variable names and database fields are all relatively long (if not multiple) words.

     

    If you actually mean "call" as in every time the method is used, no, I don't write that out every time because the code is already compiled.

     

    If you mean something else entirely, please elaborate. 



  • Use an ORM package like NHibernate or LLBLGen. Those will still support PostgreSql while taking all the manual query building out of your hands.

    Things become as easy as setting a few strongly-typed properties and calling a save method. Getting strongly typed values from strings can easily be done with Parse or TryParse methods. Even DateTime supports those, so why are you manually trying to parse a date?



  • @Weng said:

     Offhand I can think of one or two refactors that should be done (tommorrow sounds like a great day for that), but aside from working with the NpgsqlStatement.Parameters data structure directly (which results in longer line-lengths, piss poor readability, and not much shorter of a method) can't come up with much.

    I've got a couple of comments on this code.  First is that it might be nicer to use a proper record struct that represents the set of inputs to be inserted, rather than multiple arguments.  This leads into the second, which is that you should probably be using reflection to do most of this, rather than repeating code per parameter.  The code using reflection will likely be shorter since you'll be handling each type case once (and also actually more general and easier to maintain).


  • Garbage Person

    @arty said:

    record struct
     @arty said:
    should probably be using reflection

    Oh I wish so badly that the maintainence dogs were capable of understanding either concept. As of now you get negatives on code reviews for being "overly complex".

    It's sick that those of us who know what we're doing have to come down to the level of understanding of interns, rather than just handing them code that they SHOULDN'T immediately understand and letting them FUCKING LEARN.

    And yes, I am applying for a new job. PFY position just opened up at my former university and I got along real well with the BOFH when I was there. It's not programming, but it IS secure and an environment which I know for a fact is sane. (and has room for advancement - the BOFH is getting shoehorned into a teaching position, and the head of IT is reaching retirement age - plus eventually they'll pay me to get my master's and start teaching CS).



  • @Weng said:

    If you mean something else entirely, please elaborate.

    I mean that this should all be abstracted away into a proper BI/DAL. You are copy/pasting code into every form that you do not need to make.

    To add a foo to the DB, it should be as simple as something like: FooDBObject.Add(param1,param2,param3);

    Your problem isn't parameterized queries, but rather high school grade code.



  • @LoztInSpace said:

    SPs wont necessarily save you if you have any dynamic SQL creating the call. If you don't then you're essentially using prepared statements anyway. Prepared statements also give you a performance and concurrency boost, as they can bypass a lot of the parsing, saving CPU and locking of crucial system resources the parser has to use. They're not that painful in day to day stuff and the benefits are well worth it. You're smart guys though and I'm sure you already know all this really.
    You can do dynamic SQL within SPs.  Also, I'm confused, are you saying Prepared Statements are faster than Stored Procs?



  • @tster said:

    @LoztInSpace said:

    SPs wont necessarily save you if you have any dynamic SQL creating the call. If you don't then you're essentially using prepared statements anyway. Prepared statements also give you a performance and concurrency boost, as they can bypass a lot of the parsing, saving CPU and locking of crucial system resources the parser has to use. They're not that painful in day to day stuff and the benefits are well worth it. You're smart guys though and I'm sure you already know all this really.
    You can do dynamic SQL within SPs.  Also, I'm confused, are you saying Prepared Statements are faster than Stored Procs?

    Yes you can do dynamic SQL in SPs and that too will expose you to SQL injection.

    Prepared statements will not necessarily improve performance over SPs but they will tend to perform better over ad-hoc SQL when you need to execute the same thing several times, i.e. when the SQL is the same but only the parameters (or what could be parameters) differ.  The statement only needs to be parsed once but can be executed many times.  TThat said, it can be true even when simply executing an SP because the parser still needs to know your permissions, parameter types etc so you still win if you prepare & parameterise your call.  Depending on how you set your parameters to the SP you can also remove dynamic SQL by doing this.



  •  @LoztInSpace said:

    @tster said:

    @LoztInSpace said:

    SPs wont necessarily save you if you have any dynamic SQL creating the call. If you don't then you're essentially using prepared statements anyway. Prepared statements also give you a performance and concurrency boost, as they can bypass a lot of the parsing, saving CPU and locking of crucial system resources the parser has to use. They're not that painful in day to day stuff and the benefits are well worth it. You're smart guys though and I'm sure you already know all this really.
    You can do dynamic SQL within SPs.  Also, I'm confused, are you saying Prepared Statements are faster than Stored Procs?

    Yes you can do dynamic SQL in SPs and that too will expose you to SQL injection.

    Prepared statements will not necessarily improve performance over SPs but they will tend to perform better over ad-hoc SQL when you need to execute the same thing several times, i.e. when the SQL is the same but only the parameters (or what could be parameters) differ.  The statement only needs to be parsed once but can be executed many times.  TThat said, it can be true even when simply executing an SP because the parser still needs to know your permissions, parameter types etc so you still win if you prepare & parameterise your call.  Depending on how you set your parameters to the SP you can also remove dynamic SQL by doing this.

    In my experience with SQL Server 2005+ and the way I write SPs, there is really no difference between the two from a execution plan cache point of view.  A SP gets executed as a series of prepared statements from what I can tell from the profiler.  The biggest difference between a plain old SQL prepared statement and a SP is that of security.  When using SPs, you can grant the user only access to the SP and not to the underlying tables and still be able to make the necessary data modifications.  But with prepared statements, you need to grant INSERT, UPDATE, DELETE perms to the underlying tables.  When using SPs this way (users have no access to the underlying tables), you are guaranteed a consistent approach to modifying data.  That is one of the main reasons I do not use ORM tools like NHibernate and LINQ as it opens up the tables to forms of abuse. (Yes, I know you can connect NHibernate and LINQ to SPs, but that tends to diminish the value of those tools).


Log in to reply