Bug 146377 - Error doesn't propagate as first parameter in IF when "IfFalse" contains ISERROR or IFERROR
Summary: Error doesn't propagate as first parameter in IF when "IfFalse" contains ISER...
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium normal
Assignee: Eike Rathke
URL:
Whiteboard: target:7.4.0 target:7.3.0.2 target:7....
Keywords:
Depends on:
Blocks: Calc-Function
  Show dependency treegraph
 
Reported: 2021-12-22 17:16 UTC by Paul den Hollander
Modified: 2022-01-01 14:28 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Paul den Hollander 2021-12-22 17:16:59 UTC
Description:
Consider these 2 formulas:
=IF(NA();1;2)
=IF(NA();1;IFERROR(NA();2))
The NA() propagates in the first formula as expected, and the result it #N/A.
In the second formula, the NA() somehow doesn't propagate. It evalutes the "IfFalse" parameter, and gives that as result. The second formula evaluates to 2. Even if the error in the "IfFalse" parameter is unrelated, it will still not propagate the condition error, e.g.:
=IF(NA();1;2+IFERROR(#DIV/0!;2))
This evaluates to 4.

From what I've read in the OpenDocument recalculated formula specification, errors as function parameters should propagate unless noted otherwise, and in this case I don't think it's noted otherwise.

At first I tried asking about it on the "ask libreoffice" website, but it seems like a bug, so posting it here seems more appropriate.

Steps to Reproduce:
1. Have an error value in the first parameter of an IF, and use IFERROR or ISERROR  in the 3rd parameter of the IF, e.g.: =IF(NA();1;IFERROR(NA();2))

Actual Results:
The "IfFalse" parameter is evaluted.

Expected Results:
The error value propagates.


Reproducible: Always


User Profile Reset: Yes



Additional Info:
Version: 7.2.4.1 / LibreOffice Community
Build ID: 20(Build:1)
CPU threads: 16; OS: Linux 5.14; UI render: default; VCL: kf5 (cairo+xcb)
Locale: nl-NL (en_US.UTF-8); UI: en-US
7.2.4-1
Calc: threaded
Comment 1 Werner Tietz 2021-12-23 12:46:45 UTC
Hallo
it looks like `IFERROR` in second Formula "controls" the whole evaluating-chain, no matter its own position, regardless the very uncommon Formula I would call it a bug.
Comment 2 Paul den Hollander 2021-12-23 15:50:28 UTC
(In reply to Werner Tietz from comment #1)
> Hallo
> it looks like `IFERROR` in second Formula "controls" the whole
> evaluating-chain, no matter its own position, regardless the very uncommon
> Formula I would call it a bug.

Yes, I've tested IFERROR in other formulas, and unexpected things happen there too. So it's not just in IF.
An example:
=AND(NA();1)
=AND(NA();IFERROR(NA();1))
Result of first formula is #N/A, result of second is Err:504.

Basically IFERROR can affect the handling of errors that do not originate in it's parameters. This also seems to happen with the IS* functions, like ISTEXT, ISNA, etc. E.g.: =AND(NA();IF(ISNUMBER(1);1)) Results in Err:504
Comment 3 Paul den Hollander 2021-12-23 16:53:38 UTC
I am trying to achieve full compatibility compatibility between my own tool that evaluates formulas and LibreOffice Calc, so I'm trying to understand what happens. 

So after looking at the LibreOffice Calc source code, this is how I think it behaves:
Example: =IF(NA();"a";ISERROR(NA()))
ScInterpreter::ScIfJump looks at the condition, which sets ScInterpreter::nGlobalError to the error, but continues execution with a 0 value in place of the error.
Execution of ScInterpreter::ScIsError then resets ScInterpreter::nGlobalError to FormulaError::NONE.
After the formula has been evaluated, ScInterpreter::GetCellValue checks ScInterpreter::nGlobalError, which would normally be set to the error, but now has already been reset, so it just uses the execution result.

Is this correct?
Comment 4 Eike Rathke 2021-12-25 18:26:06 UTC
Quite..

While the NA() propagates the error, as a condition it never evaluates to TRUE (that's probably what you see as "continues execution with a 0 value"), so the IF()'s else-path is taken that correctly evaluates ISERROR(NA()) to TRUE. The ISERROR() function (like other IS...() error evaluating functions) clears the current nGlobalError because it must not be propagated but the result of the function be pushed instead.

I don't see this (error leading to false-path) as a wrong behaviour, though one might argue that already the IF(NA();...) should bail out and execute not even the false-path. Which btw matters only if it contains an error evaluating function.

Gnumeric seems to agree and returns #N/A.

What does Excel do for this specific case?


(In reply to Paul den Hollander from comment #2)
> An example:
> =AND(NA();1)
> =AND(NA();IFERROR(NA();1))
> Result of first formula is #N/A, result of second is Err:504.
> 
> Basically IFERROR can affect the handling of errors that do not originate in
> it's parameters. This also seems to happen with the IS* functions, like
> ISTEXT, ISNA, etc. E.g.: =AND(NA();IF(ISNUMBER(1);1)) Results in Err:504

Of course the presence of functions that may evaluate error conditions affects until what point an expression is interpreted, that's expected. However, the AND() function (and OR() as well) lacks handling/propagation of error values. It discards unexpected arguments, including error values, and pushes its own Err:504 value. That doesn't happen with =AND(NA();1) because that already doesn't even call AND() as the NA() error would persist.
Comment 5 Mike Kaganski 2021-12-25 22:06:13 UTC
(In reply to Eike Rathke from comment #4)
> What does Excel do for this specific case?

Both:

=IF(NA();1;IFERROR(NA();2))
=AND(NA();IFERROR(NA();1))

result in #N/A.
Comment 6 Commit Notification 2021-12-25 22:22:28 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-7-3":

https://git.libreoffice.org/core/commit/eb050ea5b004e2f4a6ba96474177251f678707e9

Related: tdf#146377 Let AND(), OR(), XOR() propagate the current error, if any

It will be available in 7.3.0.2.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 7 Commit Notification 2021-12-26 08:02:27 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

https://git.libreoffice.org/core/commit/07e58ea08758018906c08321591d264f85a24e66

Related: tdf#146377 Let AND(), OR(), XOR() propagate the current error, if any

It will be available in 7.2.6.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 8 Commit Notification 2021-12-26 15:21:52 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/1c14b9efb0677dea65ff220222fbb8d5c2aa6973

Resolves: tdf#146377 Propagate condition error of IF(condition)

It will be available in 7.4.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 9 Commit Notification 2021-12-26 16:48:16 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "libreoffice-7-3":

https://git.libreoffice.org/core/commit/439814c48b3194398e7abe97a855193dc3e28f12

Resolves: tdf#146377 Propagate condition error of IF(condition)

It will be available in 7.3.0.2.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 10 Commit Notification 2021-12-27 13:55:37 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/2a5b87316625d3f5cd735f01376c263dc40e7c81

tdf#146377: sc_logical_functions_test: Add unittest

It will be available in 7.4.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 11 Stéphane Guillou (stragu) 2022-01-01 14:28:24 UTC
Verified as fixed in:

Version: 7.3.0.1.0+ / LibreOffice Community
Build ID: df00367b8fe5d6b495c8e568f1ec732992e86d07
CPU threads: 4; OS: Linux 5.4; UI render: default; VCL: gtk3
Locale: en-AU (en_AU.UTF-8); UI: en-US
Calc: threaded