0

Description of the application/processing

I'm maintaining some old code in Delphi XE. The application runs a query on a database on a remote server, and then creates a report on the PC. We've had a bug report that some data in one of the fields is missing. The field in question contains a huge amount of data (a few thousand characters) collected from the server. I have no control over what data is collected and stored in this field. I discovered that the error was being caused by nulls and other control characters (line feeds etc.) embedded in this data. When the TADOQuery's SaveToFile method was called, the nulls and other control characters were causing any data later on in the field to be ignored (the SaveToFile basically got to the control characters and stopped reading the record).

The solution to the bug was to pull the field in question from the result set, filter out any control characters and save this to a temporary file (I couldn't put it back into the resultset at this point as it was in a read-only state). The data is saved as XML so I also have to make some of the characters XML friendly (as you'll see in the code snippet). Once the SaveToFile method had been called, I'd read the corrected data from the temporary file and write it back into the saved report. It's long-winded but it works.

The problem

I now have a new problem. I notice that on random occasions, a single record would be missing at the end of the file (the field in question would be blank). I've spent ages digging into the issue and it comes down to the following code (which is used to extract the data from the field before the SaveToFile is called). The code is skipping a single line. Not every time - around one in every two or three query runs. I know this for a fact because I added a counter to the loop below while debugging and compared the number of times the loop was executed against the record count, and it came back one less when the error occurred (resulting in a blank field) and equal when all data appeared in the report. No exceptions are caught, so it isn't falling over on something - it's literally skipping a record.

