[SOLVED] DBChart issues

[wp]

Doing my first database chart I came across the following issues:

- If a query casts results to date/time types the data seem to be returned as strings. Maybe this is not true for all database systems, but SQLite3 at least has this behavior. DBChartSource does not recognize these strings as valid floating point numbers. It would be helpful to have properties DateTimeX and DateTimeY next to FieldX and FieldY such that TDbChartSource.GetItem would be able to call <field>.AsDateTime instead of, or in addition to, <field>.AsFloat. Since Databases often use locale settings different from the system's settings it may be necessary also to have an event handler like OnGetChartDataItem(ADataset:TDataset; var Item:TChartDataItem) which could be used to do special conversions.

- The series draws NULL values as zeros. I think that's not always desired; in most of the cases I'd prefer to have breaks in the curve, i.e. NaN.

- In my current project TAChart crashes when the DbChartSource contains NULL values -- unfortunately I cannot reproduce this with a small demo. I'd propose to catch NULL values in the implementation of TDbChartSource.GetItem, something like this:

Code: [Select]

function TDbChartSource.GetItem(AIndex: Integer): PChartDataItem;
var
  ds: TDataSet;
  i: Integer;
  F: TField;
begin
  Result := @FCurItem;
  SetDataItemDefaults(FCurItem);
  if not FDataLink.Active then exit;

  Inc(AIndex); // RecNo is counted from 1
  ds := FDataLink.DataSet;
  if ds.IsUniDirectional then begin
    if ds.RecNo < AIndex then
      ds.First;
  end
  else begin
    if AIndex > ds.RecNo - AIndex then
      while (ds.RecNo > AIndex) and not ds.BOF do
        ds.Prior
    else
      ds.First;
  end;
  while (ds.RecNo < AIndex) and not ds.EOF do
    ds.Next;
  if ds.RecNo <> AIndex then begin
    // Either the requested item is out of range, or the dataset is filtered.
    FCurItem.X := SafeNaN;
    FCurItem.Y := SafeNaN;
    exit;
  end;
  if FieldX <> '' then begin
    // wp >>>
    F := ds.FieldByName(FieldX);
    if F.IsNull then
      FCurItem.X := SafeNaN
    else
      FCurItem.X := F.AsFloat;
    // << wp --- instead of ...
//    FCurItem.X := ds.FieldByName(FieldX).AsFloat
  end
  else
    FCurItem.X := ds.RecNo;
  if FYCount > 0 then begin
    // wp >>>
    F := ds.FieldByName(FFieldYList[0]);
    if F.IsNull then
      FCurItem.Y := SafeNaN
    else
      FCurItem.Y := F.AsFloat;
    // <<< wp   -- instead of ...
//    FCurItem.Y := ds.FieldByName(FFieldYList[0]).AsFloat;
    for i := 0 to High(FCurItem.YList) do begin
      // wp >>>
      F := ds.FieldByName(FFieldYList[i + 1]);
      if F.IsNull then
        FCurItem.YList[i] := SafeNaN
      else
        FCurItem.YList[i] := F.AsFloat;
      // <<< wp --- instead of ...
      //FCurItem.YList[i] := ds.FieldByName(FFieldYList[i + 1]).AsFloat;
    end;
  end;
  if FieldColor <> '' then begin
    // wp >>>
    F := ds.FieldByName(FieldColor);
    if not F.IsNull then
      FCurItem.Color := F.AsInteger;
    // <<< wp --- instead of ...
    // FCurItem.Color := ds.FieldByName(FieldColor).AsInteger;
  end;
  if FieldText <> '' then begin
    // wp >>>
    F := ds.FieldByName(FieldText);
    if not F.IsNull then
      FCurItem.Text := F.AsString;
    // <<< wp --- instead of...
    //FCurItem.Text := ds.FieldByName(FieldText).AsString;
  end;
end;

- If a HintTool is attached to the chart and the mouse is moved across the chart, the scroll bar of the DBGrid runs quickly up and down. The same happens when the chart is resized. I don't see the side effects, but maybe it would be required to call DisableControls/EnableControls of the dataset.

- Would it be possible to reposition the hint window above the mouse position? Now it is partly covered by the mouse pointer.

- If the HintTool does not use an OnHint event handler a huge, empty hint window is displayed.

