Did you check this with phpcs? For example the uncommented private class variables should have been disallowed by it.

This should indicate the type of variable (as you did below), not the name.

Please be consistent in usage of return types

There should be no spaces between the name and the ()

You're checking if $end_pos is false above, it'll never be equal to strlen($string ) - 1 - $start_pos and even if it is, you're not using the result of the comparison.

null isn't allowed in the parameter type

I don't think this needs to be defined twice?

Why not null?

How does this find any digits? It's just checking string length right?

It doesn't seem to do this

Don't think this is needed. We can already get the query from other places.

You can combine these two, by checking whether the query canonicalization was already done ($this->canonical_query !== NULL).
I would also rename the method to get and make a __toString() magic method that calls it.
That way we can get the canonical query using

$canonical_query = (string) $result->canonical_query();

I don't think we need those wrappers.

I'm afraid, doing it like this isn't good.
Dependency Injection demands that we can somehow mock the canonical_query() method of the MySQLCanonicalQuery class. But we can't do that if class instantitation and method call are in the same location. So we have to split this.
The best scenario I could think of is if the just return the instantiated class here (still considering that we should only instantiate it on the first call). Combined with the __toString() comment I made above this should still make for a nice API.

Some things aren't in the codestyle apparently. We should make a list :D
There should be an empty line between the open tag and the file comment.

empty line between @param and @return

leftover test code

common practice for us is that we initialize the property in the constructor. So a simple

$this->canonical_query = NULL;


should be fine here.

I prefer a !== NULL check

Put the expected value in the data provider, even if it's always the same. Makes it easier to extend in the future in case we do find a situation where it isn't.

This shouldn't be needed. $this->class is supposed to be the class we're testing.

you can name the test scenarios by setting a string key, like

$data_provider['first argument not string'] = [['013456789013456789','23','56'], []];

Please change this to read input and expected output from test files, and add more possible scenarios.

There's two possible structure we typically use (in tests/statics/<namespace>/):

foo_input.sql and foo_output.sql
input/foo.sql and output/foo.sql

Whichever you prefer.
Additional cases here that'd be worth considering are INSERT, UPDATE, REPLACE queries. SHOW would also be nice. Perhaps upserts, CTEs, etc.

one test case per test :)

Try to avoid empty() since it'll also match things like 0, which you probably don't want here. is_null is a better check I think

missing param type

missing param types

this is equal to parent::tearDown() after you implement heinz' comments

Perhaps add an example for a query using maxscale query hints: https://mariadb.com/kb/en/mariadb-maxscale-14/maxscale-hint-syntax/

I'm not sure about this. On the one hand, it's nice to test that the code would support his.
On the other hand, we'd never get to a query like this. We don't support either running multiple queries at once nor do we support fetching results for multiple queries issued at once.

This is where things get tricky I suppose.
Are these two queries canonically different?

INSERT INTO `database`.`table` (`param1`, `param2`, `param3`) VALUES (?, '?', '?'), (?, '?', '?'), (?, '?', '?'), (?, '?', '?'), (?, '?', '?');



INSERT INTO `database`.`table` (`param1`, `param2`, `param3`) VALUES (?, '?', '?'), (?, '?', '?'), (?, '?', '?'), (?, '?', '?');

You need to handle the case where $offset is an empty array (if I understood the code in find_positions correctly).
This can be the case in INSERT/REPLACE statements that don't use VALUES, like
INSERT INTO table SELECT FROM table2

I think it would be a better test if we had an actual value here in the WHERE clause :-)

this is the same row, but will return failed

Ship It!

Branch not pushed

Unit tests are failing:

There was 1 error:

1) Lunr\Gravity\Database\MySQL\Tests\MySQLQueryResultBaseTest::testCanonicalQuery
Undefined property: Lunr\Gravity\Database\MySQL\Tests\MySQLQueryResultBaseTest::$result_reflection

/mnt/progs/projects/m2mobi/backend/lunr/src/Lunr/Gravity/Database/MySQL/Tests/MySQLQueryResultBaseTest.php:182

--

There was 1 warning:

1) Warning
No tests found in class "Lunr\Gravity\Database\MySQL\Tests\MySQLCanonicalQueryTest".

--

There were 2 failures:

1) Lunr\Gravity\Filesystem\Tests\PhysicalFilesystemAccessObjectListDirectoriesTest::testGetListOfDirectoriesInAccessibleDirectory
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 'folder1'
-    1 => 'folder2'
+    0 => 'Database'
+    1 => 'folder1'
+    2 => 'folder2'
 )

/mnt/progs/projects/m2mobi/backend/lunr/src/Lunr/Gravity/Filesystem/Tests/PhysicalFilesystemAccessObjectListDirectoriesTest.php:40

2) Lunr\Gravity\Filesystem\Tests\PhysicalFilesystemAccessObjectDirectoryTest::testGetListingOfAccessibleDirectory
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
-    0 => 'file1'
-    1 => 'file2'
-    2 => 'file3'
-    3 => 'folder1'
-    4 => 'folder2'
+    0 => 'Database'
+    1 => 'file1'
+    2 => 'file2'
+    3 => 'file3'
+    4 => 'folder1'
+    5 => 'folder2'
 )

/mnt/progs/projects/m2mobi/backend/lunr/src/Lunr/Gravity/Filesystem/Tests/PhysicalFilesystemAccessObjectDirectoryTest.php:41

When I rebase the latest code in the branch against master I still get:

There was 1 error:

1) Lunr\Gravity\Database\MySQL\Tests\MySQLQueryResultBaseTest::testCanonicalQuery
Undefined property: Lunr\Gravity\Database\MySQL\Tests\MySQLQueryResultBaseTest::$result_reflection