January 18, 2012 at 11:50 am
When I execute the script below, I get the message:
Msg 102, Level 15, State 1, Line 1
Incorrect syntax near 'RETAIL_SALES'.
If I print @SELECT, @FROM, and @WHERE and paste it into a new query, it works. I believe the problem is within the WHERE statement since I don't get the error when I do not execute @WHERE. Here is the code that is generated:
SELECT RETAIL_SALES.[Sales $ LW]
FROM Evy_RH_Objects.dbo.RETAIL_SALES RETAIL_SALES
WHERE RETAIL_SALES.CUST_NO='MACY03'
I appreciate any help.
DECLARE @CustNovarchar(100)
DECLARE @SELECT VARCHAR(4000)
DECLARE @FROM VARCHAR(4000)
DECLARE @WHERE VARCHAR(4000)
SELECT @CustNo='MACY03'
SELECT @SELECT =
'SELECT RETAIL_SALES.[Sales $ LW]'
SELECT @FROM =
'FROM Evy_RH_Objects.dbo.RETAIL_SALES RETAIL_SALES'
SELECT @WHERE =
'WHERE RETAIL_SALES.CUST_NO=''' + @CustNo + ''''
EXEC(@SELECT + @FROM + @WHERE)
January 18, 2012 at 11:56 am
It may be a missing space between the FROM and the WHERE.
The easiest way to find out is to print the full string, copy and paste it into a query window, and attempt to execute it:
DECLARE @CustNo varchar(100)
DECLARE @SELECT VARCHAR(4000)
DECLARE @FROM VARCHAR(4000)
DECLARE @WHERE VARCHAR(4000)
SELECT @CustNo='MACY03'
SELECT @SELECT =
'SELECT RETAIL_SALES.[Sales $ LW]'
SELECT @FROM =
'FROM Evy_RH_Objects.dbo.RETAIL_SALES RETAIL_SALES'
SELECT @WHERE =
'WHERE RETAIL_SALES.CUST_NO=''' + @CustNo + ''''
PRINT @SELECT + @FROM + @WHERE
EXEC(@SELECT + @FROM + @WHERE)
For better assistance in answering your questions, please read this[/url].
Hidden RBAR: Triangular Joins[/url] / The "Numbers" or "Tally" Table: What it is and how it replaces a loop[/url] Jeff Moden[/url]
January 18, 2012 at 12:03 pm
I cannot overstate how ill advised your dynamic sql strategy is. You are asking someone to hijack your system with sql injection and you are ruining any chance for plan re-use.
Dynamic SQL can be a wonderful tool, but not when it's used like this. Read this article as a start if you want to understand the appropriate way to implement dynamic sql.
January 18, 2012 at 12:23 pm
I was missing the space. Thanks Chris.
January 18, 2012 at 12:47 pm
I second the opinion of not doing dynamic sql like this. Your whole query is only a single simple select statement. Even if this is "internal" only you are still losing plan reuse/caching. The performance of this type of solution will be all over the place. It may be fine most of the time but at occasionally it will be horribly slow and difficult to pinpoint. If it is externally facing the potential for sql injection is scary.
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
January 18, 2012 at 1:20 pm
Sean Lange (1/18/2012)
I second the opinion of not doing dynamic sql like this. Your whole query is only a single simple select statement. Even if this is "internal" only you are still losing plan reuse/caching. The performance of this type of solution will be all over the place. It may be fine most of the time but at occasionally it will be horribly slow and difficult to pinpoint. If it is externally facing the potential for sql injection is scary.
Even if it's supposed to be internal only I don't think it's any better. Most successful attacks gain a foothold somewhere in the system and then branch out performing "internal" operations. The best defense is to harden all systems with the assumption that someone bad will eventually have access to the interface.
January 18, 2012 at 1:26 pm
agreed sp_executesql should *always* be used. But quite why dynamic sql is even being used here is anyones guess 🙂
January 18, 2012 at 1:35 pm
bteraberry (1/18/2012)
Sean Lange (1/18/2012)
I second the opinion of not doing dynamic sql like this. Your whole query is only a single simple select statement. Even if this is "internal" only you are still losing plan reuse/caching. The performance of this type of solution will be all over the place. It may be fine most of the time but at occasionally it will be horribly slow and difficult to pinpoint. If it is externally facing the potential for sql injection is scary.Even if it's supposed to be internal only I don't think it's any better. Most successful attacks gain a foothold somewhere in the system and then branch out performing "internal" operations. The best defense is to harden all systems with the assumption that someone bad will eventually have access to the interface.
I wholeheartedly agree. I put "internal" in quotes intentionally because there really is nothing that is internal anymore. Even internal apps are being ported to the web so we can "access it from home". At the very least disgruntled employees are internal and some of the most dangerous types of attackers. They have intimate knowledge of the systems and have access to a lot of stuff so they can fly under the radar.
_______________________________________________________________
Need help? Help us help you.
Read the article at http://www.sqlservercentral.com/articles/Best+Practices/61537/ for best practices on asking questions.
Need to split a string? Try Jeff Modens splitter http://www.sqlservercentral.com/articles/Tally+Table/72993/.
Cross Tabs and Pivots, Part 1 – Converting Rows to Columns - http://www.sqlservercentral.com/articles/T-SQL/63681/
Cross Tabs and Pivots, Part 2 - Dynamic Cross Tabs - http://www.sqlservercentral.com/articles/Crosstab/65048/
Understanding and Using APPLY (Part 1) - http://www.sqlservercentral.com/articles/APPLY/69953/
Understanding and Using APPLY (Part 2) - http://www.sqlservercentral.com/articles/APPLY/69954/
January 18, 2012 at 1:35 pm
Dave Ballantyne (1/18/2012)
agreed sp_executesql should *always* be used. But quite why dynamic sql is even being used here is anyones guess 🙂
I'd guess it's not the whole picture, Dave.
For better assistance in answering your questions, please read this[/url].
Hidden RBAR: Triangular Joins[/url] / The "Numbers" or "Tally" Table: What it is and how it replaces a loop[/url] Jeff Moden[/url]
January 18, 2012 at 1:37 pm
jayw-1061726 (1/18/2012)
I was missing the space. Thanks Chris.
You're welcome, thanks for the feedback. Just out of curiosity, why are you using dynamic SQL for this? Is there a chunk of your code missing for clarity?
For better assistance in answering your questions, please read this[/url].
Hidden RBAR: Triangular Joins[/url] / The "Numbers" or "Tally" Table: What it is and how it replaces a loop[/url] Jeff Moden[/url]
January 18, 2012 at 1:53 pm
I left out a part of the original code for simplicity here, that forced me to use dynamic. I have a string coming from a txt file (@StylesSelected) that needs to be in the correct syntax to be placed in the IN clause.
'HK5707','HK56415','HK57528','HK57582','HK56479'
I am fairly new to SQL so I couldn't make it work within a standard SQL script. I would rather not use dynamic. Any suggestions would be appreciated.
Declare @StylesSelectedvarchar(500)
SELECT @StylesSelected = CONVERT(varchar(max), BulkColumn) FROM OPENROWSET( BULK '\\EVY11\RetailSales$\MACY03_RecapStyles.txt', SINGLE_BLOB) rs
--Print @StylesSelected
SELECT @WHERE =
'WHERE RETAIL_SALES.CUST_NO=''' + @CustNo + ''''
+ ' and CONVERT(varchar(4),RETAIL_SALES.YR) + Right(''0'' + CONVERT(varchar(3),RETAIL_SALES.MO),2) + CONVERT(varchar(1),RETAIL_SALES.WK) <= ''' + @EndDate + ''''
+ ' and RETAIL_SALES.[PID] IN (' + @StylesSelected + ')'
January 18, 2012 at 2:04 pm
Try this article http://www.sqlservercentral.com/articles/Tally+Table/72993/ on how to split a comma delimeted string. Then its just a simple join.
January 18, 2012 at 2:10 pm
And next, for speed, convert this:
+ ' and CONVERT(varchar(4),RETAIL_SALES.YR) + Right(''0'' + CONVERT(varchar(3),RETAIL_SALES.MO),2) + CONVERT(varchar(1),RETAIL_SALES.WK) <= ''' + @EndDate + ''''
to something like this:
AND RETAIL_SALES.YR = YEAR(@EndDate ) etc
which will make it SARGable (if there are any indexes on any of YR, MO and WK, they could be used)
For better assistance in answering your questions, please read this[/url].
Hidden RBAR: Triangular Joins[/url] / The "Numbers" or "Tally" Table: What it is and how it replaces a loop[/url] Jeff Moden[/url]
Viewing 13 posts - 1 through 12 (of 12 total)
You must be logged in to reply to this topic. Login to reply