Skip to content
This repository was archived by the owner on Jun 23, 2021. It is now read-only.

Re #1692 prevent query repeats and not fetch rows which are not required #1

Open
zainengineer wants to merge 1 commit into
base: feature/hex-value
Choose a base branch
from
Open

Re #1692 prevent query repeats and not fetch rows which are not required #1

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 20 additions & 12 deletions app/code/community/JR/AttributeOptionImage/Model/Eav/Mysql4/Entity/Attribute/Option.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,36 @@ class JR_AttributeOptionImage_Model_Eav_Mysql4_Entity_Attribute_Option extends M
{
public function getAttributeOptionImages()
{
$select = $this->getReadConnection()
->select()
->from($this->getTable('eav/attribute_option'), array('option_id', 'image'));

return $this->getReadConnection()->fetchPairs($select);
return $this->getAttributeColumnValue('image');
}

public function getAttributeOptionThumbs()
{
$select = $this->getReadConnection()
->select()
->from($this->getTable('eav/attribute_option'), array('option_id', 'thumb'));

return $this->getReadConnection()->fetchPairs($select);
return $this->getAttributeColumnValue('thumb');
}

public function getAttributeOptionHex()
{
return $this->getAttributeColumnValue('hex');
}
public function getAttributeColumnValue($columnName)
{
/**
* Reason using static instead of $_property because resource is not singleton
* and query is called multiple times on same table with same arguments
* because multiple instances exist of that option
*/
static $return = array();
if (isset($return[$columnName])){
return $return[$columnName];
}

$select = $this->getReadConnection()
->select()
->from($this->getTable('eav/attribute_option'), array('option_id', 'hex'));
->from($this->getTable('eav/attribute_option'), array('option_id', $columnName))

@parhamr parhamr Nov 24, 2014

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This query deserves a LIMIT statement to reduce the scope and severity of catastrophically bad data. Something like LIMIT 1000 would provide a decent safeguard, if we can assume this should never return more than 1000 rows.

@zainengineer zainengineer Nov 24, 2014

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@parhamr next line ->where("$columnName != ''"); should limit the result.

If in any case it does not we need to investigate it further, as if config has hex value we need it in the category listing.

->where("$columnName != ''");

@parhamr parhamr Nov 24, 2014

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 great! I’ve seen a large eav_attribute_option table with many blank rows.


return $this->getReadConnection()->fetchPairs($select);
$return[$columnName] = $this->getReadConnection()->fetchPairs($select);
return $return[$columnName];
}
}