Bug 163736 - XMATCH produce wrong results
Summary: XMATCH produce wrong results
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
24.8.2.1 release
Hardware: All All
: medium normal
Assignee: Balázs Varga (allotropia)
URL:
Whiteboard: target:25.2.0 target:24.8.4 target:24...
Keywords:
Depends on:
Blocks: Calc-Function
  Show dependency treegraph
 
Reported: 2024-11-02 10:21 UTC by Werner Tietz
Modified: 2024-11-08 10:57 UTC (History)
5 users (show)

See Also:
Crash report or crash signature:


Attachments
example.ods wich shows the issue (19.26 KB, application/vnd.oasis.opendocument.spreadsheet)
2024-11-02 10:23 UTC, Werner Tietz
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Werner Tietz 2024-11-02 10:21:03 UTC
Description:
XMATCH especially with reversed search-direction produce wrong results.
worse enough, if you use the Formulawizard it shows the correct result … but changes its »mind« later on recalculation with ctrl+shift+F9

Steps to Reproduce:
1. open attached …ods
2. select cell E12
3. switch to Formula-Wizard
4. see the (correct) return »16«
5. close the wizard ⇒ E12 shows »16« #right!
6. hit ctrl+shift+enter ⇒ E12 shows »4« #wrong 

Actual Results:
see #6. 

Expected Results:
the correct result should persist!


Reproducible: Always


User Profile Reset: No

Additional Info:
with me:
__________
Version: 24.8.2.1 (AARCH64) / LibreOffice Community
Build ID: 0f794b6e29741098670a3b95d60478a65d05ef13
CPU threads: 4; OS: Linux 6.6; UI render: default; VCL: gtk3
Locale: de-DE (de_DE.UTF-8); UI: de-DE
Flatpak
Calc: threaded

see also: https://ask.libreoffice.org/t/xmatch-and-match-in-neighbour-cells/113282/5
Comment 1 Werner Tietz 2024-11-02 10:23:46 UTC
Created attachment 197354 [details]
example.ods wich shows the issue
Comment 2 Regina Henschel 2024-11-02 19:01:44 UTC
Please try with a new user profile.
I get the wrong value with one user profile, but not with another user profile.

