Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add REGEXP_REPLACE support (including unit test) #120

Open
wants to merge 10 commits into
base: develop
Choose a base branch
from
30 changes: 30 additions & 0 deletions tests/WP_SQLite_Translator_Tests.php
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,36 @@ public static function regexpOperators() {
);
}

public function testRegexpReplace() {
/* Testing if an actual replacment works correctly */
$this->assertQuery(
"INSERT INTO _options (option_name, option_value) VALUES ('test-ignore', '1');"
);
$this->assertQuery(
"INSERT INTO _options (option_name, option_value) VALUES ('test-remove', '2');"
);
$this->assertQuery( "SELECT * FROM _options WHERE REGEXP_REPLACE(option_name, '(-ignore|-remove)', '') = 'test'" );
$this->assertCount( 2, $this->engine->get_query_results() );

/* If one of the required parameters is null, the return value is null, copying the MYSQL/MariaDB behavior */
$this->assertQuery( "SELECT REGEXP_REPLACE( null, 'a', 'x') as result" );
Zodiac1978 marked this conversation as resolved.
Show resolved Hide resolved
$results = $this->engine->get_query_results();
$this->assertEquals( null, $results[0]->result );

$this->assertQuery( "SELECT REGEXP_REPLACE( 'abc', null, 'x') as result" );
$results = $this->engine->get_query_results();
$this->assertEquals( null, $results[0]->result );

$this->assertQuery( "SELECT REGEXP_REPLACE( 'abc', 'a', null) as result" );
$results = $this->engine->get_query_results();
$this->assertEquals( null, $results[0]->result );

/* Providing an empty pattern should produce an error - but we changed that to null to avoid breaking things */
$this->assertQuery( "SELECT REGEXP_REPLACE( 'abc', '', 'x') as result" );
$results = $this->engine->get_query_results();
$this->assertEquals( null, $results[0]->result );
}
Zodiac1978 marked this conversation as resolved.
Show resolved Hide resolved

public function testInsertDateNow() {
$this->assertQuery(
"INSERT INTO _dates (option_name, option_value) VALUES ('first', now());"
Expand Down
49 changes: 49 additions & 0 deletions wp-includes/sqlite/class-wp-sqlite-pdo-user-defined-functions.php
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ public function __construct( $pdo ) {
'isnull' => 'isnull',
'if' => '_if',
'regexp' => 'regexp',
'regexp_replace' => 'regexp_replace',
'field' => 'field',
'log' => 'log',
'least' => 'least',
Expand Down Expand Up @@ -493,6 +494,54 @@ public function regexp( $pattern, $field ) {
return preg_match( $pattern, $field );
}

/**
* Method to emulate MySQL REGEXP_REPLACE() function.
*
* @param string|array $pattern Regular expression to search for (or array of strings).
* @param string|array $replacement The string or an array with strings to replace.
* @param string|array $field Haystack.
*
* @return Array if the field parameter is an array, or a string otherwise.
*/
public function regexp_replace( $field, $pattern, $replacement ) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just checked and in MySQL REGEXP_REPLACE() also takes three optional parameters:

pos: The position in expr at which to start the search. If omitted, the default is 1.
occurrence: Which occurrence of a match to replace. If omitted, the default is 0 (which means “replace all occurrences”).
match_type: A string that specifies how to perform matching. The meaning is as described for REGEXP_LIKE().

Would you be up for adding a support for those? If not, that's fine, but let's at add them to the function signature and fail with an informative warning (REGEXP_REPLACE() don't support non-default values for $pos (fourth argument), .... if you need it then you can contribute here: ....) if they're set to non-default values.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you be up for adding a support for those?

I will try to do that!

/*
* If the original query says REGEXP BINARY
* the comparison is byte-by-byte and letter casing now
* matters since lower- and upper-case letters have different
* byte codes.
*
* The REGEXP function can't be easily made to accept two
* parameters, so we'll have to use a hack to get around this.
*
* If the first character of the pattern is a null byte, we'll
* remove it and make the comparison case-sensitive. This should
* be reasonably safe since PHP does not allow null bytes in
* regular expressions anyway.
*/
Comment on lines +507 to +520
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does regexp_replace support the BINARY keyword? I couldn't find anything. If it doesn't, let's remove this comment and the special casing for the null byte.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the binary-mode something global?
See: https://mariadb.com/docs/server/ref/mdb/cli/mariadb/binary-mode/


/* Return null if one of the required parameter is null */
if ( is_null( $field ) || is_null( $pattern ) || is_null( $replacement ) ) {
return null;
}

/* Return null if the pattern is empty - this changes MySQL/MariaDB behavior! */
if ( empty( $pattern ) ) {
return null;
}
Comment on lines +527 to +530
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! Wha is the MySQL/MariaDB behavior? to just use an empty pattern? If so, let's do that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is throwing an error (because without pattern it can't work at all):

Error in the SQL query (3685): Illegal argument to a regular expression.

@bgrgicak was suggesting to return null instead to avoid breaking things, I assume.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Null seemed like a better response from an error. @Zodiac1978 let's update the comment to make it clear what this changes MySQL/MariaDB behavior means.


if ( "\x00" === $pattern[0] ) {
$pattern = substr( $pattern, 1 );
$flags = '';
} else {
// Otherwise, the search is case-insensitive.
$flags = 'i';
}
$pattern = str_replace( '/', '\/', $pattern );
$pattern = '/' . $pattern . '/' . $flags;

return preg_replace( $pattern, $replacement, $field );
}

/**
* Method to emulate MySQL FIELD() function.
*
Expand Down
Loading