View Issue Details

IDProjectCategoryView StatusLast Update
0029999Lazarus CCRFPSpreadsheetpublic2016-04-13 12:53
ReporterimballoAssigned Towp 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformi386-64OSwindows OS Version10
Summary0029999: Changed formula integer value after adding a row
DescriptionIf a formula contains a very big integer then adding a preceding row or a preceding col make the integer smaller than a smallint.
Steps To ReproduceTested with spready example program (also with a program of mine):
- set f.i. cell C5 to "=2000000";
- add a row before cell C5 (f.i. on row 3);
- now the formula has changed to "=33920".
TagsNo tags attached.
WidgetsetWin32/Win64
Attached Files

Activities

wp

2016-04-13 07:46

developer   ~0091991

Which fpspreadsheet version are you using? I cannot reproduce with fps-trunk / Laz trunk (32 bit) / fpc 3.0 / Win 7 (64 bit). Is it essential that the platform is 64 bit?

imballo

2016-04-13 08:33

reporter   ~0091992

fpspreadsheet 1.6.1
lazarus: 1.6
fpc: 3.0.0
svn: 51630
i386-win32-win32/win34
The compiled program is win32.
The os is win10-64bit.

wp

2016-04-13 09:47

developer   ~0091994

Switched to fps 1.6.1, same result: cannot reproduce. Please try the attached demo - does it show the issue with you? Are you using any special compilation parameters?

wp

2016-04-13 09:48

developer  

fps-large-int.zip (2,285 bytes)

imballo

2016-04-13 09:57

reporter   ~0091995

I think to have found the problem.
When adding the new row/column all the cells are converted to RPN and back.
During the conversion to RPN there is this procedure below that creates an integer value by passing a word argument (should be int64).
{@@ ----------------------------------------------------------------------------
  Creates an entry in the RPN array for a 2-byte unsigned integer

  @param AValue Integer value to be inserted into the formula
  @param ANext Pointer to the next RPN item in the list
-------------------------------------------------------------------------------}
function RPNInteger(AValue: Word; ANext: PRPNItem): PRPNItem;
begin
  Result := NewRPNItem;
  Result^.FE.ElementKind := fekInteger;
  Result^.FE.IntValue := AValue;
  Result^.Next := ANext;
end;

imballo

2016-04-13 10:00

reporter   ~0091996

Also reproduced in your demo.
To reproduce you have to enter a formula "=200000000" and not "20000000".

wp

2016-04-13 10:03

developer   ~0091997

Sounds reasonable. But I still wonder why I don't see this issue.

imballo

2016-04-13 10:16

reporter   ~0091998

and also here:

  TsFormulaElement = record
    ElementKind: TFEKind;
    Row, Row2: Cardinal; // zero-based
    Col, Col2: Cardinal; // zero-based
    DoubleValue: double;
    IntValue: word; //!!!!!!!!! this should be int64
    StringValue: String;
    RelFlags: TsRelFlags; // store info on relative/absolute addresses
    FuncName: String;
    ParamsNum: Byte;
  end;

imballo

2016-04-13 10:17

reporter   ~0091999

I tried changing the two type to int64 and the problem solved.

wp

2016-04-13 10:18

developer   ~0092000

I see the bug in the saved file even before inserting a row. But your solution is just a workaround, I think the formula parser must switch to floating point value once the number does not not fit into the integer range.

imballo

2016-04-13 10:26

reporter   ~0092001

Ok, as you wish. The important is to solve it.

wp

2016-04-13 11:05

developer   ~0092002

> Also reproduced in your demo.
> To reproduce you have to enter a formula "=200000000" and not "20000000".

That's what I did, it's already coded as a formula in FormCreate.

wp

2016-04-13 11:18

developer   ~0092004

Last edited: 2016-04-13 11:19

View 2 revisions

Since I cannot exactly reproduce your error I would like you to check this code before I upload it.

Please open fpsExprParser.pas, find the method TsExprParser.Primitive. Replace the
block "if (tokenType = ttNumber) ..." by

<<< --- begin code ---

  if (TokenType = ttNumber) then
  begin
    if TryStrToInt64(CurrentToken, I) then
    begin
      // Excel BIFF uses 2-byte integers. All others use floats instead of int.
      if (I > word($FFFF)) or (I < 0) then
        Result := TsConstExprNode.CreateFloat(self, 1.0*I)
      else
        Result := TsConstExprNode.CreateInteger(self, I);
    end else
    begin
      if TryStrToFloat(CurrentToken, X, FFormatSettings) then
        Result := TsConstExprNode.CreateFloat(self, X)
      else
        ParserError(Format(rsInvalidFloat, [CurrentToken]));
    end;
  end

---- end code --- >>>

imballo

2016-04-13 11:33

reporter   ~0092007

Checked again the demo. I was not making the cells to calc again (was showing the old value).
Even the coded formula you inserted got the problem.

imballo

2016-04-13 12:01

reporter   ~0092010

The problem remains. Tried to put a breakpoint inside but is not activated.
Tried to follow the execution and it seems that:
- the expression seems to be parsed correctly inside
parser.Expression := ACell^.FormulaValue;
(the expression is still correct)

- but not inside
Result := parser.RPNFormula;
(the result is downsized to word)

function TsWorksheet.BuildRPNFormula(ACell: PCell;
  ADestCell: PCell = nil): TsRPNFormula;
var
  parser: TsSpreadsheetParser;
begin
  if not HasFormula(ACell) then begin
    SetLength(Result, 0);
    exit;
  end;
  parser := TsSpreadsheetParser.Create(self);
  try
    if ADestCell <> nil then
      parser.PrepareCopyMode(ACell, ADestCell);
    parser.Expression := ACell^.FormulaValue;
    Result := parser.RPNFormula;
  finally
    parser.Free;
  end;
end;

wp

2016-04-13 12:07

developer   ~0092011

Sorry, I decided now to keep Excel chaos out of fpsExprParser and go your way. In addition, a modification of the biff writer code is required to allow integers only if the value is <65536. Uploaded to trunk, r4619. Please test and close if ok.

Issue History

Date Modified Username Field Change
2016-04-13 07:34 imballo New Issue
2016-04-13 07:42 wp Assigned To => wp
2016-04-13 07:42 wp Status new => assigned
2016-04-13 07:46 wp Note Added: 0091991
2016-04-13 08:33 imballo Note Added: 0091992
2016-04-13 09:47 wp Note Added: 0091994
2016-04-13 09:47 wp Status assigned => feedback
2016-04-13 09:48 wp File Added: fps-large-int.zip
2016-04-13 09:57 imballo Note Added: 0091995
2016-04-13 09:57 imballo Status feedback => assigned
2016-04-13 10:00 imballo Note Added: 0091996
2016-04-13 10:03 wp Note Added: 0091997
2016-04-13 10:16 imballo Note Added: 0091998
2016-04-13 10:17 imballo Note Added: 0091999
2016-04-13 10:18 wp Note Added: 0092000
2016-04-13 10:26 imballo Note Added: 0092001
2016-04-13 11:05 wp Note Added: 0092002
2016-04-13 11:18 wp Note Added: 0092004
2016-04-13 11:18 wp Status assigned => feedback
2016-04-13 11:19 wp Note Edited: 0092004 View Revisions
2016-04-13 11:33 imballo Note Added: 0092007
2016-04-13 11:33 imballo Status feedback => assigned
2016-04-13 12:01 imballo Note Added: 0092010
2016-04-13 12:07 wp Note Added: 0092011
2016-04-13 12:07 wp Status assigned => resolved
2016-04-13 12:07 wp Resolution open => fixed
2016-04-13 12:53 imballo Status resolved => closed