Tested with Version: 25.2.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 9517639bc3189e3ea4dc4d2f7004d4b33d754d47
CPU threads: 32; OS: Windows 11 X86_64 (10.0 build 22631); UI render: Skia/Vulkan; VCL: win
Locale: de-DE (de_DE); UI: de-DE
Calc: threaded
Comment 3 m_a_riosv 2024-11-02 22:55:40 UTC
Even starting in Safe Mode, I can reproduce the issue.
Version: 25.2.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 665dce4442e48b133b9fe1a2eb792ed3ef81d90c
CPU threads: 16; OS: Windows 11 X86_64 (10.0 build 26100); UI render: default; VCL: win
Locale: es-ES (es_ES); UI: es-ES
Calc: threaded
Comment 4 Werner Tietz 2024-11-03 09:02:43 UTC
(In reply to Regina Henschel from comment #2)
> Please try with a new user profile.

happens also in »save mode« or otherwise from commandline with additional:
 »-env:UserInstallation=file:///…/…«

Important:
!!! The Error __doesnt__ occur if you remove the Formula in Cell D12 !!!
Comment 5 Regina Henschel 2024-11-03 14:26:34 UTC
(In reply to Regina Henschel from comment #2)
> I get the wrong value with one user profile, but not with another user
> profile.
One recalculates on opening, the other not. So it was not the profile but recalculating.

The error occurs when the formula cells are in the same row, not only when they are adjacent.

@Balázs: Any idea about the reason?
Comment 6 Regina Henschel 2024-11-03 23:44:09 UTC
The use of ScLookupCache looks suspicious. ScLookupCache knows nothing about forward vs. revers search direction.

Not only XMATCH but XLOOKUP is effected the same way.

Solutions? Do not use ScLookupCache at all in XMATCH and XLOOKUP? Extend QueryOp with values for revers search?
Comment 7 Balázs Varga (allotropia) 2024-11-04 07:14:07 UTC
(In reply to Regina Henschel from comment #6)
> Solutions? Do not use ScLookupCache at all in XMATCH and XLOOKUP? Extend
> QueryOp with values for revers search?

Yes we can use it, just had to extend the ScLookupCache with the search mode since it can give back different results with different search modes.
https://gerrit.libreoffice.org/c/core/+/175972
Comment 8 Regina Henschel 2024-11-04 13:37:02 UTC
(In reply to Balázs Varga (allotropia) from comment #7)
> Yes we can use it, just had to extend the ScLookupCache with the search mode
> since it can give back different results with different search modes.
> https://gerrit.libreoffice.org/c/core/+/175972

The patch works for me, so I have given a +1.

But I see a general problem: For VLOOKUP nSearchMode is 0. So static_cast<SearchMode>(nSearchMode)
casts to a value which does not exist in SearchMode enum. That gives an "undefined behavior". That problem is not only with the new enum in lookupcache.hxx but already in interpr1.cxx before this patch.

But I'm no C++ expert and my concerns may be unfounded.
Comment 9 Commit Notification 2024-11-04 13:52:20 UTC
Balazs Varga committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/223696267759326af4e436e0330026e4ff8ee8e0

tdf#163736 - sc: fix searchmode cached values for lookup functions

It will be available in 25.2.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 10 Balázs Varga (allotropia) 2024-11-04 14:08:59 UTC
(In reply to Regina Henschel from comment #8)
> (In reply to Balázs Varga (allotropia) from comment #7)
> > Yes we can use it, just had to extend the ScLookupCache with the search mode
> > since it can give back different results with different search modes.
> > https://gerrit.libreoffice.org/c/core/+/175972
> 
> The patch works for me, so I have given a +1.
> 
> But I see a general problem: For VLOOKUP nSearchMode is 0. So
> static_cast<SearchMode>(nSearchMode)
> casts to a value which does not exist in SearchMode enum. That gives an
> "undefined behavior". That problem is not only with the new enum in
> lookupcache.hxx but already in interpr1.cxx before this patch.
> 
> But I'm no C++ expert and my concerns may be unfounded.

Ohh I just saw this comment. Didn't know that we used 0, here: https://opengrok.libreoffice.org/xref/core/sc/source/core/tool/interpr1.cxx?r=40468f0e#7828 for the VLOOKUP and HLOOKUP. If the VLOOKUP and HLOOKUP can only do normal forward search, we can just use 1=SearchMode::searchfwd in this function call: bFound = LookupQueryWithCache( aResultPos, aParam, refData, 0, SC_OPCODE_V_LOOKUP ); instead of 0.
Comment 11 Commit Notification 2024-11-05 16:49:53 UTC
Balazs Varga committed a patch related to this issue.
It has been pushed to "master":

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

Related: tdf#163736 - sc: Fix wrong nSearchMode (0) in vlookup

It will be available in 25.2.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 12 Commit Notification 2024-11-06 19:01:58 UTC
Balazs Varga committed a patch related to this issue.
It has been pushed to "libreoffice-24-8":

https://git.libreoffice.org/core/commit/7dcfb8417f82be1fb496702afb4d0f54cb74839f

tdf#163736 - sc: fix searchmode cached values for lookup functions

It will be available in 24.8.4.

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 13 Commit Notification 2024-11-06 19:02:01 UTC
Balazs Varga committed a patch related to this issue.
It has been pushed to "libreoffice-24-8":

https://git.libreoffice.org/core/commit/72b027af1148ef57ee3c167f5a9e768831b61d43

Related: tdf#163736 - sc: Fix wrong nSearchMode (0) in vlookup

It will be available in 24.8.4.

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 14 Commit Notification 2024-11-08 10:57:19 UTC
Balazs Varga committed a patch related to this issue.
It has been pushed to "libreoffice-24-8-3":

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

tdf#163736 - sc: fix searchmode cached values for lookup functions

It will be available in 24.8.3.

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 15 Commit Notification 2024-11-08 10:57:22 UTC
Balazs Varga committed a patch related to this issue.
It has been pushed to "libreoffice-24-8-3":

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

Related: tdf#163736 - sc: Fix wrong nSearchMode (0) in vlookup

It will be available in 24.8.3.

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.