User defined REVERSE() function Unicode support 🎉#453
Conversation
JanJakes
left a comment
There was a problem hiding this comment.
Thanks for working on this! I have some small suggestions and nitpicks. Otherwise, looks good!
| return null; | ||
| } | ||
| if ( preg_match( '/[^\x00-\x7F]/', $str ) ) { | ||
| preg_match_all( '/./u', $str, $matches ); |
There was a problem hiding this comment.
Let’s also add the s modifier so that . matches line feeds, and a test for that. Something like REVERSE('Hello\nWörld') should give dlröW\nolleH.
There was a problem hiding this comment.
Oh, and let's make sure that when the string has invalid UTF-8, we fallback to strrev. It would then become sth like this:
if (
preg_match( '/[^\x00-\x7F]/', $str )
&& preg_match_all( '/./us', $str, $matches )
) {
return implode( '', array_reverse( $matches[0] ) );
}There was a problem hiding this comment.
Thanks for catching that, it wasn't breaking until I checked the actual output and noticed it was silently dropping the newlines and just sticking lines together. And invalid utf8 / raw bytes returned nothing 😐. I've added your suggestions.
| * Method to emulate MySQL REVERSE() function. | ||
| * | ||
| * Takes a string and returns the reverse of it. | ||
| * Compatible with MySQL behaviour for utf8mb4 strings. |
There was a problem hiding this comment.
I guess this also applies to utf8mb3 so maybe let's just say sth like this:
* Reverse UTF-8 text by code point, matching MySQL behavior.There was a problem hiding this comment.
Fair. I babble sometimes I guess 😉.
… faster mb_str_split() if available
|
No worries @JanJakes, least I can do for using all of this. I've also taken the liberty of adding a check to see if |
The optimization offers little practical benefit for typical values while duplicating UTF-8 reversal across environments. Keep the PCRE implementation consistent across supported PHP versions and avoid dependence on mb_internal_encoding().
JanJakes
left a comment
There was a problem hiding this comment.
@teizz Thank you! I removed the mbstring part because it seems to be only modestly faster, and it adds more complexity (there was already a subtle bug in that mb_str_split didn't pass the UTF-8 parameter, and that would make it use mb_internal_encoding() instead). Otherwise all good, thanks!
As discussed with @JanJakes in pull request #434 I've gone ahead and added the code to support unicode characters while keeping behavior aligned with how mysql reverses a utf8mb4 unicode string, i.e. it correctly reverses multi-byte characters, but fails to preserve the correct order for grapheme clusters / combining characters.
As a practical example: both this implementation and mysql's REVERSE() function will reverse '🧟 🧟♂️ 🧟♀️' to become '♀🧟 ️♂🧟 🧟' instead of '🧟♀️ 🧟♂️ 🧟'.
I've also gone ahead and added a check to enable a fast path with
strrev()in case all characters are normal ASCII, this improves both speed and prevents additional allocation for arrays.Finally, I checked to see that
is actually about twice as fast as