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

3.1.4 - Pagination doesn't like when you sort a virtual field while containing something else using subquery #7669

Closed
patrickconroy opened this issue Nov 9, 2015 · 9 comments
Assignees
Labels
Milestone

Comments

@patrickconroy
Copy link
Contributor

Sorry for the long title. Between 3.1.1 and 3.1.2, an issue came up with paginating. Say I want to add a 'searchScore' field to a record dynamically when searching, but I also want to contain the found record using the subquery strategy..

$v = TableRegistry::get('Videos');
$v->hasMany('_i18n', [
    'foreignKey' => 'id',
    'className' => 'VideosTranslations',
    'strategy' => 'subquery'
]);
$this->paginate = [
    'Videos' => [
        'limit' => 20,
        'order' => ['searchScore' => 'desc'],
        'sortWhitelist' => ['searchScore']
    ]
];
$videos = $this->paginate(
    $v
        ->find()
        ->select(['searchScore' => 100])
        ->contain(['_i18n'])
);
pr($videos->toArray());

If that hasMany is a select, it's fine. If it's subquery, I get:
Column not found: 1054 Unknown column 'searchScore' in 'order' clause
This started in 7df65e6...9f90a04

Thanks

@dereuromark dereuromark added this to the 3.1.5 milestone Nov 9, 2015
@lorenzo
Copy link
Member

lorenzo commented Nov 9, 2015

What is the generated SQL?

@patrickconroy
Copy link
Contributor Author

In 3.1.2..

Using subquery:

SELECT 100 AS `searchScore`, `Videos`.* FROM `videos` `Videos` ORDER BY `searchScore` desc LIMIT 20 OFFSET 0
SELECT `_i18n`.`id` AS `_i18n__id`, `_i18n`.`locale` AS `_i18n__locale`, `_i18n`.`title` AS `_i18n__title`, `_i18n`.`description` AS `_i18n__description`, `_i18n`.`youtube_id` AS `_i18n__youtube_id` FROM `videos_translations` `_i18n` INNER JOIN (SELECT (`Videos`.`id`) FROM `videos` `Videos` GROUP BY `Videos`.`id`  ORDER BY `searchScore` desc LIMIT 20 OFFSET 0) `Videos` ON `_i18n`.`id` = (`Videos`.`id`)

Using select:

SELECT 100 AS `searchScore`, `Videos`.* FROM `videos` `Videos` ORDER BY `searchScore` desc LIMIT 20 OFFSET 0
SELECT `_i18n`.`id` AS `_i18n__id`, `_i18n`.`locale` AS `_i18n__locale`, `_i18n`.`title` AS `_i18n__title`, `_i18n`.`description` AS `_i18n__description`, `_i18n`.`youtube_id` AS `_i18n__youtube_id` FROM `videos_translations` `_i18n` WHERE `_i18n`.`id` in (1,2,3,4,5,6,7,8,9,10,12,13,14,15,16,17,18,20,31,34)

@patrickconroy
Copy link
Contributor Author

I updated the original comment, it started in 3.1.2, not 3.1.3.
In 3.1.1 it's fine.

@patrickconroy
Copy link
Contributor Author

In 3.1.1 (where it's ok), the SQL is..

Subquery:

SELECT `_i18n`.`id` AS `_i18n__id`, `_i18n`.`locale` AS `_i18n__locale`, `_i18n`.`title` AS `_i18n__title`, `_i18n`.`description` AS `_i18n__description`, `_i18n`.`youtube_id` AS `_i18n__youtube_id` FROM `videos_translations` `_i18n` INNER JOIN (SELECT (`Videos`.`id`) FROM `videos` `Videos` GROUP BY `Videos`.`id` ) `Videos` ON `_i18n`.`id` = (`Videos`.`id`)

Select:

SELECT 100 AS `searchScore`, `Videos`.* FROM `videos` `Videos` ORDER BY `searchScore` desc LIMIT 20 OFFSET 0
SELECT `_i18n`.`id` AS `_i18n__id`, `_i18n`.`locale` AS `_i18n__locale`, `_i18n`.`title` AS `_i18n__title`, `_i18n`.`description` AS `_i18n__description`, `_i18n`.`youtube_id` AS `_i18n__youtube_id` FROM `videos_translations` `_i18n` WHERE `_i18n`.`id` in (1,2,3,4,5,6,7,8,9,10,12,13,14,15,16,17,18,20,31,34)

@lorenzo
Copy link
Member

lorenzo commented Nov 9, 2015

Ok, so the difference is that the order clause is now preserved in the subquery?

@patrickconroy
Copy link
Contributor Author

Yeah I think that's it really. Should it be there?

@lorenzo
Copy link
Member

lorenzo commented Nov 9, 2015

No, I don't think so... I wonder if this was a side effect of the deep cloning changes @markstory did

@markstory
Copy link
Member

@lorenzo That seems like the most reasonable cause. I can take a look as there is enough here to work from.

@markstory markstory self-assigned this Nov 10, 2015
@markstory
Copy link
Member

So I took at look at this, and the failure is very likely caused by the cloning work I did. However, the original code also has an issue. The generated subqueries do not include ordering. This means that if the root query is ordered by a column, the loaded associations will joined onto a potentially different result set. For example if you had ordered your videos by upload_date the loaded associations would not be correct.

While I have a fix for the reported issue, I think its important to fix the other issue with subquery loading.

markstory added a commit that referenced this issue Nov 11, 2015
When eager loading associations that contain an ORDER, we need to
augment the selected fields to include the fields being ordered by. If
we were to remove the ORDER BY, incorrect association data would be
loaded. If we were to leave the fields alone, we'd have problems with
postgres. Instead, if we find fields in the ORDER BY, that also exist in
the column list, those fields should be preserved in the generated
subquery.

Refs #7669
@lorenzo lorenzo closed this as completed Nov 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants