diff --git a/src/Adapter/MySQL.php b/src/Adapter/MySQL.php index 8af448ce0..f84103f2e 100644 --- a/src/Adapter/MySQL.php +++ b/src/Adapter/MySQL.php @@ -380,8 +380,10 @@ protected function computeOrderByField(array $filterToTableMapping) return $orderField; } - // If we have an initial population, add the field into initial population selects, so we can use it in the outer query for sorting - if ($this->getInitialPopulation() !== null) { + // If we have an initial population, add the field into initial population selects, so we can use it in the outer query for sorting. + // An expression is skipped: it is evaluated against the outer query's joins, which the initial population does not have, + // so selecting it there refers to tables that are not joined at that level. + if ($this->getInitialPopulation() !== null && strpos($orderField, '(') === false) { $this->getInitialPopulation()->addSelectField($orderField); } diff --git a/src/Filters/Products.php b/src/Filters/Products.php index ab0083842..cac0767bf 100644 --- a/src/Filters/Products.php +++ b/src/Filters/Products.php @@ -71,6 +71,18 @@ public function getProductByFilters( $orderWay = Validate::isOrderWay($orderWay) ? $orderWay : 'ASC'; $orderBy = Validate::isOrderBy($orderBy) ? $orderBy : 'position'; + // A product has one position per category it belongs to. When the listing spans a subtree + // (PS_LAYERED_FULL_TREE), the rows are restricted with nleft/nright rather than to the browsed + // category, so `position` is not functionally dependent on the product the query groups by and + // the value picked is whichever row the server happens to keep. Pin the ordering to the browsed + // category; a product reached only through a subcategory has no position here and is listed after + // the ones the merchant did arrange. + if ($orderBy === 'position' && $query->getIdCategory()) { + $positionInCategory = 'MIN(IF(cp.id_category = ' . (int) $query->getIdCategory() . ', cp.position, NULL))'; + $this->searchAdapter->addSelectField('position'); + $orderBy = 'ISNULL(' . $positionInCategory . ') ASC, ' . $positionInCategory; + } + // Apply it to the filter $this->searchAdapter->setOrderField($orderBy); $this->searchAdapter->setOrderDirection($orderWay); diff --git a/tests/php/FacetedSearch/Adapter/MySQLTest.php b/tests/php/FacetedSearch/Adapter/MySQLTest.php index 716c160f6..7cef5cc25 100644 --- a/tests/php/FacetedSearch/Adapter/MySQLTest.php +++ b/tests/php/FacetedSearch/Adapter/MySQLTest.php @@ -70,6 +70,30 @@ protected function setUp() Configuration::setStaticExpectations($configurationMock); } + /** + * A product has one position per category, so ordering a subtree listing by position has to + * aggregate over the browsed category. The expression is evaluated against the outer query's + * joins, so it must not be selected in the initial population, which does not have them. + */ + public function testGetQueryOrderedByAnExpressionKeepsItOutOfTheInitialPopulation() + { + $this->adapter->useFiltersAsInitialPopulation(); + $this->adapter->addFilter('nleft', [9], '>='); + $this->adapter->addFilter('nright', [14], '<='); + $this->adapter->addGroupBy('id_product'); + $this->adapter->addSelectField('position'); + $this->adapter->setOrderField('ISNULL(MIN(IF(cp.id_category = 6, cp.position, NULL))) ASC, MIN(IF(cp.id_category = 6, cp.position, NULL))'); + $this->adapter->setOrderDirection('asc'); + + $query = $this->adapter->getQuery(); + $innerQuery = substr($query, strpos($query, '(') + 1, strrpos($query, ') p INNER JOIN') - strpos($query, '(') - 1); + + $this->assertNotContains('cp.id_category', $innerQuery, 'The initial population does not join category_product, so it cannot select an expression using it.'); + $this->assertNotContains('ps_category_product', $innerQuery, 'The expression is read in the outer query, so the initial population does not need the join at all.'); + $this->assertContains(') p INNER JOIN ps_category_product cp ON (p.id_product = cp.id_product)', $query, 'The outer query joins category_product, which is what the expression reads.'); + $this->assertContains('ORDER BY ISNULL(MIN(IF(cp.id_category = 6, cp.position, NULL))) ASC, MIN(IF(cp.id_category = 6, cp.position, NULL)) ASC, p.id_product DESC', $query); + } + public function testGetEmptyQuery() { $this->assertEquals(