- Talking about optimizations, TDbChartDataSource.GetItem calls TDataset.FieldByName quite often. Since GetItem is called a lot, it would be more efficient to buffer the fields as class variables and to avoid these methods (http://stackoverflow.com/questions/4765931/why-is-my-code-so-slow).

The attached project demonstrates these issues by means of a table created on the fly. The date/time issue is demonstrated when the lower radiogroup option is selected.

Please note that you'll need to have sqlite3.dll in the exe folder or in the search path, or adjust the variable SQLiteLibraryName in FormCreate to point to a valid sqlite3.dll.

[Ask]

It would be helpful to have properties DateTimeX and DateTimeY next to FieldX and FieldY such that TDbChartSource.GetItem would be able to call <field>.AsDateTime instead of, or in addition to, <field>.AsFloat

To be honest, I do not like this solution much.
It would be much better if the database would report the field types accurately,
and then I would be able to just check "F.DataType in [ftDate, ftTime, ftDateTime]"
Unfortunately, my quick test confirmed that at least SQLite is apparently incapable of that.
So I implemented your suggestion in r38025.
If you wish, you can explore whether SQLite dataset can be modified to better
determine field type. I can replace the option by an automatic check as soon
as that becomes viable.

The series draws NULL values as zeros. I think that's not always desired; in most of the cases I'd prefer to have breaks in the curve, i.e. NaN.

Yes, definitely. Fixed in r38023, will nominate for backport.

In my current project TAChart crashes when the DbChartSource contains NULL values.

I doubt the fix you proposed will change that (or, if it does, then
the bug is in either TDataset or SQLite driver).
For now, I have replaced only x/y field accesses with checks,
so less important ones are still done directly.
If you encounter another crash, please try to catch at least the exception message,
and preferably stack trace.

event handler like OnGetChartDataItem(ADataset:TDataset; var Item:TChartDataItem) which could be used to do special conversions

Implemented in r38037.
However, the usage is somewhat tricky -- see the comment in TDbChartSource.GetItem.
Suggestions for better API are welcome -- I have considered various approaches,
such as adding OnBefore/OnAfter/OnInstead(?) events and adding another option
similar to grid's DefaultDrawing, but they all seem too heavy and even more confusing.
If you have no better idea, I'll just write it up on the wiki and optimistically assume that users will read documentation

call DisableControls/EnableControls of the dataset.

This is the most complex issue. First, I did try to call DisableControls/EnableControls some time ago -- turns out they do not work in FPC 2.6 (http://bugs.freepascal.org/view.php?id=19887). As you can see, there is a workaround, but it is not perfect.
Second, the scrolling is caused be every access to the dataset, not only during the drawing. So to eliminate it, you have to:
1) Add "BeforeDraw" and "AfterDraw" calls to TBasicPointSeries.GetNearestPoint to avoid scrolling while searching for a data point near the mouse
2) Set UseDefaultHintText = false to avoid scrolling while retrieving data for the the default hint text
3) Call DisableControls/EnableControls from OnGetHintText handler
... and even then some flicker will remain due to the bug mentioned above.

As you can see, this is complicated, and, for the moment, I do not know of a good general solution. Suggestions are welcome.

Would it be possible to reposition the hint window above the mouse position? Now it is partly covered by the mouse pointer.

I have added OnHintPosition event in r38028, which should suffice for a simple case.
(Just put APoint.Y -= 20 in the handler).
More generally, the user should be given some declarative control over the hint position -- perhaps TChartMarks property similar to point labels.
Again, suggestions welcome.

If the HintTool does not use an OnHint event handler a huge, empty hint window is displayed.

Indeed. This is technically an LCL bug, I reported it as bugs.freepascal.org/view.php?id=22498.
Nevertheless, it is a good practice to check for empty hint even if just to
avoid useless system calls, so fixed in r38024.

TDbChartDataSource.GetItem calls TDataset.FieldByName quite often

True, but I suspect the effect is negligible compared to the other sources of inefficiency. In particular, unless the StackOverflow poster had a table with thousands of columns, I am quite convinced that his problems were caused by redrawing of controls, not calling FieldByName.
So I'll postpone this optimization until after the redrawing issue if resolved.

[wp]

Hey, how can it be that you are able to address all these issues so fast? I really appreciate your efforts.

you can explore whether SQLite dataset can be modified to better determine field type.

I posted a demo project in the database board today (http://www.lazarus.freepascal.org/index.php/topic,17653.0.html). Let's see how the experts there can help me. Otherwise I'll try to post it to the SQLite mailing list.

NULL values as zeros

Thank you, perfect.

If you encounter another crash, please try to catch at least the exception message

The debugger led me into the GetItem method, that's why I came up with the posted code.

OnGetChartDataItem... Suggestions for better API are welcome

No -- your solution is very straighforward to me, it can't be any better.

OnHintPosition event

Perfect, thanks.

DisableControls/EnableControls of the dataset
Suggestions are welcome.

I'll think about it.

There is another issue that I forgot: In the pivot query like that discussed in above-mentioned posting I am using spaces in the field name aliases. This makes problems with the assignment of the field name to the FieldY property of TDBChartSource where you seem to allow usage of a list of comma-separated field names. When a field name contains spaces they are used as a separator in the string list as well, and the ChartSource fails to recognize the correct field name. I solved this initially by quoting the field name. But then I had to work around another issue occuring in the new OnGetItem event handler where I needed the fieldname which could not be used directly in a FieldByName call due to the quotes. All this mess can be avoided if the stringlist FieldY has StrictDelimiter=true:

Code: [Select]

constructor TDbChartSource.Create(AOwner: TComponent);
begin
  inherited Create(AOwner);
  FDataLink := TDbChartSourceDataLink.Create(Self);
  FFieldYList := TStringList.Create;
  FFieldYList.StrictDelimiter := true;                 // added by wp
  FYCount := 0; // Set to 1 by inherited.
end;


[Ask]

FFieldYList.StrictDelimiter := true;

Yes, you are right -- other similar places in TAChart already set StrictDelimiter,
this was an oversight.
Fixed in r38038.
By the way, do you think there is enough probability of a comma
in a field name to justify moving to a "|" separator instead?
I thought is it very rare, but I thought so about spaces too...

[wp]

Yes, I could imagine a few applications where I would be tempted to label a column with a comma, like "Current, mA". Therefore, "|" would be on the safe side.

[wp]

Sorry, I have to come back to the scrolling issue discussed above. It's not only a scrolling issue, it even looks as if the program is not responding any more. Only after I remove the connection of the DBChartSource to the Dataset, everything is fine again. I can avoid this behavior if I apply the patch you mention in the bugtracker, i.e. comment out "FChartSrc.Reset" in the TDbChartSourceDataLink.DataSetChanged method.

Of course, now I have to apply this patch every time when I update the lazarus sources. Could you apply a compiler directive around this line which removes the "FChartSrc.Reset" if the directive is activated. Of course, the directive should be off by default, but I could activate it in my code. Or is there another way?

Code: [Select]

procedure TDbChartSourceDataLink.DataSetChanged;
begin
  inherited DataSetChanged;
{$IFDEF PATCH_DBCHARTSOURCE_SCROLLING_ISSUE}
  if DataSet.State = dsBrowse then
    FChartSrc.Reset;
{$ENDIF}
end;

Unfortunately the issue seems to be related to some additonal condtions in my code, so I am not able to reproduce it in a short demo quickly.

[Ask]

Hm. This will disable chart auto-refresh upon the dataset change.
Since it may also have performance implications, I can even consider
TDbChartSource.AutoRefreshChart property.
The db source in general is tricky to get right --
it was implemented more as a proof of concept.
I have added initial db source tests to the test suite --
hopefully the problems can be modelled and isolated there.
Meanwhile, if you disable auto-refresh anyway,
did you consider using listsource and CopyForm, like in the db demo?

[wp]

did you consider using listsource and CopyForm, like in the db demo?

No, I did not. Good idea, I think it will solve also two of the other issues that I came across today - see the attached demo:

• The DBChartSource does not react when the dataset is changed, i.e. when the SQL is modified (click on "Group by day") or when the dataset is deactivated (click on "Dataset active"). However, when the form is resized, the chart is updated.
  
• The chart does not display all data: Remove the checkmark of "Group by day", resize the window a bit to display the modified query results, and use the mouse to scroll the DBGrid down to the end. When you release the mouse button you will see some more datapoints appearing in the chart. This is as if the chart displays only those records that are fetched for the DBGrid.
  
• DateTimeIntervalChartSource is a bit confusing to me: How can I force it to set tick marks only at midnight? I tried to do it by turning off the options dtsMillisecond, dtsSecond, dtsMinute, dtsHour of "Steps", but then the entire axis disappears.
  

[wp]

The DBChartSource does not react when the dataset is changed

Yes, copying the DBChartSource into a ListChartSource solved this issue. When looking at the db demo I did not catch the point that this is essentially needed.

The chart does not display all data ... This is as if the chart displays only those records that are fetched for the DBGrid.

The reason is not in the DBGrid, but in the database. SQLITE does not report the dataset's RecordCount correctly unless it has not reached the last record. Therefore, I added a call to "Dataset.Last" before copying the DBChartSource, and everything was fine.

DateTimeIntervalChartSource is a bit confusing to me

Still no solution.

[wp]

Still no solution.

Increasing Params.MaxLength to 80 and leaving Steps and DateTimeFormat by their default did what I wanted.