this code takes FOREVER! HELP

[kkozar]

the background
this code is called by a query in Access that runs for approx 35,000 data points. It takes 2-3 hours to run.

Anyway to make it faster???

Here's the calling query:
UPDATE tbl_TMS_Orders_VendorNumber SET tbl_TMS_Orders_VendorNumber.TMSAcknowledgementDays = IIf(getbusinessdaysdiff(nz([tbl_TMS_Orders_VendorNumber].[ORD ISSUED DATE],#1/1/1900#),nz(([tbl_TMS_Orders_VendorNumber].[ORD ORIGIN PLANNED DEPART (S)DATE]-3),#1/1/1900#))<>-100000,getbusinessdaysdiff(nz([tbl_TMS_Orders_VendorNumber].[ORD ISSUED DATE],#1/1/1900#),nz(([tbl_TMS_Orders_VendorNumber].[ORD ORIGIN PLANNED DEPART (S)DATE]-3),#1/1/1900#)),Null);



Here's the function code:
Public Function GetBusinessDaysDiff(d1 As Date, d2 As Date) As Long

Dim rs As Recordset
Dim sql As String
Dim retval As Long


If DateDiff("D", d1, "1/1/1900") = 0 Or DateDiff("D", d2, "1/1/1900") = 0 Then

retval = -100000

Else

sql = "select count(dateid) as result from tblFiscalCalendar where BusinessDay=true and " & _
"dateid <> #" & d1 & "# and dateid between #" & d1 & "# and #" & d2 & "#;"

Set rs = CurrentDb.OpenRecordset(sql)

retval = rs![result]

If d2 > d1 Then
retval = retval * -1
End If

End If

[si_the_geek]

To start with there are two things in GetBusinessDaysDiff that are less than ideal. The first is the fact that you use Strings in this line rather than Dates:
If DateDiff("D", d1, "1/1/1900") = 0 Or DateDiff("D", d2, "1/1/1900") = 0 Then
...it should be like this:
If DateDiff("D", d1, #1/1/1900#) = 0 Or DateDiff("D", d2, #1/1/1900#) = 0 ThenUsing Strings is slower (because they need to be converted to Dates), and likely to cause bugs (because the conversion is complex and relies on external factors).


The next issue is a bit similar, albeit from the other direction. In this line, you are converting Dates to Strings:
"dateid <> #" & d1 & "# and dateid between #" & d1 & "# and #" & d2 & "#;"As above, the conversion relies on external factors - and is therefore likely to cause bugs.

When you put date values in to SQL statements, you should force the formatting to be a kind that the database will interpret as you intended. With what you have at the moment, today (August 26th) will be interpreted correctly, but in a weeks time (Sept 2nd) the date will be interpreted as Feb 9th by a large proportion of computers in the world - and yours too if you make a simple change.

For an explanation of how to do it properly, see the article How do I use values (numbers, strings, dates) in SQL statements? from our Database Development FAQs/Tutorials (at the top of the Database Development forum)



Now for the sake of easier reading, here is your Update statement with line breaks, and removal of the unnecessary use of "tbl_TMS_Orders_VendorNumber." before each field:
UPDATE tbl_TMS_Orders_VendorNumber
SET TMSAcknowledgementDays =
IIf(getbusinessdaysdiff( nz([ORD ISSUED DATE], #1/1/1900#),
nz(([ORD ORIGIN PLANNED DEPART (S)DATE]-3),#1/1/1900#)
)<>-100000,
getbusinessdaysdiff( nz([ORD ISSUED DATE], #1/1/1900#),
nz(([ORD ORIGIN PLANNED DEPART (S)DATE]-3),#1/1/1900#)),
Null);The first thing to note is that IIf always evaluates all parameters, so it is theoretically possible that getbusinessdaysdiff will be called twice for each record. I'm definitely not sure about this, but it might be noticeably faster to run two update statements, one to set the field to the result of getbusinessdaysdiff, and the other to change any that are -100000 to Null.

However, I wouldn't recommend doing that, because you already know the circumstances where that value will occur (where either date is Null), so what you should do is run one Update statement for records that meet that situation, and one for the other records, ie:
UPDATE tbl_TMS_Orders_VendorNumber
SET TMSAcknowledgementDays = Null
WHERE [ORD ISSUED DATE] Is Null
OR [ORD ORIGIN PLANNED DEPART (S)DATE] Is Null

UPDATE tbl_TMS_Orders_VendorNumber
SET TMSAcknowledgementDays = getbusinessdaysdiff([ORD ISSUED DATE],
[ORD ORIGIN PLANNED DEPART (S)DATE]-3)
WHERE [ORD ISSUED DATE] Is Not Null
AND [ORD ORIGIN PLANNED DEPART (S)DATE] Is Not Null


Just making these changes should give a noticeable speed boost, but there is still more to come by converting the getbusinessdaysdiff function to a sub-query. I've written enough for now tho, so we'll come back to that later!

[MarMan]

I just want to add that IIf is slow. It is a great function, but if you ever have a performance issue and are using IIf, it is best to rewrite the code. Even if you need 2 UPDATE statements.

[kkozar]

Thanks guys! FYI - I'm not a programmer, just a hack. our programmer left this in my hands and I'm trying to make it work (better)

so I've made the changes but I'm getting an error...

here's the first new query (Part 1 of the original action)
UPDATE tbl_TMS_Orders_VendorNumber SET TMSAcknowledgementDays = Null
WHERE [ORD ISSUED DATE] Is Null Or [ORD ORIGIN PLANNED DEPART (S)DATE] Is Null;


here's the second new query (Part 2 of the original action)
UPDATE tbl_TMS_Orders_VendorNumber SET TMSAcknowledgementDays = getbusinessdaysdiff([ORD ISSUED DATE],[ORD ORIGIN PLANNED DEPART (S)DATE])
WHERE [ORD ISSUED DATE] Is Not Null AND [ORD ORIGIN PLANNED DEPART (S)DATE] Is Not Null;


and here's the changes I made to the public function getbusinessdaysdiff
Public Function GetBusinessDaysDiff(d1 As Date, d2 As Date, dateid As Date) As Long

Dim rs As Recordset
Dim sql As String
Dim retval As Long


If DateDiff("D", d1, "1/1/1900") = 0 Or DateDiff("D", d2, "1/1/1900") = 0 Then

retval = -100000

Else

sql = "select count(dateid) as result from tblFiscalCalendar where BusinessDay=true and " & _
dateid & " <> #" & d1 & "# and " & dateid & "between #" & d1 & "# and #" & d2 & "#;"

Set rs = CurrentDb.OpenRecordset(sql)

retval = rs![result]

If d2 > d1 Then
retval = retval * -1
End If

End If

GetBusinessDaysDiff = retval
End Function

Once I run this the second query, Part 2 gives me an error saying all records can't be updated due to type conversion failure.

in tbl_TMS_Orders_VendorNumber:
TMSAcknowledgementDays is a Number
ORD ISSUED DATE is a Date
ORD ORIGIN PLANNED DEPART (S)DATE is a Date

[MarMan]

Try this: sql = "select count(dateid) as result from tblFiscalCalendar where BusinessDay=true and " & _
dateid & " <> #" & d1 & "# and " & dateid & "between #" & d1 & "# and #" & d2 & "#;"


I just copied it from your earlier post. dateid is a database field and needs to be in quotes otherwise VB tries to match it to a declaration (Dim, Public or Private) and can't find one, hence the error.

[si_the_geek]

That is because you altered the wrong thing, and you didn't make the right changes.

What you were supposed to change on that line is: & d1 & and: & d2 & as explained (with examples) in the article I linked to.


You were also supposed to change this line to what I showed before:
If DateDiff("D", d1, "1/1/1900") = 0 Or DateDiff("D", d2, "1/1/1900") = 0 Then

[kkozar]

si the geek,

Thanks for your help. I'm not kidding when I say I'm not a programmer. I'm lucky I can get into a SQL statement and follow it. I've used Access enough that I can build queries and tables, etc. but when it comes to Modules and SQL I'm very green. So please help me out and list exactly what I need to do. I read that link and I don't understand it. As for the "1/1/1900" change to #1/1/1900#, I did make the changes, but when Access errored out at the begining of the code I had to force close Access and thus lost changes.

One thing I am for sure is that the code took only 1 minute, vs 1 hour! So I'm definately interested in getting this thing nailed down!


So here's what I've got so....
Part 1 query
UPDATE tbl_TMS_Orders_VendorNumber SET TMSAcknowledgementDays = Null
WHERE [ORD ISSUED DATE] Is Null Or [ORD ORIGIN PLANNED DEPART (S)DATE] Is Null;
I think this SQL is working fine.

Part 2 query
UPDATE tbl_TMS_Orders_VendorNumber SET TMSAcknowledgementDays = getbusinessdaysdiff([ORD ISSUED DATE],[ORD ORIGIN PLANNED DEPART (S)DATE])
WHERE [ORD ISSUED DATE] Is Not Null AND [ORD ORIGIN PLANNED DEPART (S)DATE] Is Not Null;
Likewise I think this SQL is working fine.

public function
Public Function GetBusinessDaysDiff(d1 As Date, d2 As Date, dateid As Date) As Long

Dim rs As Recordset
Dim sql As String
Dim retval As Long


If DateDiff("D", d1, #1/1/1900#) = 0 Or DateDiff("D", d2, #1/1/1900#) = 0 Then

retval = -100000

Else

sql = "select count(dateid) as result from tblFiscalCalendar where BusinessDay=true and " & _
"dateid <> #" & d1 & "# and dateid between #" & d1 & "# and #" & d2 & "#;"

Set rs = CurrentDb.OpenRecordset(sql)

retval = rs![result]

If d2 > d1 Then
retval = retval * -1
End If

End If

GetBusinessDaysDiff = retval
End Function

So the above is where things fall apart for me. I get the modification of "1/1/1900" to #1/1/1900#
the & d1 &, & d2 & i don't.

Thanks for your patience.

[si_the_geek]

In the example at the end of the FAQ article, Date is the equivalent of your d1 and d2 variables... that should be enough for you to work it out.

The reason for doing it is something that you should be able to understand based on the article, as the terms it uses (apart from one instance of String [text] and one of Date [date and/or time]) are definitely not specific to programming.

[kkozar]

Geek

thanks for making me feel like I'm back in gradeschool. I read the article, I don't understand it, I'm not a programmer, I'm looking for help. If you can help please correct the code for me. If not let this linger and perhaps someone else will jump in that is not above give a non-programmer some help.

[MarMan]

The code looks OK to me. si is just trying to help you help yourself. This forum is for people who WANT to be programmers, not for people to get charity work. "give a man a fish, feed him for a day, teach him to fish, feed him for a lifetime."

If the code takes awhile its probably because its doing a lot of work. does it have to do all that work? We don't know only you or someone you are connected to does. Perhaps if you defined the task in words it may help. not just us, but you too.

[jmahaffie]

I could be misreading this whole thing, but it seems like your code is unnecessarily complicated.

In your update statement, you're checking for a null value in either of those date fields by using the Nz() function, and if it's found, you're still running the function to return -100000. Why not skip running the funtion altogether if a null value is found?
It seems to me that this whole mess of code could be distilled into something like this:

for the update part:

If IsNull([ORD ISSUED DATE]) = False AND IsNull([ORD ORIGIN PLANNED DEPART (S)DATE]) = False Then
UPDATE tbl_TMS_Orders_VendorNumber SET TMSAcknowledgementDays = GetBusinessDaysDiff([ORD ISSUED DATE],[ORD ORIGIN PLANNED DEPART (S)DATE]-3)
End If



for the function:

'Purpose - find out the number of business days between two given dates
'Input - two dates
'Output - the number of days between the two
Public Function GetBusinessDaysDiff(d1 As Date, d2 As Date) As Long
Dim retval As Long

retval = DCount("dateid","tblFiscalCalendar","BusinessDay=True and "[dateid] <> #" & d1 & "# and [dateid] BETWEEN #" & d1 & "# and #" & d2 & "#;")
If d2 > d1 Then retval = retval * -1
GetBusinessDaysDiff = retval
End Function


I am wondering how often you're running this function? It's running this code for every record in the Order_VendorNumber table. I would think something like this would be run once maybe for all the records, and then on one record at a time after a record gets updated on your form or something.

[leinad31]

Depending on how you created your indexes, you might be better of counting non-business days and subtracting that from datediff.