Operation is not allowed...

[James Baker]

I'm getting an error: "Operation is not allowed when the object is closed."
on the lines of code below. I can't imagine why the object would be closed
where it says it will be, I've just opened it on the line before. This is
all within a larger loop, but the loop seems to be working fine and other
insert statements within it seem ok. Any suggestions?

Dim cmsRS, cmsSQL
Dim insRS, insSQL

Set cmsRS = Server.CreateObject("ADODB.Recordset")
Set insRS = Server.CreateObject("ADODB.Recordset")

cmsSQL = "SELECT ClientsFileNumber FROM TblOrder WHERE FileNumber = '" &
Request.Form("ref" & intR) & "' AND ClientCode = 'CMSNY'"

cmsRS.Open cmsSQL, "DSN=kasecure1;uid=sa;pwd=pcbs"

If NOT cmsRS.EOF Then
insSQL = "INSERT INTO CMS (FileNumber, StatusDate, StatusTime,
StatusComment) VALUES('" & Request.Form("ref" & intR) & "', '" &
Request.Form("sd" & intR & "_" & intsc & "', '" & adstime & "', 'Inspection
Date has been set for " & isd & "')"

' / Begin Problem
insRS.Open insSQL, "DSN=kasecure1;uid=sa;pwd=pcbs"
insRS.Close ' Errors on this line
' / End Problem
End If

cmsRS.Close
Set cmsRS = Nothing
Set insRS = Nothing

 

[Jeff Dillon]

try passing in a valid connection object to the recordset open, instead of a
connection string.

Jeff

 

[Aaron [SQL Server MVP]]

OK.

(a) stop using a DSN (http://www.aspfaq.com/2126).

(b) stop using ADODB.Recordset (http://www.aspfaq.com/2191), especially for
AFFECTING data.

(c) don't just arbitrarily insert user input into a SQL string... search
google groups for "SQL Injection" to understand why.

(d) use a stored procedure, if this is SQL Server
(http://www.aspfaq.com/2201).

(e) tell us what database and version you are using, so we don't have to
guess (http://www.aspfaq.com/5009).

(f) please give us complete specs, including DDL, sample data, desired
results (http://www.aspfaq.com/5006).

(g) don't separate date and time... what is the purpose of that? Are you
storing date and time values in VARCHAR columns?

Why don't you try an approach like this instead...

Stored procedure:

CREATE PROCEDURE dbo.InsertFile
@fn VARCHAR(12),
@dt VARCHAR(12),
@tm VARCHAR(12),
@comment VARCHAR(255)
AS
BEGIN
SET NOCOUNT ON
IF EXISTS
(
SELECT 1
FROM TblOrder
WHERE FileNumber = @fn
AND ClientCode = 'CMSNY'
)
INSERT CMS
(
FileNumber,
StatusDate,
StatusTime,
StatusComment
)
SELECT
@fn,
@dt,
@tm,
@comment
END
GO

ASP code:

fn = Replace(Request.Form("ref" & intR), "'", "''")
dt = Replace(Request.Form("sd" & intR & "_" & intsc), "'", "''")
set conn = CreateObject("ADODB.Connection")
conn.open "< use a real connection string here >"
sql = "EXEC dbo.InsertFile " & _
"@fn='" & fn & "'," & _
"@dt='" & dt & "'," & _
"@tm='" & adstime & "'," & _
"@comment='Inspection Date has been set for " & isd & "'"
conn.execute sql,,129
conn.close: set conn = nothing

 

[Bob Barrows [MVP]]

I'm getting an error: "Operation is not allowed when the object is
closed." on the lines of code below. I can't imagine why the object
would be closed where it says it will be, I've just opened it on the
line before. This is all within a larger loop, but the loop seems to
be working fine and other insert statements within it seem ok. Any
suggestions?

Dim cmsRS, cmsSQL
Dim insRS, insSQL

Set cmsRS = Server.CreateObject("ADODB.Recordset")
Set insRS = Server.CreateObject("ADODB.Recordset")

Since you are issuing multiple commands to the database, you should be using
a single Connection object instead of opening a new connection for each
command:

Dim cn
Set cn = Server.CreateObject("ADODB.Connection")
cn.Open "DSN=kasecure1;uid=sa;pwd=pcbs"

BTW, it is a horrible idea to use sa for your applications. Create a sql
login with limited permissions and use that instead of sa.

Also, you should use OLEDB instead of the obsolete ODBC.

cn.Open "Provider=SQLOLEDB;" & _
"Data Source=your_server_name;" & _
"Initial Catalog=the_database;" & _
"User ID = xxxx;Password=xxxx"

cmsSQL = "SELECT ClientsFileNumber FROM TblOrder WHERE FileNumber =
'" & Request.Form("ref" & intR) & "' AND ClientCode = 'CMSNY'"

cmsRS.Open cmsSQL, cn,,,1 '1=adCmdText

If NOT cmsRS.EOF Then
insSQL = "INSERT INTO CMS (FileNumber, StatusDate, StatusTime,
StatusComment) VALUES('" & Request.Form("ref" & intR) & "', '" &
Request.Form("sd" & intR & "_" & intsc & "', '" & adstime & "',
'Inspection Date has been set for " & isd & "')"

Is there any chance at all that this query will return records? No? So why
create and open an expensive recordset?

' / Begin Problem
insRS.Open insSQL, "DSN=kasecure1;uid=sa;pwd=pcbs"
insRS.Close ' Errors on this line

Since there were no records returned from your query, of course you don't
have an open re cordset at this point. Get rid of that createobject
statement for the second recordset and use this to execute a query that does
not return records:

cn.Execute insSQL,,129 '129=adCmdText(1)+adExecuteNoRecords(128)

HTH,
Bob Barrows

 

[Aaron [SQL Server MVP]]

BTW, it is a horrible idea to use sa for your applications. Create a sql

login with limited permissions and use that instead of sa.

Good catch, I was pretty thorough but I missed that one (even though that
was the only clue that this was SQL Server).

 

[Bob Barrows [MVP]]

Good catch, I was pretty thorough but I missed that one (even though
that was the only clue that this was SQL Server).

Thanks. You're right. That was my clue that we were dealing with sql server
(although it could have been sybase, I guess)

Bob

 

[James Baker]

I appreciate all of your feedback. The main answer to most of your "WHY?!"
questions is politics. I'm dealing with some of the worst code ever
written, backed by some of the worst SQL 2000 database design I've ever
seen. The head programmer doesn't want anything done differently than the
way he does it, and for the most part, he does it the wrong way.

Some of his code is absolutely insane. He once wrote an insert statement
into a page with a typo, "0rder has been reviewed" instead of "Order has
been reviewed". Well, instead of going back, fixing the typo and fixing any
inserts that were related to it, he decided he'd just query based on 0rder
instead. So now, half the records have Order, half have 0rder and the
confusion is never ending. He never indents his code, everything is flush
left. I've complained about the SA thing since day 1, and he refuses to
create another user. Just had to vent.

Anyway, I'll try rewriting this with a stored procedure, even though my DB
knowledge is pretty slim other than the "relational" aspect. I'll slip in
the removal of the DSN as well and hope it goes unnoticed.

Do you believe this will solve my original problem? I don't know what
exactly was causing that.

Thanks much,
James

 

[Aaron [SQL Server MVP]]

Your original problem stemmed from using an ADODB.Recordset object to commit
an INSERT statement. Recordsets are for *retrieving* data, and even then an
explicit ADODB.Recordset is only advantageous in certain scenarios.

--
http://www.aspfaq.com/
(Reverse address to reply.)

 

[James Baker]

If the connection is open, can I execute two statements against it before I
close the connection? I'm opening the one recordset I'm using with it and
I'm also potentially executing the INSERT with it as well. Ultimately I
rewrote my code (pre-Stored Procedure) as:

Dim cmsRS, cmsSQL
Dim insSQL

Dim cn
Set cn = Server.CreateObject("ADODB.Connection")
cn.Open "Provider=SQLOLEDB;" & _
"Data Source=XXXXX;" & _
"Initial Catalog=XXXXX;" & _
"User ID = XXXXX;Password=XXXXX"

Set cmsRS = Server.CreateObject("ADODB.Recordset")

cmsSQL = "SELECT ClientsFileNumber FROM TblOrder WHERE FileNumber = '" &
filenumber & "' AND ClientCode = 'CMSNY'"
cmsRS.Open cmsSQL, cn, , , 1

If NOT cmsRS.EOF Then
insSQL = "INSERT INTO CMS (FileNumber, StatusDate, StatusTime,
StatusComment) VALUES('" & cmsRS("ClientsFileNumber") & "', '" & date() &
"', '" & adstime & "', 'Order has entered review process')"
cn.Execute insSQL,,129
End If

cmsRS.Close
Set cmsRS = Nothing
cn.Close

 

[Bob Barrows [MVP]]

If the connection is open, can I execute two statements against it
before I close the connection?

Absolutely. Did you have a problem doing it?

I'm opening the one recordset I'm
using with it and I'm also potentially executing the INSERT with it
as well. Ultimately I rewrote my code (pre-Stored Procedure) as:

Looks good.

Dim cmsRS, cmsSQL
Dim insSQL

Dim cn
Set cn = Server.CreateObject("ADODB.Connection")
cn.Open "Provider=SQLOLEDB;" & _
"Data Source=XXXXX;" & _
"Initial Catalog=XXXXX;" & _
"User ID = XXXXX;Password=XXXXX"

Set cmsRS = Server.CreateObject("ADODB.Recordset")

cmsSQL = "SELECT ClientsFileNumber FROM TblOrder WHERE FileNumber =
'" & filenumber & "' AND ClientCode = 'CMSNY'"

Can this return more than one record? If so, you can streamline things by
changing it to:
cmsSQL = "SELECT Count(ClientsFileNumber) FROM TblOrder WHERE " & _
"FileNumber = '" & filenumber & "' AND ClientCode = 'CMSNY'"

cmsRS.Open cmsSQL, cn, , , 1

And change this:

If NOT cmsRS.EOF Then

To:
If cmsRS(0) > 0 then 'if the first field of the recordset contains a value >
0

insSQL = "INSERT INTO CMS (FileNumber, StatusDate, StatusTime,
StatusComment) VALUES('" & cmsRS("ClientsFileNumber") & "', '" &
date() & "', '" & adstime & "', 'Order has entered review process')"
cn.Execute insSQL,,129
End If

cmsRS.Close
Set cmsRS = Nothing
cn.Close

Don't forget:
Set cn=nothing

Although some will argue against the need to do that given that you've
destroyed all the child objects first ...

Bob Barrows

 

[James Baker]

cmsSQL = "SELECT ClientsFileNumber FROM TblOrder WHERE FileNumber =

Can this return more than one record? If so, you can streamline things by
changing it to:
cmsSQL = "SELECT Count(ClientsFileNumber) FROM TblOrder WHERE " & _
"FileNumber = '" & filenumber & "' AND ClientCode = 'CMSNY'"

It can't return more than one record...it's basically a 0 or 1 thing.
Thanks for all of your help, I'll suggest we start handling things with this
approach ASAP. Is there somewhere I can get a list/better understanding of
what the adXXX constants are? i.e. adCmdText and adExecuteNoRecords. I'd
like to know which ones to use in which situations.

Thanks again!
Jim

 

[Bob Barrows [MVP]]

It can't return more than one record...it's basically a 0 or 1 thing.
Thanks for all of your help, I'll suggest we start handling things
with this approach ASAP. Is there somewhere I can get a list/better
understanding of what the adXXX constants are? i.e. adCmdText and
adExecuteNoRecords. I'd like to know which ones to use in which
situations.

Thanks again!
Jim

You could go to the documentation at http://msdn.microsoft.com/library.
Specifically, here:
http://msdn.microsoft.com/library/en-us/ado270/htm/mdmscadoenumerations.asp
The adCmdText constant is explained in the CommandTypeEnum link, and the
adExecuteNoRecords constant is described in the ExecuteOptionEnum.

You should read the sections about the recordset Open method to see the
different cursor and lock types available, as well as the section on the
execute method.

But, a book will probably help better. Although outdated, Bill Vaughn's "ADO
Examples and Best Practices" took me most of the way through my learning
curve. David Sceppa's "Programming ADO" brought me the rest of the way.

Bob Barrows

 

[Jeff Dillon]

Run SQL Profiler to verify the SQL you are executing. It's an easy tool to
use, and very effective.

Jeff

 

[James Baker]

If I have the following logic:

Create Connection

Open RecordSet w/Connection

Close RecordSet

Close Connection

Set them both = Nothing

What does closing the Recordset do for me? Does it close the connection,
empty the recordset, something else?

Thanks,
Jim

 

[Bob Barrows [MVP]]

If I have the following logic:

Create Connection

Open RecordSet w/Connection

Close RecordSet

Close Connection

Set them both = Nothing

What does closing the Recordset do for me?

It closes the recordset, which makes sure that the connection will be
allowed to close. In some cases, failure to close the recordset can prevent
the connection from closing.

Does it close the
connection,

No. Closing the connection closes the connection.

empty the recordset, something else?

If the connection is not allowed to close (due to a child object, such as a
recordset, still in the open state), then the automatic garbage collection
system will not be able to destroy te object, leading to a memory leak which
will eventually cause your server to crash (this is well-documented).
Expliciltly close and destroy your ADO objects in the proper order to avoid
this issue.

Bob Barrows

 

[James Baker]

Thanks for the response. I very much fear that pre-existing code isn't
written in this fashion and could be the source of the problems we've been
having. Is there a way to monitor this, aside from going into every page
and seeing if the code is written properly? Our site has a tendency to get
really slow after a while and it wouldn't surprise me if it's something to
do with this.

Is this an acceptable order?

Open Connection
Open Recordset
Close Recordset
Close Connection
Set Both = Nothing

Thanks again,
James

 

[Aaron [SQL Server MVP]]

Is this an acceptable order?

Open Connection
Open Recordset
Close Recordset
Close Connection
Set Both = Nothing

I usually do this:

set conn = createobject("ADODB.Connection")
conn.open "<connection string>"
set rs = conn.execute(sql)
rs.close(): set rs = nothing
conn.close(): set conn = nothing

There is no reason to wait until after the conn.close() to set rs = nothing;
in fact, I think that's a bit counter-intuitive.

 

[Bob Barrows [MVP]]

Thanks for the response. I very much fear that pre-existing code
isn't written in this fashion and could be the source of the problems
we've been having. Is there a way to monitor this, aside from going
into every page and seeing if the code is written properly? Our site
has a tendency to get really slow after a while and it wouldn't
surprise me if it's something to do with this.

I know of no way to monitor this, other than to periodically check your sql
server for inactive connections. The problem is, findig them this way is
"after the fact". Killing them in the sql server will not cause them to be
destroyed in your web server since they've already been "marked" for
destruction by the garbage collector.

Is this an acceptable order?

Open Connection
Open Recordset
Close Recordset
Close Connection
Set Both = Nothing

What Aaron said.

Bob Barrows