insert into ... select ... from ...where...in

N

nicholas

I have this in my stored procedure:

SET @CatInsert = 'INSERT INTO tbl_shopscats (shopID, categoryID) SELECT ' +
CONVERT(varchar,@shopID) + ', categoryID From tbl_categories Where
categoryID IN (' + @categoryID + ')'

@categoryID is a string of ID's separated by comma's. ex: "1,25,78"

So if @shopID = 63

We should have these records inserted:
record1: shopID= 63 and categoryID=1
record2: shopID= 63 and categoryID=25
record3: shopID= 63 and categoryID=78

The problem is that it only insert when the categoryID is "1".
The second problem is that it also inserts "1" if the ID is for ex. "124"

If there is no "1" in the string, nothing is inserted. ex: when @categoryID
= "25,78"

Any ideas ?
Thx,
Nic
 
K

Karl Seguin

bah..sorry about that,
anyways, if you go into your master database and execut:

declare @sql nvarchar(1024)
declare @query varchar(128)
set @query = '5,10,11'
set @sql = 'SELECT dbid from sysdatabases where dbid in (' + @query + ')'
exec sp_executesql @sql

you'll see that it works as expected... you might want to break down your
query into something like that and try and see what it is you are doing
wrong. Our queries look almost the same except \
(a) I don't know how you are executing @CatInsert
(b) Your query is slightly more complex (try removing the insert and see if
the select works on its own like mine)
(c) I'm not sure of the exec value of @categoryID (though I can't imagine
it's any different than my @query)

On a side note, this is ugly and dangerous code (sql injection)...consider
alternatives, such as a UDF to convert @categoryId into a table datatype

Karl
 
N

nicholas

Thanks to all of you, but I changed my code completely.
It indeed wasn't a good way for doing this.
Everything works fine now...and I think it's safer too.

Again thank you,
Nic
 
E

Ed Gibbs

Nic,

If @categoryID is a parameter to your stored procedure, make sure
you're defining the parameter length. The procedure:

CREATE PROCEDURE nicholas
@shopID INT,
@categoryID VARCHAR
AS ...

will give @categoryID a length of 1 (see Books Online / Transact SQL
Reference / char and varchar). This explains the behavior where your
"1,25,78" and "124" parameters are acting like "1". As for nothing
happening with "25,78", my guess is it's being truncated to "2" and
you don't have a tbl_categories.categoryID of 2?

At any rate, you need to declare a comfy size for your @categoryID
parameter. Max is 8,000. I don't know if you pay any penalty for
declaring 8000, as Books Online claims that storage size is the actual
string size, not the declared string size (lurkers please correct if
I'm wrong; thanks!).

Sooo, try the following instead:

CREATE PROCEDURE nicholas
@shopID INT,
@categoryID VARCHAR(8000)
AS ...


I'll also give the usual warning about jamming a list into a string:
if you think you'll *ever* need more than 8K characters for the list
of category ID's, consider another approach.

Hope this helps,
Ed
 
E

Ed Gibbs

Nic,

I posted a reply earlier that seems to have gone south...

At any rate, I'm guessing the problem is that you declared @categoryID
as VARCHAR without a length specifier. When you do that, the length is
defaulted to 1 (see Books Online / Transact SQL Reference / char and
varchar). That would explain "1,25,78" and "124" acting like "1". As
for "25,78" doing nothing, perhaps you don't have a
tbl_categories.categoryid of "2"?

I don't know where @categoryID comes from, but if it's a parameter to
the SP you need to go from this:

CREATE PROCEDURE whatever @categoryID VARCHAR

to this:

CREATE PROCEDURE whatever @categoryID VARCHAR(8000)

The same default length of 1 also applies to DECLARE statements.

8000 is the max VARCHAR length, and as far as I can tell you don't pay
a penalty for declaring the max length (according to Books Online,
storage is actual size rather than declared size, but I could be
wrong; lurkers welcome to weigh in).

Finally, I'll give the standard warning about representing a list in a
string: if you think you'll *ever* need more than 8000 characters to
represent the list, consider a different approach.

Hope this helps,
Ed
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,755
Messages
2,569,534
Members
45,007
Latest member
obedient dusk

Latest Threads

Top