MATCH Problems with Int/Float Compare and Wildcards

[oleibman]

Fix #3141. Function matchSmallestValue did not recognize that an integer could match a float. Adding test cases, it seems that matchFirstValue had the same problem. However, matchLargestValue seemed to handle things correctly - but see below.

In addition, the wildcard logic in matchFirstValue is faulty. It ignored tilde as a wildcard character. Although it would have been easy to just add that, I think it was wrong to determine on its own if a wildcard was in use. Just using the already available wildcard functions whenever comparing two strings is sufficient.

I note that Excel doesn't seem to follow its own rules for MATCH (https://support.microsoft.com/en-us/office/match-function-e8dffd45-c762-47d6-bf89-533f4a37673a?ns=excel&version=90&syslcid=1033&uilcid=1033&appver=zxl900&helpid=xlmain11.chm60112&ui=en-us&rs=en-us&ad=us). PhpSpreadsheet's results match Excel's, so no problem. However, when match_type is not zero, the match array is supposed to be sorted, so I would expect #N/A when it isn't; but that's not how Excel operates. I have no idea what Excel is doing. If MATCH(2,{2,0,4,3},1) isn't #N/A because of the unsorted array, then surely it should be 1 (item 1 of the array is the largest number less than or equal to the lookup value); but Excel and PhpSpreadsheet (before and after changes) return 2. I have moved this example to be the first of the test cases. And, if you have a theory for why this works way (after all PhpSpreadsheet can match it), try it with MATCH(5,{8,6,3,1},1), and see if your theory works for the Excel result of #N/A (which PhpSpreadsheet doesn't duplicate).

One would think strings would behave similarly. But, no - see the second test case. This time Excel does look for an exact match. But the existing logic doesn't get the matching result in PhpSpreadsheet. It requires a whole new block of code, one which doesn't work correctly for numeric lookup value. Ugh.

LibreOffice doesn't always agree with Excel. It seems that it will use wildcard matching even when the match type is not zero (Excel documentation says wildcards are only for type zero, which is just as well because I don't really know what greater/less mean when wildcards are involved). This behavior is now duplicated in PhpSpreadsheet. For the record, Gnumeric agrees with Excel here.

This is:

- [x] a bugfix
- [ ] a new feature
- [ ] refactoring
- [ ] additional unit tests

Checklist:

• Changes are covered by unit tests
  • Changes are covered by existing unit tests
  • New unit tests have been added
• Code style is respected
• Commit message explains why the change is made (see https://github.com/erlang/otp/wiki/Writing-good-commit-messages)
• CHANGELOG.md contains a short summary of the change
• Documentation is updated as necessary

Why this change is needed?

Provide an explanation of why this change is needed, with links to any Issues (if appropriate).
If this is a bugfix or a new feature, and there are no existing Issues, then please also create an issue that will make it easier to track progress with this PR.

[oleibman]

No intention to act on Scrutinizer "complexity" warnings.

[oleibman]

@MarkBaker I seem to have opened up a can of worms here (I should have paid more attention to your "not straightforward" comment). I think more test cases are probably needed. In your comments on the original issue, you said "we can't then use the same comparison for numerics and non-numeric strings" - I think my code, but not necessarily the tests, are taking care of that. Do you have additional test cases to suggest?

[MarkBaker]

The most obvious additional tests would be using numeric strings (containing both '1.0' float and '1' int values) for the input value

[MarkBaker]

Do we need an is not numeric check here?

[oleibman]

I don't think so, although this is another confusing aspect of MATCH. If you look at the test to which I added the label 'string compared to number type -1', numeric 6 does not seem to match string '6', and that test returned #N/A both before and after this change, as does Excel. I will add more tests.

[oleibman]

Another surprise (which will require a code change) - Excel accepts anything numeric for match type, paying attention only to the sign - floats, numeric strings, values other than 0/1/-1. Also, non-numeric strings for match type will generate VALUE error, not NA.