handsontable / hyperformula

HyperFormula is an open-source headless spreadsheet for business web apps. It comes with over 400 formulas, CRUD operations, undo-redo, clipboard support, and sorting.
https://hyperformula.handsontable.com/
Other
1.98k stars 109 forks source link

`Incorrect array size` when calling `MATCH` with a column range referencing an empty sheet (previously: Match Function on Column with Empty Sheet Errors Incorrect Array Size) #1147

Closed BrianHung closed 9 months ago

BrianHung commented 1 year ago

Description

The following code errors with Incorrect array size

Uncaught (in promise) Error: Incorrect array size
    at new ArraySize (ArraySize.js:15:13)
    at new SimpleRangeValue (SimpleRangeValue.js:16:19)
    at SimpleRangeValue.onlyRange (SimpleRangeValue.js:35:12)
    at Interpreter.evaluateAstWithoutPostprocessing (Interpreter.js:222:35)
    at Interpreter.evaluateAst (Interpreter.js:48:20)
    at LookupPlugin.evaluateAst (FunctionPlugin.js:177:29)
    at FunctionPlugin.js:66:43
    at Array.map (<anonymous>)
    at FunctionPlugin.runFunction (FunctionPlugin.js:66:26)
    at LookupPlugin.match (LookupPlugin.js:61:17)
// Create an empty HyperFormula instance.
const hf = HyperFormula.buildEmpty({
  licenseKey: "gpl-v3"
});

// Add a new sheet and get its id.
const table1 = hf.addSheet("table1");
const table2 = hf.addSheet("table2");

// Fill the HyperFormula sheet with data.
hf.setCellContents(
  {
    row: 0,
    col: 0,
    sheet: hf.getSheetId(table1)
  },
  []
);

hf.setCellContents(
  {
    row: 0,
    col: 0,
    sheet: hf.getSheetId(table2)
  },
  [["=MATCH(0, table1!A:A)"]]
);

Expected behavior is that table2!A1 evaluates to #N/A since no match occurs. For some reason, table1!A:A is parsed as a CELL_RANGE instead of a COL_RANGE, like in =SUM(table1!A:A).

Steps to reproduce

https://codesandbox.io/s/spring-cherry-jx071t?file=/src/hyperformulaConfig.js

sequba commented 1 year ago

Hi @BrianHung, thanks for reporting this issue. I can confirm it is a bug, so we'll work on it in one of the upcoming releases.

Apparently, it occurs only when sheet table1 is empty, so a workaround for that would be to initialize sheet1 with non-empty array:

hf.setCellContents(
  {
    row: 0,
    col: 0,
    sheet: hf.getSheetId(table1)
  },
  [['']]
);
sequba commented 1 year ago

Note to self: make sure other array functions work correctly in a similar scenario.

BrianHung commented 1 year ago

Other array functions like VLOOKUP also break, as you noted.

alvarovillafane commented 1 year ago

@sequba COUNTIF also break

alvarovillafane commented 1 year ago

Hi @sequba , I was doing some implementation and testing, and the issue also appears and breaks when referencing a sheet in which whose content is not initialized just yet 🤔 .


const hfInstance = HyperFormula.buildEmpty({ licenseKey: "gpl-v3" });

const sheetOne = [""];
const sheetTwo = ["=COUNTIF(sheetThree!A:A, test)"];
const sheetThree = [""];

hfInstance.addSheet("sheetOne");
hfInstance.addSheet("sheetTwo");
hfInstance.addSheet("sheetThree");

hfInstance.setSheetContent(0, [sheetOne]);
hfInstance.setSheetContent(1, [sheetTwo]);
hfInstance.setSheetContent(2, [sheetThree]);

see https://codesandbox.io/s/mystifying-ardinghelli-o136jr?file=/src/index.js:42-448

Any alternative option to make this work? because the workaround using '' on the sheetThree it doesn't work 🤔

Thanks.

BrianHung commented 1 year ago

Suspending evaluation works as a workaround: https://codesandbox.io/s/unruffled-silence-3zs70q?file=/src/index.js And using buildFromSheets also works: https://codesandbox.io/s/wonderful-shannon-fsnxfc?file=/src/index.js

Although yeah, I think hfInstance.setSheetContent(1, [sheetTwo]) should first #REF out and then be valid after hfInstance.setSheetContent(2, [sheetThree]).

alvarovillafane commented 1 year ago

Thanks @BrianHung , the issue with those workarounds is that I initialise the sheets dynamically as they load, and don't have all the content at the start. So can't suspend evaluation/buildFromSheets easily.

alvarovillafane commented 1 year ago

Just noticed that if I initialized the sheets before with an empty string, it also works as a workaround, instead of using suspend/resume evaluation.

There is any drawback on doing it like this?

import HyperFormula from "hyperformula";

const hfInstance = HyperFormula.buildEmpty({
  licenseKey: "gpl-v3"
});

const sheetOne = [""];
const sheetTwo = ["=sheetOne!A1", '=COUNTIF(sheetThree!X:X, "test")'];
const sheetThree = [""];

hfInstance.addSheet("sheetOne");
hfInstance.addSheet("sheetTwo");
hfInstance.addSheet("sheetThree");

hfInstance.setSheetContent(0, [[""]]);
hfInstance.setSheetContent(1, [[""]]);
hfInstance.setSheetContent(2, [[""]]);

hfInstance.setSheetContent(0, [sheetOne]);
hfInstance.setSheetContent(1, [sheetTwo]);
hfInstance.setSheetContent(2, [sheetThree]);

console.log(hfInstance.getAllSheetsSerialized());
console.log(hfInstance.getAllSheetsValues());

https://codesandbox.io/s/keen-taussig-58hfqf?file=/src/index.js:337-453

AMBudnik commented 1 year ago

@vmalvaro I also confirm it is working, and as it does not seem to do anything else than use the official API, it does not seem to be harmful in any way.

gittysachin commented 1 year ago

The workaround doesn't work perfectly @sequba.

When there are no rows to insert in the sheet, and we need to use the COUNTA function on a column, it gives incorrect results if the column contains null or empty string values. In such cases, if we intend to accurately count the empty cell values in the column, the workaround will yield incorrect results. It's important to note that similar issues may also arise with other aggregation functions when using the workaround.

=COUNTA("sheetOne!A:A") will return 1 but it should return 0 logically as there are no rows.

Can we please prioritise and resolve this issue in the upcoming release?

AMBudnik commented 1 year ago

Hi @gittysachin

In order to ensure a timely release, it is crucial to address the remaining two issues within the current release cycle, which is planned to conclude at the end of May. It is important to note that your reported issue will be thoroughly considered for inclusion in one of the upcoming releases. We understand the significance of adhering to the release schedule and are committed to delivering a product of the highest quality that meets your expectations. Your patience and understanding during this process are greatly appreciated.

sequba commented 1 year ago

Increasing impact to High

gittysachin commented 1 year ago

Thanks for the quick update, @sequba @AMBudnik. Eagerly looking forward to the resolution!

AMBudnik commented 9 months ago

Another great news @BrianHung This issue is also fixed in v 2.6.1!

CC @gittysachin @alvarovillafane