cakephp icon indicating copy to clipboard operation
cakephp copied to clipboard

Value wont bind() with Translation Behavior (shadow strategy)

Open PaulHendriks opened this issue 4 years ago • 2 comments

This is a:

  • [x] bug

  • [ ] enhancement

  • [ ] feature-discussion (RFC)

  • CakePHP Version: 4.2.8

  • Platform and Target: Apache2, 10.4.18-MariaDB, PHP 7.4.6

What i did

I am trying to implement Translation Behavior with shadow strategy https://book.cakephp.org/4/en/orm/behaviors/translate.html#shadow-table-strategy Which works fine, until i add a MATCH AGAINST expression with bind():

$nodes = $this->Nodes->find('translations')
            ->where(['MATCH (nodes.segment) AGAINST (:segment IN NATURAL LANGUAGE MODE)'])
            ->bind(':segment', $segment, 'string')
            ->limit(50);

What happened

This is the stacktrace:

2021-08-30 06:29:59 Error: [PDOException] SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near ':segment IN NATURAL LANGUAGE MODE) GROUP BY Nodes.id LIMIT 50) Nodes ON Node...' at line 1 in C:\workspace\POC\TM\app\vendor\cakephp\cakephp\src\Database\Statement\MysqlStatement.php on line 39

This is the query that was generated:

FROM nodes_translations NodesTranslations
INNER JOIN (
	SELECT (Nodes.id)
	FROM nodes Nodes
	WHERE MATCH (nodes.segment) AGAINST (:segment IN NATURAL LANGUAGE MODE)
	GROUP BY Nodes.id
	LIMIT 50) Nodes ON NodesTranslations.id = (Nodes.id)

Which runs without error in a querybrowser.

What I expected to happen

:segment to be bound.

        $nodes = $this->Nodes->find('all')
            ->contain(['NodesTranslations'])
            ->where(['MATCH (nodes.segment) AGAINST (:segment IN NATURAL LANGUAGE MODE)'])
            ->bind(':segment', $segment, 'string')
            ->limit(50);

Does not produce an error.

PaulHendriks avatar Aug 30 '21 08:08 PaulHendriks

The select loader will assign a new empty value binder to the subquery:

https://github.com/cakephp/cakephp/blob/f667f708baeaf700bb7f3934fe2cc75c240d77bf/src/ORM/Association/Loader/SelectLoader.php#L405

Interestingly the test for this doesn't seem to fail when not doing this. Seemingly because the function expression now uses param instead of c (which it was supposed to do already back then, but there was a bug) for the placeholder names, thus the conflict that was originally fixed by using a new binder is not being triggered anymore.

The problem slips in when the compiler tries to inherit bindings from the subquery:

https://github.com/cakephp/cakephp/blob/f667f708baeaf700bb7f3934fe2cc75c240d77bf/src/Database/QueryCompiler.php#L112-L121

Here it will overwrite values in the cloned binder with values from the main query's original binder, in case the SQL for the subquery has matching placeholders. Original :c0 held the function expression argument, now that won't happen anymore, as the value bound for the function is now :param0, and no such placeholder can be found in the subquery SQL, as the original fields are not being selected.

The problem however still exists, adding for example 'name !=' => 'foobar' to the conditions will cause the test to fail again when not using a new value binder, as now it's :c1 that will get overwritten (should be foobar, but will be mariano).

Long story short, it's a delicate process held together by chewing gum and sewing cotton, and I don't want to touch it, but maybe my findings are helpful for somebody else who wants to look into this 😛

ps. as a workaround you can try chaning the association strategy:

$table->addBehavior('Translate', [
    // ...
    'strategy' => \Cake\ORM\Association::STRATEGY_SELECT,
]);

ndm2 avatar Aug 30 '21 13:08 ndm2

Thank you for the elaborate reply @ndm2 :-) The workaround you mentioned works.

PaulHendriks avatar Aug 31 '21 06:08 PaulHendriks