• Paula Bean (unregistered) 
    begin
	set @_Query = dbo.GetQuery('FristType1');
	execute sp_executeSQL @_Query
   end
  • (nodebb)

    But DistType 57 does something just a little bit different. Because you know, Reasons. Now you have a bug which will pop up once every seventh full moon when Jupiter and Mercury align with a Monday in a leapyear.

  • Dave Hemming (unregistered)

    Somewhere in there is going to be something like

    if @_DistType = 47 begin set @_Query = dbo.GetQuery('DistType47-NEW'); execute sp_executeSQL @_Query

  • James (unregistered)

    I wish I had old screen shots and code snippets. The greatest WTF I ever saw was a place where they thought DTS packages were a great place to implement business logic. No generating end of day reports, sending exceptions, or updating summery tables for the days transactions using clunky stored procs. Nope all that could be do in the beautiful drag and drop world of DTS.

    Wait some of you who remember SQL Sever 7.0 and SQL Server 2000 are asking; wasn't DTS a and ETL tool didn't just apply a serious of transforms and offer some limited branching to deal with failures etc. Yes - but you see it also had an 'ActiveX Object' which you place in the designer. That object could run vbscript, and get com objects for the DTS job itself and guess what someone discovered the current job step was a writable property. So just like that you added a looping construct...

    This effectively allow the ETL tool to become a full blown control-break-processing engine. All kinds of logic could now be described. It was like what COBOL would have been if someone had invented it in 1997.

  • Alistair (unregistered)

    The shortened code fails in interesting ways if @_DistType is not an integer in the range 1-75.

  • Scott (unregistered)

    Or, we could have the decision in a business logic layer, go directly to the query we want to execute, and get rid of this foolishness completely. Name the queries something sensible while we're up.

  • Michael Albert (unregistered)

    Regarding "not changing code", I was involved in one project (as a consultant, I must admit) which had rather onerous rules for code revision, but data update was basically ok, so they basically used stored procedures to implement an ad hoc language, the "code" being read "as data" from the database. This way, they only had to change data, not code.

  • (nodebb)

    Oddly, my experience with MySQL is not that the procedure language is hard to use due to being too data-focused, but that it's hard to use due to not being data-focused enough. In particular, a major problem when writing complex procedures is the fact that a result set - the full set of data returned from a query or stored procure - is not a first-class object in the procedure language; you can't assign it to a variable or do much in the way of passing it around.

    The second half of my answer at https://stackoverflow.com/a/26539823/1709587 is basically a giant writeup of all the ways I discovered in which MySQL's procedure language is garbage in the course of writing one big procedure in it, and is probably of interest to anyone wondering why Remy views them with such distrust.

  • Josh (unregistered)

    Besides the "naming convention" this can be a valid approach, if you have multiple instances of a single solution each behaving differently depending on the database referenced.

    Development environments supporting the discovery of named references in code it would prefer a if @_DistType = 1 begin set @_Query = dbo.GetQuery('DistType1') end ... execute sp_executeSQL @_Query style implementation with a full identifier string in a well-known function call. That also solves the input validation problem.

    Regarding change management: Use a database that supports versioning of stored code and schema. Or use a combination of development environment and database that support extraction of textual representations of stored code (also granting "wtf is this query doing").

  • ZZartin (unregistered) in reply to Scott

    You just know there's another layer on top of this that maps some useful query name from the business layer like GetCustomerInfo to those integers.

  • (nodebb)

    Of course, the real reason for NOT defaulting to Remy's approach is that, congratulations, you've now created a SQL injection vulnerability in your code (since, presumably, the stored procedure in question is taking user input and then determining how to call the underlying stored procedure). Granted, there are additional steps that can be taken before or with the CAST operation to sanitize the inputs, but the "simpler" approach, and what will show up more in SQL Programming for Dummies manuals, is the explicit IF statements with a "safe" call to the underlying procedure that doesn't directly use the untrustworthy input.

  • WTFGuy (unregistered)

    I halfway suspect that in the beginning there were two DistTypes numbered "1" & "2". At which point using an If/else pattern would make sense. Lots of people believe that variable allocation / deallocation is expensive whereas branching code is cheap.

    Then came the third DistType and now if/else/else nesting would get messy. Since SQL lacks a switch / CASE construct for control flow* they implemented the new requirement as a set of three simple IFs on the same variable.

    As more DistTypes get added, when is the moment when somebody has the guts to rewrite what's already there (and buy responsibility for any bugs introduced), versus simply C&Ping DistTypeN to become DistTypeN+1 ?? Who among us has the 'nads to throw the first stone at that ogre?

    • Yes, most SQL dialects have a CASE construct. But it's essentially an n-valued equivalent of the terniery expression operator. Which means it's fine for calling functions or making values, but not for control flow. At least not without even more contortions. (e.g. heavily nested terniery lambdas anyone?)
  • WTFGuy (unregistered) in reply to izzion

    @izzion: My aged recollection of at least MS's SQL Server is there was a global administrative setting that enabled or disabled the ability to execute a query or stored proc whose name was derived by any form of computation or retrieval as opposed to having to be explicit in the source code. Intended of course as a defense in depth against an injection attack.

    The fact these folks have clearly switched off any such protections their SQL dialect supports is also not a good sign on the security engineering front.

  • OldCoder (unregistered) in reply to WTFGuy

    Actually, there is a switch/case construct available in every database. It's called a table.

    Instead of doing a switch on "DistType<n>" and then a repeatable line of bespoke SQL, why not just make <n> the key to the table? If that seems too hard, it can be one field in the key.

  • dpm (unregistered) in reply to Dave Hemming

    You. I want you on my team.

  • (nodebb) in reply to Cabbage

    Temporary tables are the general purpose data structure of stored procedures.

    Thankfully, we don't do much stored procedure programming in our shop. Whenever I see complex stored procedures, it reminds me of programming BASIC in the 70's and 80's.

  • Gumpy_Gus (unregistered)

    I seem to remember about a decade ago I was working at a place with a database with like 177 fields.

    The Business Analyst decided we should have a way to search on any field.

    The Database Czar would not let us run custom queries.

    So I think we ended up with one function with 177 SELECT statements.

    Memory fuzzy, which is a good thing in this case.

  • (nodebb)

    I love these Get Query type functions. I mean, why would anyone use anything else, like, a stored procedure??

  • (nodebb) in reply to ZZartin

    And that the only difference between the text of DistType23 and DistType24 is that the former references the "CustomerInfo" table where the latter references "SupplierInfo".

  • snoofle (unregistered) in reply to Michael Albert

    Agreed. I've worked on numerous financial applications where changing code, regardless of size, complexity or importance of the change, there was a massive multi-level review and signoff process before anything could get deployed. But data changes could be done on senior developer say-so alone. Out of sheer self defense and the need to be reasonably responsive where possible, we put as much as we could into db tables named configlogic_xxx to shield it from deployment/approval bean counters, but all developers knew that they were really changing logic.

    However, it was limited to senior developers, and everyone knew that abuse was grounds for termination.

    Sometimes you have to commit stupidity to get around the bureaucracy.

  • DCL (unregistered)

    "But what I’m not about to do is copy and paste the same block of code with minor variations." Then you are never going to make it as a coding monkey in an offshore coding centre.

  • Tony (unregistered)

    What I'm getting here is that neither the author of the code or the author of this article know much about stored procedures. They're no more a breeding ground of WTF-ery than any other programming language. A good database Developer will write good, elegant, maintainable stored procedures. In my experience, most app developers write poor database code, then blame the database for it.

  • Daemon (unregistered)

    A beautiful variation on this, is code that is built for many different hardware variations. I work on a C-based embedded code-base that supports a Lot(!) of different products. Deciding which product it runs on, is done compile-time with pragma's. ('#if defined(PRODUCT_NAME) /// #else if / #endif'-style) Most of these blocks are the same though. Or VERY similar. What you end up with, is a .c file, containing 10000 lines of code, about 20% of which might, or will not be 'active' at a given time. The only reason I'm still sane (lol) is that Eclipse is able to show me which code is or is not active. Eclipse's search function doesn't have that notion though.....

  • Rudolf Polzer (google)

    I've done that before:

    https://github.com/divVerent/s2tc/blob/master/s2tc_algorithm.cpp#L1417

    I did it to move function arguments into template arguments to force the compiler to duplicate the implementation code and remove conditional branches.

    Why?

    Because I knew that almost all callers of the library will only ever invoke one of the many variants throughout their entire runtime. So eliminating those branches was a noticeable performance benefit, despite branch prediction.

    Obviously, none of this applies to the SQL code in question.

  • Rudolf Polzer (google)

    I've done that before:

    https://github.com/divVerent/s2tc/blob/master/s2tc_algorithm.cpp#L1417

    I did it to move function arguments into template arguments to force the compiler to duplicate the implementation code and remove conditional branches.

    Why?

    Because I knew that almost all callers of the library will only ever invoke one of the many variants throughout their entire runtime. So eliminating those branches was a noticeable performance benefit, despite branch prediction.

    Obviously, none of this applies to the SQL code in question.

  • hung out to dry (unregistered)

    [sarcasm on]What's this? Queries stored /in a database/? Queries stored /in a table/ in a database? My god, Codd must be smiling down on us.[sarcasm off]

    Actually, more likely that GetQuery is a stored procedure that builds queries, since that's a common anti-pattern.

Leave a comment on “Going the Distwince”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article