Skip to content

Conversation

@jmarble
Copy link

@jmarble jmarble commented Oct 26, 2025

Fixed transitivity violation in zendi_smart_strcmp() that caused sort() and array_unique() to produce incorrect results with mixed numeric and alphanumeric strings.

When both strings have numeric prefixes, the comparison now consistently uses numeric comparison, with trailing data as a tie-breaker when the numeric values are equal.

The bug occurred because the comparison function would:

  • Compare "5" vs "10" numerically (5 < 10)
  • Compare "10" vs "3A" lexically ("1" < "3")
  • Compare "5" vs "3A" lexically ("5" > "3")

This violated transitivity (if a<b and b<c, then a<c must hold), causing sort() to fail to group equal values and array_unique() to miss duplicates.

The fix ensures that when both strings have numeric prefixes (even with trailing data), numeric comparison is always used. When numeric values are equal, strings without trailing data are considered "less than" strings with trailing data as a tie-breaker.

This maintains backward compatibility while ensuring comparison transitivity.

Fixed transitivity violation in zendi_smart_strcmp() that caused sort()
and array_unique() to produce incorrect results with mixed numeric and
alphanumeric strings.

When both strings have numeric prefixes, the comparison now consistently
uses numeric comparison, with trailing data as a tie-breaker when the
numeric values are equal.
The order of some of the files in the output was incorrect. This commit
corrects the test to match the expected order.
@jmarble
Copy link
Author

jmarble commented Oct 26, 2025

I'm not sure why the mbstring test failed in CI everywhere but FREEBSD. The test passes for me locally:
https://gist.github.com/jmarble/d9afea199fbeab310f9b3dac20f5823b

@jmarble jmarble marked this pull request as draft October 27, 2025 00:33
@jmarble jmarble closed this Oct 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant