Comment On All Pain, No Gain

32231213

exec. 'nuff said.

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.

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)

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).

Its a little like doing keyhole surgery wearing hockey gloves.

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.

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

I'll correct myself.

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

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

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..."

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

CAPTCHA: autocompleted, yet again.

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.

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.

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

Gumpy Guss:

Advice: start shopping around your resume ASAP.

From the article:

Thankfully, I don't work there anymore

.

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.

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

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 "

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.

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..."

Examples of this WTF used in real life, too:

http://social.msdn.microsoft.com/Forums/en-US/adodotnetdataproviders/thread/70af8b07-8e1a-44c7-9f27-6ec6dc0900f6/

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.

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

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. The problem is that one of the tables that is joined exists in another database, as defined by the @database parameter. It's not possible in SQL to use a parameter as a table identifier in a table join, so dynamic SQL is probably the easiest way to do it. Not really worthy of a WTF entry imo.

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?

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

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?

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

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

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.

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.

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.

...
+ '' 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

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.

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.

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.

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.

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.

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.

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.

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?

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.

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?

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

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...

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.

:)

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!

:)

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.

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?

Does this actually prevent SQL injection attacks either?

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