Make SQL Cursor code more efficient

[vikpaw]

Hi,

Any thoughts or views on this code please, it's not that it's particularly slow or anything, just thought there might be a better way than the cursor...?

Ignore the reference to Students and subsequent reference to Staff, the original db was called students and i've just dumped the staff tables in there too. It all works okay.

I figure there might be a better way to assign the the variables with a combined query rather than separate selects maybe ... ?

 

-- Write all database images (jpg) to file. --------- --------- --------- --------- --------- --------- ---------  
use Students
DECLARE CURSOR_ProductIds CURSOR FOR (SELECT [RECORD_NR] FROM [students].[dbo].[staff_card_info]
 WHERE Staff_card_info.UpdateTime > GETDATE()-0.04)

DECLARE @ProductId INT; 

OPEN CURSOR_ProductIds 

FETCH NEXT FROM CURSOR_ProductIds INTO @ProductId 
WHILE (@@FETCH_STATUS <> -1) 
BEGIN 
 DECLARE @ImageData varbinary(max); 
 SELECT @ImageData = (SELECT convert(varbinary(max), Photo, 1) FROM dbo.staff_card_info WHERE Record_Nr = @ProductId); 

 DECLARE @path nvarchar(1024); 
 SELECT @path = 'C:\photo'; 

 DECLARE @filename NVARCHAR(1024); 
 SELECT @filename = (SELECT Second_Name + Left(First_Name,1) AS Name FROM dbo.Staff_card_info WHERE RECORD_NR = @ProductId); 

 DECLARE @FullPathToOutputFile NVARCHAR(2048); 
 SELECT @FullPathToOutputFile = @path + '\' + @filename + '.jpg'; 

 DECLARE @ObjectToken INT 
 EXEC sp_OACreate 'ADODB.Stream', @ObjectToken OUTPUT; 
 EXEC sp_OASetProperty @ObjectToken, 'Type', 1; 
 EXEC sp_OAMethod @ObjectToken, 'Open'; 
 EXEC sp_OAMethod @ObjectToken, 'Write', NULL, @ImageData; 
 EXEC sp_OAMethod @ObjectToken, 'SaveToFile', NULL, @FullPathToOutputFile, 2; 
 EXEC sp_OAMethod @ObjectToken, 'Close'; 
 EXEC sp_OADestroy @ObjectToken; 

 PRINT @ProductId
 PRINT @filename	 
 PRINT ' '
 
 FETCH NEXT FROM CURSOR_ProductIds INTO @ProductId 
 
END 


CLOSE CURSOR_ProductIds 
DEALLOCATE CURSOR_ProductIds

[localzuk]

Personal preference for me has always been to split data from processing. So, the SQL server just serves up the records from simple SELECT requests, the client program does the processing, and then if records need changing, INSERT, UPDATE, DELETE are called. Keeps the complexity down, and prevents bottlenecks most of the time.

 

But that's personal preference as I say.

[ChrisMiles]

Personal preference for me has always been to split data from processing. So, the SQL server just serves up the records from simple SELECT requests, the client program does the processing, and then if records need changing, INSERT, UPDATE, DELETE are called. Keeps the complexity down, and prevents bottlenecks most of the time.

 

But that's personal preference as I say.

 

I agree with this, except that its personal preference, its not, it's good practice. The SQL service account shouldn't even have permissions to write those files to the disk in the first place.

 

I'm more than happy to supply vbscript, powershell or .net app to replace this process, but if you absolutely have to do it all on the SQL server, I'll have a look at it and get back to you.

Edited November 27, 2012 by ChrisMiles

[ChrisMiles]

If it must be done in SQL, this is how I'd do it. You'd need to switch to a temporary table rather than a table variable if the binary data was very large, but as they're jpgs I'd imagine it wont be too bad.

 

USE Students

DECLARE  @studentData TABLE
(
ImageID INT IDENTITY PRIMARY KEY,
ImagePath VARCHAR(MAX),
ImageData VARBINARY(MAX)
)
DECLARE @CurrentRow INT
DECLARE @RowCount INT
DECLARE @ImagePath VARCHAR(MAX)
DECLARE @ImageData VARBINARY(MAX)
DECLARE @ObjectToken INT 

