Would you send this simple SQL back for rework?

We have a web application in which the users perform ad-hoc queries based on parameters that have been entered. I might also mention that response 开发者_JAVA技巧time is of high importance to the users.

The web page dynamically construct a SQL to execute based on the parameters entered. For example, if the user enters "1" for "Business Unit" we construct a SQL like this:

SELECT * FROM FACT WHERE 
BUSINESS_UNIT = '1'
--AND other criteria based on the input params

I found that where the user does not specify a BUSINESS_UNIT the following query is constructed

SELECT * FROM FACT WHERE 
BUSINESS_UNIT LIKE '%'
--AND other criteria based on the input params

IMHO, this is unnecessarily (if not grossly) inefficient and warrants sending the code bad for modification but since I have a much higher rate of sending code back for rework than others, I believe that I may be earning a reputation as being "too picky."

If this is an inappropriate question because it is not a direct coding Q, let me know and I will delete it immediately. I am very confused whether subjective questions like this are allowed or not! I'll be watching your replies.

ty

Update:

I am using an Oracle database.

My impression is that Oracle does not optimize "LIKE '%'" by removing the condition and that leaving it in is less efficient. Could someone confirm?

The two queries are completely different (from the point of view of a result set)

SELECT * FROM FACT;

and

SELECT * FROM FACT WHERE  
BUSINESS_UNIT LIKE '%';

The first will return all rows, the second, if there are NULL values those rows will not be returned because anything compared to NULL is NULL and therefore does not satisfy the predicate. This is how it would work in Oracle.

Although that looks grossly inefficient, I just tested it out in SQL Server, and the query optimizer was smart enough to filter it out.

In other words,

SELECT * FROM FACT WHERE 
BUSINESS_UNIT LIKE '%'

and

SELECT * FROM FACT

generated the exact same query plan. So there shouldn't be a performance difference (depending on your DB engine I guess), although it does look kind of sloppy.

That may affect your decision to send it back or not. Personally I probably would, but if you're under a cloud already then it's something you can probably relax on, at least in terms of performance.

That query, with the BUSINESS_UNIT LIKE '%', doesn't look quite efficient -- I suppose it'll force a full scan of the table...

So, yes, I would try to have that query reworked, to deal with that kind of situation in a proper way -- or, at least, I would report that problem through our bug-tracker (Not sure of the priority I would assign, but, still, the problem would be logged somewhere, and someone will correct it someday, when there is no higher-priority problem to deal with).

SELECT * is a red flag. Specify a column list for the query.

I would send it back and I like the question.

All code review is to some extent subjective. The criteria should be based on a number of factors.

• Does it work.

• Does it meet reasonable expectations of performance, maintainability, usability, and scalability

While I have not tested this particular construct -- I suspect (as do you) this code will do horrible things to your SQL server. Thus performance and scalability are called into question.

The writer wanted a dummy statement so that he/she could append "and" to all the subsequent statements. changing it to a better "no-op" statement like 1==1 would help. Or they could do slightly more work and insert the "where" intelligently.

I would question why bind variables are not being used (unless there is a good reason in particular cases):

SELECT * FROM FACT WHERE 
BUSINESS_UNIT = '1'

Why is it not:

SELECT * FROM FACT WHERE 
BUSINESS_UNIT = :bu