Rewrote the $select->setPart() usage because it looses the "where type"

[fballiano]

In this part of code

magento-lts/app/code/core/Mage/Catalog/Model/Resource/Layer/Filter/Price.php

Lines 118 to 128 in 15f83af

	// processing WHERE part
	$wherePart = $select->getPart(Zend_Db_Select::WHERE);
	$excludedWherePart = Mage_Catalog_Model_Resource_Product_Collection::MAIN_TABLE_ALIAS . '.status';
	foreach ($wherePart as $key => $wherePartItem) {
	if (strpos($wherePartItem, $excludedWherePart) !== false) {
	$wherePart[$key] = new Zend_Db_Expr('1=1');
	continue;
	}
	$wherePart[$key] = $this->_replaceTableAlias($wherePartItem);
	}
	$select->setPart(Zend_Db_Select::WHERE, $wherePart);

There's a weird hack (that 1=1 replacement) that doesn't look very nice but most of all the usage of the $select->setPart(Zend_Db_Select::WHERE, $wherePart); is wrong, I'll try to explain.

When doing $select->where("some condition") you're actually calling

magento-lts/app/code/core/Zend/Db/Select.php

Lines 1031 to 1052 in 15f83af

	protected function _where($condition, $value = null, $type = null, $bool = true)
	{
	if (count($this->_parts[self::UNION])) {
	#require_once 'Zend/Db/Select/Exception.php';
	throw new Zend_Db_Select_Exception("Invalid use of where clause with " . self::SQL_UNION);
	}
	if ($value !== null) {
	$condition = $this->_adapter->quoteInto($condition, $value, $type);
	}
	$cond = "";
	if ($this->_parts[self::WHERE]) {
	if ($bool === true) {
	$cond = self::SQL_AND . ' ';
	} else {
	$cond = self::SQL_OR . ' ';
	}
	}
	return $cond . "($condition)";
	}

where the $bool adds the "AND" where type.

With the current implementation this whole thing gets bypassed and the $select->setPart(Zend_Db_Select::WHERE, $wherePart); looses the where type, creating the problem described in #2658

This PR rewrites that small part of code removing the 1=1 hack which becomes useless, the code is more correct and looks better IMHO.

Also, you can see in the same file modified by this PR, just a few lines below

magento-lts/app/code/core/Mage/Catalog/Model/Resource/Layer/Filter/Price.php

Lines 129 to 136 in 15f83af

	$excludeJoinPart = Mage_Catalog_Model_Resource_Product_Collection::MAIN_TABLE_ALIAS . '.entity_id';
	foreach ($priceIndexJoinConditions as $condition) {
	if (strpos($condition, $excludeJoinPart) !== false) {
	continue;
	}
	$select->where($this->_replaceTableAlias($condition));
	}
	$select->where($this->_getPriceExpression($filter, $select) . ' IS NOT NULL');

it uses the same writing style that I'm using in this PR.

[fballiano]

I think this shouldn’t be closed, it’s bad code what’s in use now

[sreichel]

Was closed by github/my PR... just reopen.

[sreichel]

I think this shouldn’t be closed, it’s bad code what’s in use now

What's "bad"?

[fballiano]

It should be explained the in pr description

[leissbua]

I like it, make me a reviewer and i approve it.

[fballiano]

I think you can review it anyway, it won't be a green check but it's still very valuable, 1 green mark and 2 gray it's 100 approved

[elidrissidev]

I tested this now but it doesn't resolve the problem completely, I get a stray AND in the WHERE clause:

SELECT FLOOR((ROUND((e.min_price) * 1, 2)) / 100) + 1 AS `range`, COUNT(*) AS `count`
FROM `catalog_product_index_price` AS `e`
INNER JOIN `catalog_category_product_index` AS `cat_index` ON cat_index.product_id=e.entity_id AND cat_index.store_id='1' AND cat_index.visibility IN(3, 4) AND cat_index.category_id = '2'
WHERE (AND (e.entity_id IN(554, 442, 445, 441, 446, 398, 408, 409, 410))) AND ( e.website_id = '1' ) AND ( e.customer_group_id = 0) AND (e.min_price IS NOT NULL) GROUP BY FLOOR((ROUND((e.min_price) * 1, 2)) / 100) + 1 ORDER BY FLOOR((ROUND((e.min_price) * 1, 2)) / 100) + 1 ASC

You need to have "Use In Search Results Layered Navigation" set to "Yes" for the price attribute to generate this issue.

[fballiano]

isn't it possible that that problem comes from another site and it's not related to the modified method? cause, checking what i modified in this PR, it seems weird

[leissbua]

I guess i have to agree the ORM is to stupid so the dev choose to add the 1=1 before, so the AND does not become a problem. Need to work on this, I am going to check your solution in a customer project and report back.

[leissbua]

But if this happens due to ORM stupidity, and the reported error is continuous, why not just prepend 1=1 to the where part array in general. I check whats going on in the next few days.

[elidrissidev]

isn't it possible that that problem comes from another site and it's not related to the modified method? cause, checking what i modified in this PR, it seems weird

It actually makes sense, since getPart(Zend_Db_Select::WHERE) returns an array of conditions including their delimiter (AND, OR...), and where() method doesn't strip them off, so the fix is gonna be more complicated.

[fballiano]

I'll try to check it out ASAP, if you've ideas they're obviously more then welcome :-)

[fballiano]

I tested this now but it doesn't resolve the problem completely, I get a stray AND in the WHERE clause:

SELECT FLOOR((ROUND((e.min_price) * 1, 2)) / 100) + 1 AS `range`, COUNT(*) AS `count`
FROM `catalog_product_index_price` AS `e`
INNER JOIN `catalog_category_product_index` AS `cat_index` ON cat_index.product_id=e.entity_id AND cat_index.store_id='1' AND cat_index.visibility IN(3, 4) AND cat_index.category_id = '2'
WHERE (AND (e.entity_id IN(554, 442, 445, 441, 446, 398, 408, 409, 410))) AND ( e.website_id = '1' ) AND ( e.customer_group_id = 0) AND (e.min_price IS NOT NULL) GROUP BY FLOOR((ROUND((e.min_price) * 1, 2)) / 100) + 1 ORDER BY FLOOR((ROUND((e.min_price) * 1, 2)) / 100) + 1 ASC

You need to have "Use In Search Results Layered Navigation" set to "Yes" for the price attribute to generate this issue.

@elidrissidev I can't reproduce the problem you see, can you help me here?