View Issue Details

IDProjectCategoryView StatusLast Update
0036532FPCfpReportpublic2020-01-14 08:01
ReporterPascal RiekenbergAssigned ToMichael Van Canneyt 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformi386OSWindows 10 x64OS Version1903
Product Version3.3.1Product Build43889 
Target VersionFixed in Version3.3.1 
Summary0036532: Some changes broke nested group demo
DescriptionSome changes of the last two years broke the nested loop demo.
Steps To ReproduceSome of the changes in the attached patch may brake other functionality.
If there is a test report and data for a complex master detail scenario i can construct a new report which includes the aggregation funtions from the nested group demo to have most complex test.
TagsNo tags attached.
Fixed in Revision43930
FPCOldBugId
FPCTarget3.2.0
Attached Files
  • fpreport.pp.08.01.20.2.patch (2,935 bytes)
    Index: packages/fcl-report/src/fpreport.pp
    ===================================================================
    --- packages/fcl-report/src/fpreport.pp	(revision 43889)
    +++ packages/fcl-report/src/fpreport.pp	(working copy)
    @@ -22,7 +22,7 @@
     // Global debugging
     { $define gdebug}
     // Separate for aggregate variables
    -{$define gdebuga}
    +{ $define gdebuga}
     
     interface
     
    @@ -3279,8 +3279,8 @@
         begin
         if FDataName='' then
           ExtractDataName(AData);
    -    if (FDataName='') then
    -      raise EReportError.CreateFmt(SErrAggregateWithoutDataName, [FExpressionNode.AsString]);
    +    //if (FDataName='') then
    +    //  raise EReportError.CreateFmt(SErrAggregateWithoutDataName, [FExpressionNode.AsString]);
         FExpressionNode.InitAggregate;
         end
       else
    @@ -3298,15 +3298,24 @@
     procedure TFPReportVariable.ExtractDataName(aData : TFPReportDataCollection; ANode : TFPExprNode);
     
     Var
    -  L,I : Integer;
    -  DS : String;
    +  L,I,P : Integer;
    +  DS,N : String;
     
     begin
       if (aNode is TFPExprVariable) then
         begin
    -    DS:=ExtractWord(1,TFPExprVariable(ANode).Identifier.Name,['.']);
    -    If AData.FindReportData(DS)<>Nil then
    -      FDataName:=DS;
    +    N:=TFPExprVariable(ANode).Identifier.Name;
    +    P:=Pos('.', N);
    +    if P > 0 then
    +      begin
    +      DS:=Copy(N, 1, P-1);
    +      If AData.FindReportData(DS)<>Nil then
    +        FDataName:=DS
    +      else
    +        raise EReportError.CreateFmt(SErrAggregateWithoutDataName, [ANode.AsString]);
    +      end
    +    else
    +      FDataName := aData.GetData(0).Data.Name;
         end
       else if (ANode is TFPExprFunction) then
         begin
    @@ -3513,8 +3522,8 @@
         exit;
       If not IsFirstPass then
         begin
    -    FLastValue.ResultType:=rtFloat;
    -    FLastValue.ResFloat:=0;
    +    //FLastValue.ResultType:=rtFloat;
    +    //FLastValue.ResFloat:=0;
         exit;
         end;
       if (FResetValue=#0) then
    @@ -8511,6 +8520,9 @@
               writeln('registering (dotted name)... '+ d+'.'+f);
               {$endif}
               df.ExprIdentierDef := FExpr.Identifiers.AddVariable(d+'.'+f, r, @df.GetRTValue);
    +          // for main data also add simple identifier, so you can leave out qualifier in expression
    +          if J=0 then
    +            FExpr.Identifiers.AddVariable(f, r, @df.GetRTValue);
               end
             else
               begin
    @@ -8878,7 +8890,7 @@
       if PageCount=0 then
         aErrors.Add(SErrNeedPages);
       For I:=0 to PageCount-1 do
    -    Pages[i].Validate(aErrors);
    +    Pages[I].Validate(aErrors);
     end;
     
     procedure TFPCustomReport.Validate;
    @@ -12098,12 +12110,11 @@
             {$endif}
             // DumpData(aPageData);
             PrepareRecord(aData);
    +        Report.UpdateAggregates(aPage,aData);
             if FNewPage then
               StartNewPage;
             ShowDataHeaderBand;
             HandleGroupBands;
    -        // This must be done after the groups were handled.
    -        Report.UpdateAggregates(aPage,aData);
             ShowDataBand;
             aData.Next;
             end;
    
  • countries3.csv (9,830 bytes)
  • test3.fpr (62,713 bytes)
  • fpreport.pp.13.01.20.patch (1,675 bytes)
    Index: packages/fcl-report/src/fpreport.pp
    ===================================================================
    --- packages/fcl-report/src/fpreport.pp	(revision 43921)
    +++ packages/fcl-report/src/fpreport.pp	(working copy)
    @@ -3513,11 +3513,7 @@
       if Not SameText(aData.Name,FDataName) then
         exit;
       If not IsFirstPass then
    -    begin
    -    FLastValue.ResultType:=rtFloat;
    -    FLastValue.ResFloat:=0;
         exit;
    -    end;
       if (FResetValue=#0) then
         begin
         FResetValue:=#255;
    @@ -3624,8 +3620,8 @@
             inc(FAggregateValuesIndex);
             end;
           FResetValue:=lResetValue;
    +      FLastValue:=FAggregateValue;
           FAggregateValue:=PFPExpressionResult(FAggregateValues[FAggregateValuesIndex])^;
    -      FLastValue:=FAggregateValue;
           end
         else
           begin
    @@ -11932,12 +11928,11 @@
             {$endif}
             // DumpData(lData);
             PrepareRecord(lData);
    +        Report.UpdateAggregates(lPage,lData);
             if FNewPage then
               StartNewPage;
             ShowDataHeaderBand;
             HandleGroupBands;
    -        // This must be done after the groups were handled.
    -        Report.UpdateAggregates(lPage,lData);
             ShowDataBand;
             lData.Next;
             end;  { while not lData.EOF }
    @@ -12133,12 +12128,11 @@
             {$endif}
             // DumpData(aPageData);
             PrepareRecord(aData);
    +        Report.UpdateAggregates(aPage,aData);
             if FNewPage then
               StartNewPage;
             ShowDataHeaderBand;
             HandleGroupBands;
    -        // This must be done after the groups were handled.
    -        Report.UpdateAggregates(aPage,aData);
             ShowDataBand;
             aData.Next;
             end;
    

Activities

Pascal Riekenberg

2020-01-08 16:04

reporter  

fpreport.pp.08.01.20.2.patch (2,935 bytes)
Index: packages/fcl-report/src/fpreport.pp
===================================================================
--- packages/fcl-report/src/fpreport.pp	(revision 43889)
+++ packages/fcl-report/src/fpreport.pp	(working copy)
@@ -22,7 +22,7 @@
 // Global debugging
 { $define gdebug}
 // Separate for aggregate variables
-{$define gdebuga}
+{ $define gdebuga}
 
 interface
 
@@ -3279,8 +3279,8 @@
     begin
     if FDataName='' then
       ExtractDataName(AData);
-    if (FDataName='') then
-      raise EReportError.CreateFmt(SErrAggregateWithoutDataName, [FExpressionNode.AsString]);
+    //if (FDataName='') then
+    //  raise EReportError.CreateFmt(SErrAggregateWithoutDataName, [FExpressionNode.AsString]);
     FExpressionNode.InitAggregate;
     end
   else
@@ -3298,15 +3298,24 @@
 procedure TFPReportVariable.ExtractDataName(aData : TFPReportDataCollection; ANode : TFPExprNode);
 
 Var
-  L,I : Integer;
-  DS : String;
+  L,I,P : Integer;
+  DS,N : String;
 
 begin
   if (aNode is TFPExprVariable) then
     begin
-    DS:=ExtractWord(1,TFPExprVariable(ANode).Identifier.Name,['.']);
-    If AData.FindReportData(DS)<>Nil then
-      FDataName:=DS;
+    N:=TFPExprVariable(ANode).Identifier.Name;
+    P:=Pos('.', N);
+    if P > 0 then
+      begin
+      DS:=Copy(N, 1, P-1);
+      If AData.FindReportData(DS)<>Nil then
+        FDataName:=DS
+      else
+        raise EReportError.CreateFmt(SErrAggregateWithoutDataName, [ANode.AsString]);
+      end
+    else
+      FDataName := aData.GetData(0).Data.Name;
     end
   else if (ANode is TFPExprFunction) then
     begin
@@ -3513,8 +3522,8 @@
     exit;
   If not IsFirstPass then
     begin
-    FLastValue.ResultType:=rtFloat;
-    FLastValue.ResFloat:=0;
+    //FLastValue.ResultType:=rtFloat;
+    //FLastValue.ResFloat:=0;
     exit;
     end;
   if (FResetValue=#0) then
@@ -8511,6 +8520,9 @@
           writeln('registering (dotted name)... '+ d+'.'+f);
           {$endif}
           df.ExprIdentierDef := FExpr.Identifiers.AddVariable(d+'.'+f, r, @df.GetRTValue);
+          // for main data also add simple identifier, so you can leave out qualifier in expression
+          if J=0 then
+            FExpr.Identifiers.AddVariable(f, r, @df.GetRTValue);
           end
         else
           begin
@@ -8878,7 +8890,7 @@
   if PageCount=0 then
     aErrors.Add(SErrNeedPages);
   For I:=0 to PageCount-1 do
-    Pages[i].Validate(aErrors);
+    Pages[I].Validate(aErrors);
 end;
 
 procedure TFPCustomReport.Validate;
@@ -12098,12 +12110,11 @@
         {$endif}
         // DumpData(aPageData);
         PrepareRecord(aData);
+        Report.UpdateAggregates(aPage,aData);
         if FNewPage then
           StartNewPage;
         ShowDataHeaderBand;
         HandleGroupBands;
-        // This must be done after the groups were handled.
-        Report.UpdateAggregates(aPage,aData);
         ShowDataBand;
         aData.Next;
         end;

Michael Van Canneyt

2020-01-08 20:50

administrator   ~0120276

The changes were applied for a reason. Unfortunately, I no longer see why I changed the behaviour :(

Can you send me a report design that goes wrong and that is fixed by this fix ?

Pascal Riekenberg

2020-01-08 21:00

reporter  

countries3.csv (9,830 bytes)

Pascal Riekenberg

2020-01-08 21:00

reporter   ~0120278

nestedgroups demo in fcldemo:

fcldemo -d nestedgroups

Attached fpr is the multicolumn version done for PasCon 2017 (done in code)

test3.fpr (62,713 bytes)

Pascal Riekenberg

2020-01-08 21:06

reporter   ~0120279

First error is:
ExprVariable has Aggregate but cannot determine data source: sum(strtofloat(population) / 1000000).

"population" is field in the first and only datasource assigned to the page only. This datasource has a blank name.
In theory an expression could have fields of different datasources. Though didn't test it.

Pascal Riekenberg

2020-01-08 21:09

reporter   ~0120280

The following is not needed for fixing, just convenience.

@@ -8511,6 +8520,9 @@
           writeln('registering (dotted name)... '+ d+'.'+f);
           {$endif}
           df.ExprIdentierDef := FExpr.Identifiers.AddVariable(d+'.'+f, r, @df.GetRTValue);
+ // for main data also add simple identifier, so you can leave out qualifier in expression
+ if J=0 then
+ FExpr.Identifiers.AddVariable(f, r, @df.GetRTValue);
           end
         else
           begin

Michael Van Canneyt

2020-01-08 21:15

administrator   ~0120281

I'm not sure about leaving away the dataset name. If you use the designer, then an empty dataset name cannot happen.
In code it can probably happen, but I'm of half a mind to make it obligatory. I'll think about this.

Michael Van Canneyt

2020-01-08 21:16

administrator   ~0120282

The convenience we will not do, as it may create name clashes when you have 2 datasets with the same field.
(that was one of the cases that needed fixing)

And there we have the reason for requiring the dataset.name. If you leave it out, we'll have to check there is only 1 dataset.

Pascal Riekenberg

2020-01-08 21:41

reporter   ~0120283

The check will be very simple!

Pascal Riekenberg

2020-01-09 14:37

reporter   ~0120288

Related changes are:
r38799
r38803

Michael Van Canneyt

2020-01-09 14:54

administrator   ~0120290

I know.
Unfortunately, I cannot find why I did those changes.
Probably the result of some discussions with Stephano :(

Pascal Riekenberg

2020-01-09 16:33

reporter   ~0120291

Can you contact him and request samples?

Pascal Riekenberg

2020-01-09 16:35

reporter   ~0120293

We should add tests for every issue.

Pascal Riekenberg

2020-01-13 14:25

reporter   ~0120402

Most simple patch to get old nested group demo work again. I missed one issue in the last patch.

In UpdateExpressionValue the order of setting FLastValue and FAggregateValue got mixed up which probably
lead to the other changes done in r38803 to make things work again.

I suggest we apply this patch and i'll create a test for the nested group demo. When the error prior r38803 occurs
again we can guarantee not to break nested group demo again.

fpreport.pp.13.01.20.patch (1,675 bytes)
Index: packages/fcl-report/src/fpreport.pp
===================================================================
--- packages/fcl-report/src/fpreport.pp	(revision 43921)
+++ packages/fcl-report/src/fpreport.pp	(working copy)
@@ -3513,11 +3513,7 @@
   if Not SameText(aData.Name,FDataName) then
     exit;
   If not IsFirstPass then
-    begin
-    FLastValue.ResultType:=rtFloat;
-    FLastValue.ResFloat:=0;
     exit;
-    end;
   if (FResetValue=#0) then
     begin
     FResetValue:=#255;
@@ -3624,8 +3620,8 @@
         inc(FAggregateValuesIndex);
         end;
       FResetValue:=lResetValue;
+      FLastValue:=FAggregateValue;
       FAggregateValue:=PFPExpressionResult(FAggregateValues[FAggregateValuesIndex])^;
-      FLastValue:=FAggregateValue;
       end
     else
       begin
@@ -11932,12 +11928,11 @@
         {$endif}
         // DumpData(lData);
         PrepareRecord(lData);
+        Report.UpdateAggregates(lPage,lData);
         if FNewPage then
           StartNewPage;
         ShowDataHeaderBand;
         HandleGroupBands;
-        // This must be done after the groups were handled.
-        Report.UpdateAggregates(lPage,lData);
         ShowDataBand;
         lData.Next;
         end;  { while not lData.EOF }
@@ -12133,12 +12128,11 @@
         {$endif}
         // DumpData(aPageData);
         PrepareRecord(aData);
+        Report.UpdateAggregates(aPage,aData);
         if FNewPage then
           StartNewPage;
         ShowDataHeaderBand;
         HandleGroupBands;
-        // This must be done after the groups were handled.
-        Report.UpdateAggregates(aPage,aData);
         ShowDataBand;
         aData.Next;
         end;

Michael Van Canneyt

2020-01-13 22:15

administrator   ~0120413

Applied the last patch, thank you.
Now we should indeed set a baseline, as you correctly suggest.

Pascal Riekenberg

2020-01-14 08:01

reporter   ~0120423

Thanks.

Issue History

Date Modified Username Field Change
2020-01-08 16:04 Pascal Riekenberg New Issue
2020-01-08 16:04 Pascal Riekenberg Status new => assigned
2020-01-08 16:04 Pascal Riekenberg Assigned To => Michael Van Canneyt
2020-01-08 16:04 Pascal Riekenberg File Added: fpreport.pp.08.01.20.2.patch
2020-01-08 20:50 Michael Van Canneyt Status assigned => feedback
2020-01-08 20:50 Michael Van Canneyt FPCTarget => -
2020-01-08 20:50 Michael Van Canneyt Note Added: 0120276
2020-01-08 21:00 Pascal Riekenberg File Added: countries3.csv
2020-01-08 21:00 Pascal Riekenberg File Added: test3.fpr
2020-01-08 21:00 Pascal Riekenberg Note Added: 0120278
2020-01-08 21:00 Pascal Riekenberg Status feedback => assigned
2020-01-08 21:06 Pascal Riekenberg Note Added: 0120279
2020-01-08 21:09 Pascal Riekenberg Note Added: 0120280
2020-01-08 21:15 Michael Van Canneyt Note Added: 0120281
2020-01-08 21:16 Michael Van Canneyt Note Added: 0120282
2020-01-08 21:41 Pascal Riekenberg Note Added: 0120283
2020-01-09 14:37 Pascal Riekenberg Note Added: 0120288
2020-01-09 14:54 Michael Van Canneyt Note Added: 0120290
2020-01-09 16:33 Pascal Riekenberg Note Added: 0120291
2020-01-09 16:35 Pascal Riekenberg Note Added: 0120293
2020-01-13 14:25 Pascal Riekenberg File Added: fpreport.pp.13.01.20.patch
2020-01-13 14:25 Pascal Riekenberg Note Added: 0120402
2020-01-13 22:15 Michael Van Canneyt Status assigned => resolved
2020-01-13 22:15 Michael Van Canneyt Resolution open => fixed
2020-01-13 22:15 Michael Van Canneyt Fixed in Version => 3.3.1
2020-01-13 22:15 Michael Van Canneyt Fixed in Revision => 43930
2020-01-13 22:15 Michael Van Canneyt FPCTarget - => 3.2.0
2020-01-13 22:15 Michael Van Canneyt Note Added: 0120413
2020-01-14 08:01 Pascal Riekenberg Status resolved => closed
2020-01-14 08:01 Pascal Riekenberg Note Added: 0120423