April 15, 2009 at 10:43 am
Hello,
I've inherited a cursor that is taking a long time. This is run once an hour. It usually finds about 500 records to insert. The problem is that it is querying very big tables. Can anyone give some advice on changing this query from a cursor?
Thank you.
alter PROCEDURE ce_sp_RefreshTables
as
Declare @dbid int, @CompanyName nvarchar(255), @CurrentTable char(7), @sql varchar(5000), @q nvarchar(4), @GPModDate AS DATETIME
Set @q = ''''
Set @CurrentTable = 'GL20000'
Set @GPModDate = GETDATE()
Declare CompanyCursor cursor for
select interid from DYNAMICS..SY01500 where UDCOSTR2 = 'Active' and interid <> 'Audit' order by interid
open CompanyCursor
fetch next from CompanyCursor into @CompanyName
While @@fetch_status = 0
Begin
InsertLoop:
Set @sql = 'USE CE_CUSTOM;
Insert into GL_Transactions
(
SOURCEINTERID,
SOURCETBL,
DEX_ROW_ID,
SOURCDOC,
OPENYEAR,
OPENMTH,
OPENDAY,
ACTINDX,
ACCTTYPE,
ACTNUMBR_1,
ACTNUMBR_2,
ACTNUMBR_3,
ACTNUMBR_4,
ACTNUMBR_5)
Select ' + @q + rtrim(@CompanyName) + @q + ',' +
@q + @CurrentTable + @q + ',' +
'A.DEX_ROW_ID,
A.SOURCDOC,
DATEPART(yyyy, A.TRXDATE),
DATEPART(mm, A.TRXDATE),
DATEPART(dd, A.TRXDATE),
B.ACTINDX,
B.ACCTTYPE,
B.ACTNUMBR_1,
B.ACTNUMBR_2,
B.ACTNUMBR_3,
B.ACTNUMBR_4,
B.ACTNUMBR_5' +
'From ' + rtrim(@CompanyName) + '..' + @CurrentTable + ' A ' +
'Join ' + rtrim(@CompanyName) + '..GL00100 B ON A.ACTINDX = B.ACTINDX ' +
'Where A.TRXDATE > = ' + @q + 'Oct 1, 2004' + @q + ' and ' +
'NOT EXISTS (Select * from CE_CUSTOM..GL_Transactions C Where C.DEX_ROW_ID = A.DEX_ROW_ID and C.SOURCEINTERID = ' +
@q + rtrim(@CompanyName) + @q + ' and C.SOURCETBL = ' + @q + @CurrentTable + @q + ') ' +
'Group By
A.DEX_ROW_ID,
B.ACTINDX,
A.SOURCDOC,
A.TRXDATE,
B.ACTNUMBR_1,
B.ACTNUMBR_2,
B.ACTNUMBR_3,
B.ACTNUMBR_4,
B.ACTNUMBR_5'
Exec (@sql)
If @CurrentTable = 'GL20000'
Begin
Set @CurrentTable = 'GL30000'
GoTo InsertLoop
End
Else
Begin
Set @CurrentTable = 'GL20000'
End
Fetch next from CompanyCursor into @CompanyName
End
Deallocate CompanyCursor
GO
Things will work out. Get back up, change some parameters and recode.
April 15, 2009 at 11:07 am
To make sure I'm understanding this, it looks like you have a tables in a number of different databases that you're querying, and inserting some data into a single master table. The purpose of the cursor is to step through each database. Is that correct?
Beyond that question, it would be very helpful if you could provide create scripts for the tables involved, and some sample data that can be inserted into them.
Multi-database dynamic queries are one of the places where cursors often do work reasonably well, but the query can probably be improved nonetheless.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
April 15, 2009 at 11:16 am
Yes, that is correct.
1) First it creates a result set of companies which could be 500 rows.
2) Then it looks at each company's database
3) It finds either the GL20000
4) If it finds rows, the inserts those rows
5) Then it loops back and finds the records using the GL30000 table
6) Then it goes to the next company row
The create statements for the tables are quite big. I have to see if I can post things like the create scripts and then create some sample data. I know I can't post actual data.
Thanks for helping. It may be that we actually need a cursor. I was always taught cursors were a bad thing.
Tony
Things will work out. Get back up, change some parameters and recode.
April 15, 2009 at 11:24 am
One thing you might try is changing the cursor to either "Static Forward_Only", or "Fast_Forward". You're not using it to update the base table, so that will often speed things up a bit.
But I'd be willing to bet the part that's taking all the time is the actual inserts.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
April 15, 2009 at 11:54 am
It may be, but I think the problem is scrolling throgh the resultset one at a time.
We get a company from the initial resultset, query the GL20000 table and insert rows, then query the GL30000 table and insert rows and then go to the next company.
This may be the right way, but it just seems that we shouldn't have to go one row at a time.
Tony
Things will work out. Get back up, change some parameters and recode.
April 15, 2009 at 12:10 pm
One thing you could try is building a single dynamic string, like this:
create table #T (
ID int identity primary key,
Name varchar(100));
insert into #T (Name)
select 'Ink Inc' union all
select 'ACME Co';
declare @SQL varchar(max);
select @SQL = coalesce(
@SQL + ';' + 'select * from #T where id = ' + cast(id as varchar(10)),
'select * from #T where id = ' + cast(id as varchar(10)))
from #T;
print @SQL;
Result:
select * from #T where id = 1;select * from #T where id = 2
You can build complex queries that way from data in tables, and then instead of "print", used "exec()". That's going to be faster than a cursor on that part.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
April 15, 2009 at 12:19 pm
I've never heard of that before. Creating a table with dynamic strings? Let me try to create a table and see if I understand what you are saying.
Thanks again.
Things will work out. Get back up, change some parameters and recode.
April 15, 2009 at 12:26 pm
You don't create the table with dynamic strings. You build the string with data from a table.
I just created the temp table and put some data in it for a simple proof-of-concept demo.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
April 15, 2009 at 12:36 pm
Your dynamic concat would look more like this:
create table #Tables (
TName varchar(100) primary key);
insert into #Tables (TName)
select 'GL20000' union all
select 'GL30000';
declare @SQL varchar(max), @Q char(4);
select @Q = '''';
select @SQL = coalesce(
@SQL +
'begin transaction;
Insert into CE_CUSTOM..GL_Transactions
(
SOURCEINTERID,
SOURCETBL,
DEX_ROW_ID,
SOURCDOC,
OPENYEAR,
OPENMTH,
OPENDAY,
ACTINDX,
ACCTTYPE,
ACTNUMBR_1,
ACTNUMBR_2,
ACTNUMBR_3,
ACTNUMBR_4,
ACTNUMBR_5)
Select ' + @q + rtrim(interid) + @q + ',' +
@q + TName + @q + ',' +
'A.DEX_ROW_ID,
A.SOURCDOC,
DATEPART(yyyy, A.TRXDATE),
DATEPART(mm, A.TRXDATE),
DATEPART(dd, A.TRXDATE),
B.ACTINDX,
B.ACCTTYPE,
B.ACTNUMBR_1,
B.ACTNUMBR_2,
B.ACTNUMBR_3,
B.ACTNUMBR_4,
B.ACTNUMBR_5' +
'From ' + rtrim(interid) + '..' + TName + ' A ' +
'Join ' + rtrim(interid) + '..GL00100 B ON A.ACTINDX = B.ACTINDX ' +
'Where A.TRXDATE > = ' + @q + 'Oct 1, 2004' + @q + ' and ' +
'NOT EXISTS (Select * from CE_CUSTOM..GL_Transactions C Where C.DEX_ROW_ID = A.DEX_ROW_ID and C.SOURCEINTERID = ' +
@q + rtrim(interid) + @q + ' and C.SOURCETBL = ' + @q + TName + @q + ') ' +
'Group By
A.DEX_ROW_ID,
B.ACTINDX,
A.SOURCDOC,
A.TRXDATE,
B.ACTNUMBR_1,
B.ACTNUMBR_2,
B.ACTNUMBR_3,
B.ACTNUMBR_4,
B.ACTNUMBR_5; while @@trancount> 0 commit;',
--
'begin transaction;
Insert into CE_CUSTOM..GL_Transactions
(
SOURCEINTERID,
SOURCETBL,
DEX_ROW_ID,
SOURCDOC,
OPENYEAR,
OPENMTH,
OPENDAY,
ACTINDX,
ACCTTYPE,
ACTNUMBR_1,
ACTNUMBR_2,
ACTNUMBR_3,
ACTNUMBR_4,
ACTNUMBR_5)
Select ' + @q + rtrim(interid) + @q + ',' +
@q + TName + @q + ',' +
'A.DEX_ROW_ID,
A.SOURCDOC,
DATEPART(yyyy, A.TRXDATE),
DATEPART(mm, A.TRXDATE),
DATEPART(dd, A.TRXDATE),
B.ACTINDX,
B.ACCTTYPE,
B.ACTNUMBR_1,
B.ACTNUMBR_2,
B.ACTNUMBR_3,
B.ACTNUMBR_4,
B.ACTNUMBR_5' +
'From ' + rtrim(interid) + '..' + TName + ' A ' +
'Join ' + rtrim(interid) + '..GL00100 B ON A.ACTINDX = B.ACTINDX ' +
'Where A.TRXDATE > = ' + @q + 'Oct 1, 2004' + @q + ' and ' +
'NOT EXISTS (Select * from CE_CUSTOM..GL_Transactions C Where C.DEX_ROW_ID = A.DEX_ROW_ID and C.SOURCEINTERID = ' +
@q + rtrim(interid) + @q + ' and C.SOURCETBL = ' + @q + TName + @q + ') ' +
'Group By
A.DEX_ROW_ID,
B.ACTINDX,
A.SOURCDOC,
A.TRXDATE,
B.ACTNUMBR_1,
B.ACTNUMBR_2,
B.ACTNUMBR_3,
B.ACTNUMBR_4,
B.ACTNUMBR_5; while @@trancount> 0 commit;')
from DYNAMICS..SY01500
cross join #Tables
where UDCOSTR2 = 'Active' and interid <> 'Audit';
print @SQL;
--exec (@SQL);
Test it with the print command, then run it on a test server with the exec command.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
April 15, 2009 at 1:10 pm
Wow!!
GSquared, this is whole new territory for me. I am going to do some tests and try this example.
Thanks so much for your help, ideas and getting me going in the right direction.
Tony
Things will work out. Get back up, change some parameters and recode.
April 15, 2009 at 1:23 pm
WebTechie38 (4/15/2009)
Wow!!GSquared, this is whole new territory for me. I am going to do some tests and try this example.
Thanks so much for your help, ideas and getting me going in the right direction.
Tony
Well, till it's tested fully, we don't know if it's the right direction or not, but you're welcome anyway. 🙂
Let me know how your tests turn out on it, and if there's anything else that we can help with on this.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
April 16, 2009 at 7:54 am
So I am running into some difficulties when running the script.
1) I first have to get a result set of the available companies. This is from the beginning of the cursor
Declare CompanyCursor cursor for
select interid from DYNAMICS..SY01500 where UDCOSTR2 = 'Active' and interid 'Audit' order by interid
interid = CompanyName
This also equates to the database names.
Select ' + @q + rtrim(@CompanyName) + @q + ',' +
@q + @CurrentTable + @q + ',' +
'A.DEX_ROW_ID,
2) As the cursor iterates through the databases, it queries the two tables from each database
@Currenttable = 'GL20000' or @CurrentTable = 'GL30000'
3) How would you rewrite the part that is iterating through the database names (companynames) to initialize the variable @CompanyName. If I had one database to query on with a static name, your example would be perfect. It definitely helps with @Currentable.
'From ' + rtrim(@CompanyName) + '..' + @CurrentTable + ' A ' +
'Join ' + rtrim(@CompanyName) + '..GL00100 B ON A.ACTINDX = B.ACTINDX ' +
'Where A.TRXDATE > = ' + @q + 'Oct 1, 2004' + @q + ' and ' +
'NOT EXISTS (Select * from CE_CUSTOM..GL_Transactions C Where C.DEX_ROW_ID = A.DEX_ROW_ID and C.SOURCEINTERID = ' +
@q + rtrim(@CompanyName) + @q + ' and C.SOURCETBL = ' + @q + @CurrentTable + @q + ') ' +
Thanks again.
Tony
Things will work out. Get back up, change some parameters and recode.
April 16, 2009 at 7:58 am
Where it has the "print @SQL" in it, what output does it give you? The string should be built with the various database names already in it.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
April 16, 2009 at 1:25 pm
The message says
(2 row(s) affected)
begin transaction;
Insert into CE_CUSTOM..GL_Transactions
(
SOURCEINTERID,
SOURCETBL,
DEX_ROW_ID,
SOURCDOC,
OPENYEAR,
OPENMTH,
OPENDAY,
ACTINDX,
ACCTTYPE,
ACTNUMBR_1,
ACTNUMBR_2,
ACTNUMBR_3,
ACTNUMBR_4,
ACTNUMBR_5)
Select '
(2 row(s) affected)
The two rows affected are:
GL20000
GL30000
Those are the two tables that are queried for each database. But how would I iterate through all the databases?
Things will work out. Get back up, change some parameters and recode.
April 16, 2009 at 1:33 pm
You won't have to iterate through them. Run the test case I gave you a few posts back, you'll see how it works.
What this kind of script function does is build a string that has the SQL commands, one after the other, for all of the rows in the table. It then executes them as a batch.
Try it on a smaller string, like the sample I posted, and you'll see what I'm talking about.
- Gus "GSquared", RSVP, OODA, MAP, NMVP, FAQ, SAT, SQL, DNA, RNA, UOI, IOU, AM, PM, AD, BC, BCE, USA, UN, CF, ROFL, LOL, ETC
Property of The Thread
"Nobody knows the age of the human race, but everyone agrees it's old enough to know better." - Anon
Viewing 15 posts - 1 through 15 (of 17 total)
You must be logged in to reply to this topic. Login to reply