Cannot Use ColdFusion CFQueryParam With SQL TOP Directive
Posted January 10, 2007 at 12:36 PM by Ben Nadel
I just came across a strange little error that took me a few minutes to debug. I was running this SQL query:
- DELETE FROM
- reward
- WHERE
- order_id = 0
- AND
- delivery_id = <cfqueryparam value="#id#" cfsqltype="CF_SQL_INTEGER" />
- AND
- id IN
- (
- SELECT TOP <cfqueryparam value="#(qUnused.reward_count - FORM.quantity)#" cfsqltype="CF_SQL_INTEGER" />
- id
- FROM
- reward
- WHERE
- order_id = 0
- AND
- delivery_id = <cfqueryparam value="#id#" cfsqltype="CF_SQL_INTEGER" />
- ORDER BY
- id DESC
- )
... and it was throwing this error:
[Macromedia][SQLServer JDBC Driver][SQLServer]
Line 11: Incorrect syntax near '@P2'.
Looking at the debugging SQL:
- DELETE FROM
- reward
- WHERE
- order_id = 0
- AND
- delivery_id = (param 1)
- AND
- id IN
- (
- SELECT TOP (param 2)
- id
- FROM
- reward
- WHERE
- order_id = 0
- AND
- delivery_id = (param 3)
- ORDER BY
- id DESC
- )
... I can see that Param 2 is the SQL TOP value. Going in and take the CFQueryParam out and replace it with:
- SELECT TOP #(qUnused.reward_count - FORM.quantity)#
... things work just fine. Apparently, you cannot use the CFQueryParam with the SQL TOP statement. Not sure what this is all about. It seems like this is a place where SQL injection could occur (especially in my case where the TOP value is being derived from a FORM value). Perhaps this is just not the best way for me to accomplish this goal.
Reader Comments
There was a bug in recent updates to CF that caused using Maxrows on a query to be problematic. I went changing some old queries that used the maxrows to use top instead and came across this as well. I don't think this is a CF but rather a MS SQL issue.
There was a bug in recent updates to CF that caused using Maxrows on a query to be problematic. I went changing some old queries that used the maxrows to use top instead and came across this as well. I don't think this is a CF but rather a MS SQL issue.
Sorry my comment got posted twice, but FYI I got a 500 null error when I submitted it both times and didn't think it went through...also I checked remember my information and it does not seem to have done so. Just letting you know.
Thanks for letting me know. Something has gone a bit cookey with the comment posting in the last few days. I have to figure out what happened. I am on it!
Ok I made an update to the comment. Please let me know if anything goes wrong again.
Thanks!
How about this Ben? Before the query do:
declare @param int
set @param = <cfqueryparam value="#id#" cfsqltype="CF_SQL_INTEGER" />
then in your query do a:
select top @param......
not tested, but should work....
OK i lied - maybe it doesn't work...
Todd,
Nice thought. I guess it just can't handle binding in general? As Brian says above, at least it's not a ColdFusion error, but one of the SQL server.
If it's that important, you could always make the sql dynamic like this:
DECLARE @param int
set @param = <cfqueryparam value="#id#" cfsqltype="CF_SQL_INTEGER" />
DECLARE @sql nvarchar(4000)
SELECT @sql = ' SELECT top ' + @param + 'columns and stuff'+
' FROM ...' +
' WHERE...'
EXEC sp_executesql @sql
That one WAS tested ;)
In general, unless it is completely necessary, I avoid dynamic sql like that. The reason is that it is my understanding that when using dynamic sql like that MS SQL recompiles the execution plan on every request - so it would negate the performance benefit of using the queryparam to create a prepared statement I believe. This would also be the case even if your dynamic SQL was in a stored procedure...execution plan is recreated on each call...
Todd,
Sorry, that goes way beyond my understanding. I mean, I get what you are doing, but I know zero about dynamic SQL execution :)
What about this Brian/Ben?
<cfquery...>
set rowcount <cfqueryparam value="1" cfsqltype="cf_sql_integer">
select *
from tbl...
where stuff...
set rowcount 0
</cfquery>
Would this cache the execution plan? It solves the TOP n issue, and it works...
Though I would be nervous doing that with a delete...
sorry for being a comment wh*re...
I would imagine the reason it doesn't work is you can't bind a variable to the TOP directive.
Why not just use:
SELECT TOP #val(qUnused.reward_count - val(trim(FORM.quantity)))#
This should prevent SQL Injections and the val() function will make sure your value is seen as an integer.
@Todd,
No worries on being a wh*re ;) Comments rock. Thinking out-loud rocks.
@Dan,
Nice tip on Val().
Good stuff all around.
