Pagination generating invalid SQL query when association uses a custom finder.

[ah8r]

This is a (multiple allowed):

• bug
• enhancement
• feature-discussion (RFC)
• CakePHP Version: 3.2.9
• Platform and Target: Nginx / MySQL

What you did

Firstly, I originally reported this in another issue but it was closed as not a bug (#8876 (comment)). However after debugging on the CakePHP forum and in the Slack channel, I was convinced to report it again, because it does appear that the CakePHP Paginator is generating invalid SQL.

I have simplified the setup to help identify the cause of the error. Firstly, here are the tables I am using: database.sql.txt

I have three models, Jobs, Users, and UserTypes.

The associations are:

Users belongsToMany UserTypes
UserTypes belongsToMany Users
Jobs belongsTo Users

However, for the Jobs belongsTo Users association, I am using a custom finder from the UsersTable.php file:

public function findManagers(Query $query, array $options)
{
    return $query->matching('UserTypes', function ($q) {
        return $q->where([
            'UserTypes.name' => 'Manager'
        ]);
    });
}

In short, this should only allow Jobs to belong to Users who have a UserType where the name is "Manager". In the example database I uploaded, this would only apply to User 2.

Attached are the JobsController.php.txt, JobsTable.php.txt, UsersTable.php.txt, and UserTypesTable.php.txt

All these files were generated using cake bake. The UsersTable.php file has been modified to add the custom finder, and the JobsController.php file has been modified so that the users list in the add() / edit() functions uses the custom finder.

Expected Behavior

When visiting /jobs the jobs in the database should be listed using the Paginator as is default. When visiting the /jobs/add page, the drop down box for "Users" should only display the Users with a UserType of "Manager". The same should apply for /jobs/edit/X.

Actual Behavior

When visiting the /jobs/add and /jobs/edit/x pages, the application functions normally. The drop down box containing the users only shows the user who has the "Manager" UserType (User 2).

When visiting the /jobs page however, provided there is a job in the table, an error is thrown:

Database Error
PDOException

Error: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'UserTypes.name' in 'on clause'

The stack trace can be found here: stack.txt

On the advice of a user in the Slack channel, I hijacked the /vendor/cakephp/cakephp/src/Database/Statement/MysqlStatement.php file and got it to output all queries. These can be found here: queries.txt

The one in particular to note, and which I believe is the cause of the issue, is this one:

SELECT (COUNT(*)) AS `count` FROM jobs Jobs INNER JOIN users Users ON (UserTypes.name = :c0 AND Users.id = (Jobs.user_id)) INNER JOIN user_types UserTypes ON UserTypes.name = :c1 INNER JOIN users_user_types UsersUserTypes ON (Users.id = (UsersUserTypes.user_id) AND UserTypes.id = (UsersUserTypes.user_type_id))

Specifically, the first join is Jobs on Users, but in the ON clause it references the UserTypes table.

Given that I am using code from cake bake, and am following the CakePHP book when it comes to defining associations with custom finders, the fact that this SQL is generated incorrectly appear to me to be a bug in CakePHP and not as a result of my code.

My apologies if this is not the case, but I have been looking into this issue for a couple of days now and have not found any other reason why it could be happening, and the support channel did suggest making this bug report.

[markstory]

I'd agree this is not expected/desired results. The count() query has additional join conditions that the non-count() query does not have.

[lorenzo]

I'm confused on how I should reproduce this... Why

Users hasMany UserTypes and UserTypes hasMany Users ??

Should it not be Users belongsTo UsersTypes ? Or maybe Users belongsToMany UsersTypes ?

[ah8r]

@lorenzo,

My apologies, when I wrote up the bug I wrote "hasMany" rather than "belongsToMany". The code examples I attached (UsersTable and UserTypesTable) are correct.

To clarify, the association between Users and UserTypes is many to many using a join table (users_user_types).

In UsersTable.php:

$this->belongsToMany('UserTypes', [
    'foreignKey' => 'user_id',
    'targetForeignKey' => 'user_type_id',
    'joinTable' => 'users_user_types'
]);

In UserTypesTable.php:

$this->belongsToMany('Users', [
    'foreignKey' => 'user_type_id',
    'targetForeignKey' => 'user_id',
    'joinTable' => 'users_user_types'
]);

[Spriz]

I ran into the same issue as of today with 3.1.12 - details found here: https://gist.github.com/Spriz/e9b9d9d172e44aca53a5a31f929918cb - I couldn't figure out where that extra line (https://gist.github.com/Spriz/e9b9d9d172e44aca53a5a31f929918cb#file-count-sql-L6) comes from so can't fix it in Pagination's paginate() method

[Spriz]

Just tried altering the code in the PaginatorComponent here:

https://github.com/cakephp/cakephp/blob/master/src/Controller/Component/PaginatorComponent.php#L173 to:

debug($query);
try {
    $count = $numResults ? $query->count() : 0;
} catch (\Exception $e) {
    debug($e);
    die;
}

and this is the resulting SQLs

SELECT * FROM forms Forms 
INNER JOIN form_fields FormFields ON (content_value >= :c0 AND content_value <= :c1 AND Forms.id = (FormFields.form_id) AND (FormFields.deleted) IS NULL) 
INNER JOIN form_field_types FormFieldTypes ON (FormFieldTypes.identifier = :c2 AND FormFieldTypes.id = (FormFields.form_field_type_id) AND (FormFieldTypes.deleted) IS NULL) 
WHERE (Forms.Company_Id = :c3 AND (Forms.deleted) IS NULL) ORDER BY Forms.created desc LIMIT 20 OFFSET 0

SELECT (COUNT(*)) AS `count` FROM forms Forms 
INNER JOIN form_fields FormFields ON (content_value >= :c0 AND content_value <= :c1 AND FormFieldTypes.identifier = :c2 AND Forms.id = (FormFields.form_id) AND (FormFields.deleted) IS NULL) 
INNER JOIN form_field_types FormFieldTypes ON (FormFieldTypes.identifier = :c3 AND FormFieldTypes.id = (FormFields.form_field_type_id) AND (FormFieldTypes.deleted) IS NULL) 
WHERE (Forms.Company_Id = :c4 AND (Forms.deleted) IS NULL)

so I guess it's something in https://github.com/cakephp/cakephp/blob/master/src/ORM/Query.php#L775 that's bugging me, but I can't figure out what exactly.

[Spriz]

@ah8r did you eventually get around this?

Managed to narrow down to this is the place where it gets altered and the extra statement is inserted by doing debug($clone->sql()); just before/after. Well, I did actually use debugger, but you can validate by putting debug()s in.

https://github.com/cakephp/cakephp/blob/master/src/ORM/Query.php#L728

[ah8r]

@Spriz I did eventually get around it to some degree, but it requires quite a few changes to the Models.

Since the custom finder doesn't work, instead I reverted to a regular Jobs belongsTo Manager, and I added in a validation check to prevent non-Manager users from being associated with Jobs:

// JobsTable.php

public function initialize(array $config)
{
    $this->belongsTo('Managers', [
        'className' => 'Users',
        'foreignKey' => 'manager_id',
        'joinType' => 'INNER'
    ]);
    ...
}

public function validationDefault(Validator $validator)
{
    $validator
        ->requirePresence('manager_id', 'create')
        ->notEmpty('manager_id')
        ->add('manager_id', 'validManager', [
            'rule' => 'isManager',
            'message' => 'Manager is invalid.',
            'provider' => 'table'
    ]);
    ...
}

public function isManager($value, array $context)
{
    $users = TableRegistry::get('Users');

    $managers = $users->find('all')->distinct('Users.id')->innerJoinWith('UserTypes', function ($q) {
        return $q->where([
            'UserTypes.name' => 'Managers'
        ]);
    })->where([
        'Users.id' => $value
    ]);

    if ($managers->count() == 1)
    {
        return true;
    }

    return false;
}

When loading in the list of managers in the add/edit functions of the controller, I just add an innerJoinWith to ensure that they are actually Managers.

This means only users who are managers can be associated with Jobs via the application. Of course, the downside is that you can edit the manager_id in the database and change it to a non-manager user, and the application would still load that user when you contain "Managers" on the job. For practical purposes it doesn't matter that much though.