View Issue Details

IDProjectCategoryView StatusLast Update
0025876LazarusLCLpublic2015-02-20 12:02
ReporterRicardo Cervera Assigned ToLuiz Americo  
PrioritynormalSeverityminorReproducibilityalways
Status resolvedResolutionfixed 
Platformlazarus 1.0.14OSWindows  
Summary0025876: DBEdit doesn´t updates LookUp fields
DescriptionIn the example attached if I edit the record in the Grid, when I move the cursor to another field the LookUpField is updated.

But if I edit the record using DBEdits, the LookUpField is not updated until the associatte DBEdit get the focus. (it doesn't matter if the DBEdit is ReadOnly or not)

It looks like that when lookup field changes it does not generates deFieldChange event and TDBEdit is not notified about change.
TagsNo tags attached.
Fixed in Revision44505
LazTarget-
Widgetset
Attached Files

Activities

Ricardo Cervera

2014-03-17 20:58

reporter  

Lookup Error.rar (34,359 bytes)

Luiz Americo

2014-03-18 03:08

developer   ~0073801

The problem is related to fpc / db components not to Lazarus

Currently the lookup field is not notified when the LookupDataset is changed.

To fix this, one idea is to use a datalink in TField so it can be notified

Ricardo Cervera

2014-03-19 21:35

reporter   ~0073855

So, do you think this a bug or not? I don´t know exactly what I can expect and I don´t think I have the skill to solve it
Perhaps I have to open a ticket in another place.

Luiz Americo

2014-03-21 00:47

developer   ~0073871

It's a missing feature. Even Delphi 7 does not implements it.

I prepared a patch to implement this feature but it leads to undesired side effects.

IMHO is not worth implementing this feature.

Attached a patch and the program to test.

Also moved the report to fpc

Luiz Americo

2014-03-21 00:52

developer  

lookup.diff (8,204 bytes)   
Index: packages/fcl-db/src/base/dataset.inc
===================================================================
--- packages/fcl-db/src/base/dataset.inc	(revision 27215)
+++ packages/fcl-db/src/base/dataset.inc	(working copy)
@@ -83,6 +83,7 @@
 procedure TDataSet.BindFields(Binding: Boolean);
 
 var i, FieldIndex: Integer;
+  TheLookupDataSet: TDataset;
 
 begin
   { FieldNo is set to -1 for calculated/lookup fields, to 0 for unbound field
@@ -98,13 +99,14 @@
           FOffset := FCalcFieldsSize;
           Inc(FCalcFieldsSize, DataSize + 1);
           if FieldKind in [fkLookup] then begin
-            if ((FLookupDataSet = nil) or (FLookupKeyFields = '') or
+            if ((FLookupDataSource = nil) or (FLookupKeyFields = '') or
                (FLookupResultField = '') or (FKeyFields = '')) then
               DatabaseErrorFmt(SLookupInfoError, [DisplayName]);
+            TheLookupDataSet := FLookupDataSource.DataSet;
             FFields.CheckFieldNames(FKeyFields);
-            FLookupDataSet.Open;
-            FLookupDataSet.Fields.CheckFieldNames(FLookupKeyFields);
-            FLookupDataSet.FieldByName(FLookupResultField);
+            TheLookupDataSet.Open;
+            TheLookupDataSet.Fields.CheckFieldNames(FLookupKeyFields);
+            TheLookupDataSet.FieldByName(FLookupResultField);
             if FLookupCache then RefreshLookupList;
           end
         end else begin
Index: packages/fcl-db/src/base/fields.inc
===================================================================
--- packages/fcl-db/src/base/fields.inc	(revision 27215)
+++ packages/fcl-db/src/base/fields.inc	(working copy)
@@ -627,8 +627,8 @@
 begin
   if FLookupCache then
     Value := LookupList.ValueOfKey(FDataSet.FieldValues[FKeyFields])
-  else if Assigned(FLookupDataSet) and FDataSet.Active then
-    Value := FLookupDataSet.Lookup(FLookupKeyfields, FDataSet.FieldValues[FKeyFields], FLookupresultField);
+  else if Assigned(FLookupDataSource) and FDataSet.Active then
+    Value := FLookupDataSource.DataSet.Lookup(FLookupKeyfields, FDataSet.FieldValues[FKeyFields], FLookupresultField);
 end;
 
 function TField.GetIndex: longint;
@@ -645,6 +645,28 @@
   Result := FieldKind = fkLookup;
 end;
 
+function TField.GetLookupDataSet: TDataSet;
+begin
+  if FLookupDataSource <> nil then
+    Result := FLookupDataSource.DataSet
+  else
+    Result := nil;
+end;
+
+procedure TField.LookupDataChange(Sender: TObject; Field: TField);
+begin
+  if (Field <> nil) and (Field = LookupDataSet.FindField(LookupResultField))
+    and (DataSet <> nil) and DataSet.Active then
+      DataSet.CalculateFields(DataSet.ActiveBuffer);
+end;
+
+procedure TField.LookupStateChange(Sender: TObject);
+begin
+  //when the linked lookup dataset is destroyied a state change event is fired
+  if FLookupDataSource.DataSet = nil then
+    FreeAndNil(FLookupDataSource);
+end;
+
 function TField.GetAsLargeInt: LargeInt;
 begin
   Raise AccessError(SLargeInt);
@@ -708,43 +730,36 @@
 procedure TField.RefreshLookupList;
 var
   tmpActive: Boolean;
+  TheLookupDataset: TDataSet;
 begin
-  if not Assigned(FLookupDataSet) or (Length(FLookupKeyfields) = 0)
+  if not Assigned(FLookupDataSource) or (Length(FLookupKeyfields) = 0)
   or (Length(FLookupresultField) = 0) or (Length(FKeyFields) = 0) then
     Exit;
-    
-  tmpActive := FLookupDataSet.Active;
+  TheLookupDataset := FLookupDataSource.DataSet;
+  tmpActive := TheLookupDataSet.Active;
   try
-    FLookupDataSet.Active := True;
+    TheLookupDataSet.Active := True;
     FFields.CheckFieldNames(FKeyFields);
-    FLookupDataSet.Fields.CheckFieldNames(FLookupKeyFields);
-    FLookupDataset.FieldByName(FLookupResultField); // I presume that if it doesn't exist it throws exception, and that a field with null value is still valid
+    TheLookupDataSet.Fields.CheckFieldNames(FLookupKeyFields);
+    TheLookupDataset.FieldByName(FLookupResultField); // I presume that if it doesn't exist it throws exception, and that a field with null value is still valid
     LookupList.Clear; // have to be F-less because we might be creating it here with getter!
 
-    FLookupDataSet.DisableControls;
+    TheLookupDataSet.DisableControls;
     try
-      FLookupDataSet.First;
-      while not FLookupDataSet.Eof do
+      TheLookupDataSet.First;
+      while not TheLookupDataSet.Eof do
       begin
-        FLookupList.Add(FLookupDataSet.FieldValues[FLookupKeyfields], FLookupDataSet.FieldValues[FLookupResultField]);
-        FLookupDataSet.Next;
+        FLookupList.Add(TheLookupDataSet.FieldValues[FLookupKeyfields], TheLookupDataSet.FieldValues[FLookupResultField]);
+        TheLookupDataSet.Next;
       end;
     finally
-      FLookupDataSet.EnableControls;
+      TheLookupDataSet.EnableControls;
     end;
   finally
-    FLookupDataSet.Active := tmpActive;
+    TheLookupDataSet.Active := tmpActive;
   end;
 end;
 
-procedure TField.Notification(AComponent: TComponent; Operation: TOperation);
-
-begin
-  Inherited Notification(AComponent,Operation);
-  if (Operation = opRemove) and (AComponent = FLookupDataSet) then
-    FLookupDataSet := nil;
-end;
-
 procedure TField.PropertyChanged(LayoutAffected: Boolean);
 
 begin
@@ -995,6 +1010,22 @@
   FieldKind := ValueToLookupMap[AValue];
 end;
 
+procedure TField.SetLookupDataSet(Value: TDataSet);
+begin
+  if Value <> nil then
+  begin
+    if FLookupDataSource = nil then
+    begin
+      FLookupDataSource := TDataSource.Create(Self);
+      FLookupDataSource.OnDataChange := @LookupDataChange;
+      FLookupDataSource.OnStateChange := @LookupStateChange;
+    end;
+    FLookupDataSource.DataSet := Value;
+  end
+  else
+    FreeAndNil(FLookupDataSource);
+end;
+
 procedure TField.SetReadOnly(const AValue: Boolean);
 begin
   if (FReadOnly<>AValue) then
Index: packages/fcl-db/src/base/db.pas
===================================================================
--- packages/fcl-db/src/base/db.pas	(revision 27215)
+++ packages/fcl-db/src/base/db.pas	(working copy)
@@ -287,7 +287,7 @@
     FIsIndexField : Boolean;
     FKeyFields : String;
     FLookupCache : Boolean;
-    FLookupDataSet : TDataSet;
+    FLookupDataSource : TDataSource;
     FLookupKeyfields : String;
     FLookupresultField : String;
     FLookupList: TLookupList;
@@ -307,6 +307,9 @@
     FProviderFlags : TProviderFlags;
     function GetIndex : longint;
     function GetLookup: Boolean;
+    function GetLookupDataSet: TDataSet;
+    procedure LookupDataChange(Sender: TObject; Field: TField);
+    procedure LookupStateChange(Sender: TObject);
     procedure SetAlignment(const AValue: TAlignMent);
     procedure SetIndex(const AValue: Longint);
     function GetDisplayText: String;
@@ -316,6 +319,7 @@
     procedure SetDisplayWidth(const AValue: Longint);
     function GetDisplayWidth: integer;
     procedure SetLookup(const AValue: Boolean);
+    procedure SetLookupDataSet(Value: TDataSet);
     procedure SetReadOnly(const AValue: Boolean);
     procedure SetVisible(const AValue: Boolean);
     function IsDisplayStored : Boolean;
@@ -352,7 +356,6 @@
     function GetParentComponent: TComponent; override;
     procedure GetText(var AText: string; ADisplayText: Boolean); virtual;
     function HasParent: Boolean; override;
-    procedure Notification(AComponent: TComponent; Operation: TOperation); override;
     procedure PropertyChanged(LayoutAffected: Boolean);
     procedure ReadState(Reader: TReader); override;
     procedure SetAsBCD(const AValue: TBCD); virtual;
@@ -440,7 +443,7 @@
     property ImportedConstraint: string read FImportedConstraint write FImportedConstraint;
     property KeyFields: string read FKeyFields write FKeyFields;
     property LookupCache: Boolean read FLookupCache write FLookupCache;
-    property LookupDataSet: TDataSet read FLookupDataSet write FLookupDataSet;
+    property LookupDataSet: TDataSet read GetLookupDataSet write SetLookupDataSet;
     property LookupKeyFields: string read FLookupKeyFields write FLookupKeyFields;
     property LookupResultField: string read FLookupResultField write FLookupResultField;
     property Origin: string read FOrigin write FOrigin;
lookup.diff (8,204 bytes)   

Luiz Americo

2014-03-21 00:52

developer  

bugLookupChange.lpr (1,932 bytes)

LacaK

2014-03-21 11:57

developer   ~0073876

IMHO root of problem is somewhere else.
If user updates regular field then event deFieldChange is received by procedure TDataSet.DataEvent
Then in HandleFieldChange is called CalculateFields(...)
CalculateFields set State=dsCalcFields and calls also CalcLookupValue for lookup field(s).
CalcLookupValue assigns to lookup field new value.

BUT because State is dsCalcFields for this lookup field is not generated deFieldChange ... so DB-aware controls are not notified and does not refresh displayed data (but IMO in the "lookup field" is right value)

Comparing to Delphi documentation IMO there must be set State=dsCalcFields in GetCalcFields method and not in CalculateFields method.

Luiz Americo

2014-03-23 03:00

developer   ~0073904

@Lacak
The change that must be intercepted is not in the dataset that has the lookup field. It's in the field (fkData type) in the dataset that the lookup field references.

See the attached program:
Db1 has no lookup fields
Db2 has a lookup fields that references a Db1 field
When field referenced by the lookup (Db1.ActualField) changes, the Db2 lookup field does not change

I tested in Delphi 7. I also does not update the lookup field.

Technically is possible but can open a can of worms of dataset state synchronization. There's also performance considerations since, for correctness sake, all records in the lookup field should be notified, not only the active.

Luiz Americo

2014-03-23 04:06

developer   ~0073905

Last edited: 2014-03-23 04:11

View 2 revisions

Seems that are two different issues. One i described with changes in other dataset and other related to changes in the same dataset.

I tested in Delphi and when the key field changes in the same dataset a deFieldChange event is fired for the lookup field

Strangely the field instance passed to the lookup event is from the key field

See log below.

IntValue is a fkData field. DetailValue is a lookup field that has as key IntValue.

The address in both events are 10443928 (IntValue). Somehow the ItemValue deRecordChange event is forwarded to the lookup field

TMyDataLink is a TFieldDatalink descendant

IntValue updated DetailDatasetRecNo:1
DataEvent (Field: IntValue) deFieldChange - Info: 10443928
TMyDataLink.RecordChanged - aField: 10443928 / IntValue
DataEvent (Field: DetailValue) deFieldChange - Info: 10443928
TMyDataLink.RecordChanged - aField: 10443928 / IntValue


procedure TMyDataLink.DataEvent(Event: TDataEvent; Info: Integer);
begin
  Form1.Memo1.Lines.Add(Format('DataEvent (Field: %s) %s - Info: %d', [FieldName,DataEventNames[Event], Info]));
  inherited;
end;
procedure TMyDataLink.RecordChanged(aField: TField);
var
  S: String;
begin
  if aField <> nil then
    S:= aField.FieldName
  else
    S:= 'nil';
  Form1.Memo1.Lines.Add(Format('TMyDataLink.RecordChanged - aField: %d / %s', [Integer(aField), S]));
  inherited;
end;

Luiz Americo

2014-03-23 11:22

developer   ~0073909

After further look seems that the problem is in TFieldDataLink.

It receives the deFieldChange event but does not propagate.

I will patch it.

Moving back to Lazarus ;-)

LacaK

2014-03-24 07:54

developer   ~0073925

@Luiz
IMO fkLookup fields should work as you describe in 0073905 not as you mentioned in 0073904
If record or value of LookupKeyFields changes in LookupDataSet it should does not affect Field which references this LookupDataSet (as you wrote also in Delphi it does not work so).
Only if KeyFields changes then lookup should be performed and value of LookupResultField should be stored to our field.

Luiz Americo

2014-03-24 13:39

developer   ~0073931

@Lacak
To be clear, i'm also against implementing propagating changes from the lookup result since can lead to performance and dataset state issues. Initially i understand that the reporter was requesting this feature (i could not run the test application since is not database agnostic)

Hopefully i fixed in TFieldDataLink

Issue History

Date Modified Username Field Change
2014-03-17 20:58 Ricardo Cervera New Issue
2014-03-17 20:58 Ricardo Cervera File Added: Lookup Error.rar
2014-03-18 03:08 Luiz Americo Note Added: 0073801
2014-03-19 21:35 Ricardo Cervera Note Added: 0073855
2014-03-21 00:47 Luiz Americo Note Added: 0073871
2014-03-21 00:52 Luiz Americo File Added: lookup.diff
2014-03-21 00:52 Luiz Americo File Added: bugLookupChange.lpr
2014-03-21 00:53 Luiz Americo Project Lazarus CCR => FPC
2014-03-21 11:57 LacaK Note Added: 0073876
2014-03-23 03:00 Luiz Americo Note Added: 0073904
2014-03-23 04:06 Luiz Americo Note Added: 0073905
2014-03-23 04:11 Luiz Americo Note Edited: 0073905 View Revisions
2014-03-23 11:22 Luiz Americo Note Added: 0073909
2014-03-23 11:22 Luiz Americo Project FPC => Lazarus
2014-03-24 07:54 LacaK Note Added: 0073925
2014-03-24 13:39 Luiz Americo Note Added: 0073931
2014-03-24 13:40 Luiz Americo Fixed in Revision => 44505
2014-03-24 13:40 Luiz Americo LazTarget => -
2014-03-24 13:40 Luiz Americo Widgetset Win32/Win64 =>
2014-03-24 13:40 Luiz Americo Status new => resolved
2014-03-24 13:40 Luiz Americo Resolution open => fixed
2014-03-24 13:40 Luiz Americo Assigned To => Luiz Americo