INSERT INTO  @studentData (ImagePath, ImageData)
SELECT
'C:\photo\' + Second_Name + LEFT(First_Name, 1) + '.jpg',
CONVERT(VARBINARY(MAX), Photo, 1)
FROM dbo.staff_card_info

SELECT @CurrentRow = 1

SELECT @RowCount = COUNT(*)
FROM  @studentData

WHILE (@CurrentRow < @RowCount)
BEGIN

SELECT @ImagePath = ImagePath, @ImageData = ImageData
FROM  @studentData
WHERE (ImageID = @CurrentRow)

EXEC sp_OACreate 'ADODB.Stream', @ObjectToken OUTPUT; 
EXEC sp_OASetProperty @ObjectToken, 'Type', 1; 
EXEC sp_OAMethod @ObjectToken, 'Open'; 
EXEC sp_OAMethod @ObjectToken, 'Write', NULL, @ImageData; 
EXEC sp_OAMethod @ObjectToken, 'SaveToFile', NULL, @ImagePath, 2; 
EXEC sp_OAMethod @ObjectToken, 'Close'; 
EXEC sp_OADestroy @ObjectToken; 

PRINT  @ProductId
PRINT  @filename	 
PRINT ' '

SELECT @CurrentRow = @CurrentRow + 1

END

 

Note: Don't have a database to test so you may have to check for bugs.

Edited November 27, 2012 by ChrisMiles

[ChrisMiles]

If you want to use a cursor, this is more efficient:

 

USE Students

DECLARE ImagesCursor CURSOR FAST_FORWARD FOR
SELECT
	'c:\photo\' + Second_Name + Left(First_Name,1) + '.jpg' as ImagePath,
	CONVERT(VARBINARY(MAX), Photo, 1) as ImageData
FROM dbo.Staff_card_info
WHERE UpdateTime > GETDATE()-0.04

DECLARE @ImagePath NVARCHAR(MAX); 
DECLARE @ImageData VARBINARY(MAX); 

OPEN ImagesCursor 

FETCH NEXT FROM ImagesCursor INTO @ImagePath, @ImageData

WHILE (@@FETCH_STATUS <> -1) 
BEGIN 
DECLARE @ObjectToken INT 
EXEC sp_OACreate 'ADODB.Stream', @ObjectToken OUTPUT; 
EXEC sp_OASetProperty @ObjectToken, 'Type', 1; 
EXEC sp_OAMethod @ObjectToken, 'Open'; 
EXEC sp_OAMethod @ObjectToken, 'Write', NULL, @ImageData; 
EXEC sp_OAMethod @ObjectToken, 'SaveToFile', NULL, @ImagePath, 2; 
EXEC sp_OAMethod @ObjectToken, 'Close'; 
EXEC sp_OADestroy @ObjectToken; 

PRINT  @ProductId
PRINT  @filename	 
PRINT ' '

FETCH NEXT FROM ImagesCursor INTO @ImagePath, @ImageData
END 

CLOSE ImagesCursor 
DEALLOCATE ImagesCursor

Edited November 27, 2012 by ChrisMiles

[vikpaw]

You're right, it's just i didn't have the time and so went straight in for the data. Initially, i had a sixth former come up with some nifty php pages to look at and analyse the data, but as the photo was in a proprietary format it wasn't happy loading that, or doing anything with it. I started looking for a way to export them directly and came across that script. It turns out with a tweak on the client side software it allows export to jpeg, and so works. One thing i did have to change was a setting in SQL for those OA_sp.... procedures to work, which are turned off by default.

 

With that client tweak the php page could have worked, but it was causing a problem getting all the data out for a jpeg, and only showing the first few lines, and now the script kiddie's gone back to uni'!

 

An app would be good, but as it's only something i do, on an internal server it's no biggie if it's a script. I'll take a look at the SQL at any rate if it's better practise. Like i said, it wasn't that it was slow, i just saw in the other thread about using the Execution Plan to see the cost and realised it wasn't that efficient.

 

I'll be interested to see how the new script analyses for cost.