assignfile(f, tempFilename);
rewrite(f);
adoqry.first;
repeat  
    try
        bytes := adoqry.fieldByName(fld).AsBytes;
        tmp := '';
        for i := 0 to length(bytes) - 1 do
            if bytes[i] < 32 then    // strip any control characters (nulls, line feeds etc.) from the string
                 tmp := tmp + ' '
            else
            begin
                 case char(bytes[i]) of
                      '&': tmp := tmp + '&amp;';
                      '''': tmp := tmp + '&apos;';
                      '"': tmp := tmp + '&quot;';
                      '>': tmp := tmp + '&gt;';
                      '<': tmp := tmp + '&lt;';
                 else
                      tmp := tmp + char(bytes[i]);
                 end;
            end;
        // when debugging, inc counter here to prove that the loop has been executed
        writeln(f, UTF8String(tmp));
        setLength(bytes, 0);
    except on e: exception do
        writeln(f, '');
    end;
    adoqry.next;
until adoqry.eof;
closefile(f);

Question

Is there any reason why the above code would skip a record (i.e. only execute the loop n - 1 times, where n is the record count)? Is there something that would cause the "adoquery.next" call to skip a record?

Edit to clarify issues raised in comments

Data is missing from the report. It's always a single record that isn't being processed. I have over 20,000 records in the report and the missing record is somewhere in the middle, but it's hard to narrow it down with there being so much data. As a single record is being skipped, everything after that record shifts up a record (meaning that a lot of the report is then wrong), with the final record containing a blank field.

Jeedee
  • 548
  • 5
  • 24
  • 4
    This is not the cause of your problem, but you should never use a repeat loop to iterate a dataset. Use "while not DataSet.Eof do " instead. That way, your code won't go wrong if the dataset happens to be empty. – MartynA Jun 23 '15 at 18:06
  • Did you try simplify your code? Something like `qry.First; while not qry.Eof do begin Writeln(f, qry.Fields[0].AsString); qry.Next; end;`? If this simplest code does not reproduces your problem then the problem is somewhere else. – Abelisto Jun 23 '15 at 18:15
  • Btw, when you call .SaveToFile, are you using passing pfADTG or pfXML as the TPersistfile parameter? And do you get the same problem if you use the other one? Just seems unlikely to me that as widely used a routine as SaveToFile would be incorrectly escaping the data whenit writes it tp the file. – MartynA Jun 23 '15 at 18:18
  • 1
    You say "(the field in question would be blank)". Is it blank or missing. You write out a blank line if you exception, and you put your counter inside the try block. If you have the correct number of lines in the temp file but your counter was wrong then you must have hit an exception. I have had cases where I hit an exception where the IDE was not breaking because of my options. I thought I was not hitting the except block when I really was. Try moving the counter right before adoqry.net and see what numbers you get. – Mark Elder Jun 23 '15 at 18:21
  • If I were debugging this I would: Change the exception handling to write out something other than blank, preferably e.message; Add a line before the AsBytes, writeln ( F, 'About to read a record:' ); After the AsBytes, add writeln ( F, 'Read ', length(bytes), ' bytes' ); This would make it easier to distinguish errors from blank values. – David Dubois Jun 23 '15 at 19:09
  • @MartynA I'm saving the file as pfXML as it makes processing it easier. I'm pretty much stuck with doing things that way as there's a lot of processing elsewhere in the application that depends on the report data being in XML format. – Jeedee Jun 24 '15 at 10:49
  • @Abelisto Yes, I've simplified the code while debugging (literally looping through the recordset and printing a count instead of doing anything with the data). I get the same issue. There's one less record saved than there should be. – Jeedee Jun 24 '15 at 10:51
  • @MarkElder Sorry for being vague. The field is missing. It isn't throwing/catching an exception (verified by me writing the exception message in the catch block instead of a blank line - this never appears in the report). I have one less line than there are records in the temp file. The problem is that there are over 20,000 records and the missing data is somewhere in the middle. This means that everything after the missing record is out by a record, with the last record of the report containing no data for that field. There's too much data in the report to figure out what's missing. – Jeedee Jun 24 '15 at 10:56
  • @DavidDubois As I've just mentioned in the comment above, there are no errors in the report (i.e. no exceptions being thrown/caught). I've changed the exception handling to write the exception message and it doesn't appear in the report, verifying this. The loop is skipping a single record, which is missing from somewhere in the middle of the report. – Jeedee Jun 24 '15 at 10:58
  • 1
    are you using threads? – whosrdaddy Jun 24 '15 at 11:10
  • @whosrdaddy Yes - this processing is done in a thread. This is to allow the user to run multiple queries at once, rather than running them in sequence. In this case, this is the only query that is being run. – Jeedee Jun 24 '15 at 11:20
  • You need to tell this upfront, please edit your question and show ALL relevant code. Where does your TADOConnection is being created, from the main thread? – whosrdaddy Jun 24 '15 at 11:26
  • @whosrdaddy Everything is being created in, and owned by, the thread that's running the query. Everything is thread safe. – Jeedee Jun 24 '15 at 13:55
  • That is a bold statement, the reason why I asked you about threads is because this sort of problem is typically because of threads. Anyway you are not telling the whole story in your question and you should add all relevant code because I don't see any problems in the code you showed us... – whosrdaddy Jun 24 '15 at 14:03
  • @whosrdaddy My bold statement wasn't intended to come across as such - the thread creates everything that it needs and has been working without issue for a number of years (aside from truncating one of the fields when SaveToFile is called, which prompted this new piece of processing). The new processing is where the problem is. I seem to have stumbled across a solution, prompted by you urging me to look at the rest of the code in the thread. I'm creating an answer to explain. Thanks for your help. – Jeedee Jun 24 '15 at 14:20

1 Answers1

0

I seem to have stumbled across a solution to this problem. I'm still testing, but it seems to have fixed things at the moment.

Before I get into that, I tracked down the record that was being skipped. There are a number of records with exactly the same data in them. I deleted the record that was being skipped to see what would happen and the processing skipped a different record (that contained the same data) instead. As before, it was only skipping it intermittently (once in every two or three query runs).

Onto the "fix"...

I thought that I'd try to make a few tweaks to how the TADOQuery was configured to see if that had any effect. For years, it has been running fine with the following option configured:

adoqry.executeOptions := [eoAsyncFetch];

I commented this line out and suddenly the processing works fine. I've run several queries and they all come back with all rows. Nothing is being skipped. I'll continue to test but this seems (at the moment) to have fixed things.

Jeedee
  • 548
  • 5
  • 24
  • Mmm yeah, That option makes absolutely no sense if you are using threads, if that is active you need to wait for the `OnFetchComplete` event, only then you know all data has been fetched... – whosrdaddy Jun 24 '15 at 14:25
  • @whosrdaddy Yep, indeed. I dug around in the config after being prompted by your comment and came across that. Must have been in there since before we moved the query running into threads and overlooked. – Jeedee Jun 24 '15 at 14:48