Možná chyba v cachování sloupců u dotazů v Nette\Database

duke

Narazil jsem na podivné chování algoritmu cachování sloupců v Nette\Database.

Uvažme následující presenter:

class TestPresenter extends BasePresenter
{
	/** @var Model\UsersModel */
	private $usersModel;

	public function injectUsersModel(Model\UsersModel $usersModel)
	{
		$this->usersModel = $usersModel;
	}

	public function renderFoo()
	{
		$recordValues = iterator_to_array($this->usersModel->findById(1)); // array
	}

	public function renderBar()
	{
		$record = $this->usersModel->findById(1); // ActiveRow
	}
}

Metoda UsersModel::findById pak vypadá takto:

public function findById($id)
{
	return $this->database->table('users')->wherePrimary($id)->fetch();
}

Uvažme dále, že usersModel obsahuje záznam s id 1.

Když provedeme akci Test:foo poprvé, položí se následující dotazy:

SHOW FULL COLUMNS FROM `users`;
SELECT * FROM `users` WHERE (`id` = 1)

a následná opakování akce Test:foo budou pokládat jediný dotaz:

SELECT * FROM `users` WHERE (`id` = 1)

Nyní když provedeme akci Test:bar, položí se dotaz:

SELECT * FROM `users` WHERE (`id` = 1)

a následná opakování akce Test:bar budou pokládat dotaz:

SELECT `id` FROM `users` WHERE (`id` = 1)

Až sem je to nejspíš v pořádku. Ale teď když se vrátíme k akci Test:foo, budou se pokládat dva dotazy:

SELECT `id` FROM `users` WHERE (`id` = 1);
SELECT * FROM `users` WHERE (`id` = 1)

… a na tom už nezmění žádná další volání akcí Test:foo ani Test:bar. Mám za to, že toto je chyba, a že Test:foo by mělo volat jediný dotaz (a pokud někdy zavolá dotazů více, mělo by to vést k úpravě cachovaných dat tak, aby se v budoucnu volal jediný dotaz).

hrach

Toto je vcelku umyslne chovani, ktere jaksi neni chybou. Dokonce portovane z NotOrmu. Otazkou zustava, jestli to ma logiku, uz jsem nad tim take premyslel.

Proste volani iteratoru nad ActiveRow nacte znovu vsechny sloupce. (Pokud nebyly nacteny napoprve). Ma to vcelku i logiku, pokud chci iterovat nad konkretnim resultem, tak me zrejme zajimaji vsechny sloupce. Vzhledem k tomu, ze data jsou uz fetchla, tak se nelze vyhnout druhemu dotazu. Resenim pro konkretni usecase muze byt pouziti ->select('*').

duke

Chápu, že volání iteratoru nad ActiveRow načte znovu všechny sloupce (pokud nebyly dříve načteny). V tom ale problém nevidím. Ten vidím už v tom prvním fetchi, kde mám za to, že se nemá načítat pouze sloupec id, ale rovnou všechny sloupce (protože při minulém spuštění skriptu se ukázalo, že je zapotřebí načíst všechny sloupce). Je nějaký důvod proč to trvá na tom, že se bude nejdříve dotazovat jen na sloupec id?

hrach

No, jde samozrejme o to, ze takove jedno volani iteratoru by pro vsechny dotazy stejne struktury prakticky znehodnotilo cachovani sloupcu, protoze by vsude selectovalo *. To je imo zasadni nevyhoda mozne opravy, a z tohoto duvodu mi prijde korektnejsi stavajici chovani. Pouhe pridani select(*) by to totiz spravne resilo.

enumag

@hrach: Tohle velmi úzce souvisí s tímto problémem. Osobně nepoužívám ActiveRow jako iterátor ani nepoužívám toArray takže můj názor je v tomto případě nepodstatný.

duke

hrach napsal(a):

No, jde samozrejme o to, ze takove jedno volani iteratoru by pro vsechny dotazy stejne struktury prakticky znehodnotilo cachovani sloupcu, protoze by vsude selectovalo *. To je imo zasadni nevyhoda mozne opravy, a z tohoto duvodu mi prijde korektnejsi stavajici chovani. Pouhe pridani select(*) by to totiz spravne resilo.

Pokud by bylo cachování sloupců vázané na Presenter:action (což zjevně teď není, neboť v mnou uvedeném příkladu ovlivňuje akce Test:bar cachování i pro Test:foo), tak by ono znehodnocení nemuselo být tak zásadní.

K onomu znehodnocování dochází i teď. Když si jediná akce libovolného presenteru v aplikaci sáhne ke všem sloupcům (přímo, nikoli přes iterátor), budou dotazy pokládané ze všech akcí všech ostatních presenterů požadovat tyto sloupce. Pokud si k nim sáhne přes iterátor či přes toArray (jak uvádí @enumag), nedojde sice ke „znehodnocení“, ale akce používající iterátor či toArray budou zbytečně volat více dotazů.

Nebylo by lepší cachování vázat k aktuálnímu requestu, tj. alespoň podle Presenter:action? A nebylo by potom lepší zajistit, aby generování (nebo ještě lépe až procházení) iterátoru bylo ekvivalentní z hlediska cachování sloupců ke jmenovitému přístupu ke všem sloupcům (to teď není, protože když nahradím iterování za tento jmenovitý přístup ke všem sloupcům, žádné dotazy navíc se volat nebudou, jen dojde k onomu znehodnocení cachování pro všechny ostatní akce).

enumag

@duke: Vázat cache databázové vrstvy na aktuální request je už z principu nesmysl. Do budoucna by NDB měla být samostatný balíček tj. aby ji bylo možné používat i bez ostatních částí Nette – tedy třeba i Nette\Application. Asi jediná závislost bude na Nette\Caching.

Současné chování iterator_to_array a metody toArray je mi připadá nelogické, ale jak už jsem řekl, ani jedno nepoužívám takže můj názor by zde neměl mít velkou váhu.

duke

enumag napsal(a):

@duke: Vázat cache databázové vrstvy na aktuální request je už z principu nesmysl. Do budoucna by NDB měla být samostatný balíček tj. aby ji bylo možné používat i bez ostatních částí Nette – tedy třeba i Nette\Application. Asi jediná závislost bude na Nette\Caching.

Myslím, že soudíš příliš rychle. Uvědom si k čemu vlastně cachování sloupců slouží. Slouží k tomu, aby se kladly dotazy s co nejmenším možným počtem sloupců. A pokud budeš mít v aplikaci např. 10 presenterů a v nich řekněme celkem 30 akcí, které budou klást podobné dotazy, stačí ti v současné době jedna jediná akce jednoho z těchto prestenterů, která bude chtít všechny sloupce (tak, že si pro ně jmenovitě sáhne), a celé slavné cachování sloupců pro tento dotaz je v podstatě na nic. Přitom by klidně daná optimalizace mohla fungovat, kdyby cachování bylo vázané na Presenter:action.

Z hlediska principu samostatnosti máš samozřejmě pravdu v tom, že by tato závislost na requestu (ale obecně na čemkoliv vnějším) neměla být do Nette\Database zadrátovaná natvrdo, ale měla by tam být možnost specifikovat kontext pro cachování a tuto možnost by právě šlo využít např. pro vložení kontextu Presenter:action. Cachování sloupců by tak mohlo být více efektivní.

hrach

Ad zavislost na requestu/ – no, neni to tak uplne od veci, mam nejakou ideu,jak by to slo snad efektivne resit.

Ad iterator/ – mozna nejkorektnejsim resenim bude to, ze proste iterator bude vracet to, co je vyselektovane. Je to vcelku i logicteji a neni v tom tolik magie, jako ted…

duke

hrach napsal(a):

Ad iterator/ – mozna nejkorektnejsim resenim bude to, ze proste iterator bude vracet to, co je vyselektovane. Je to vcelku i logicteji a neni v tom tolik magie, jako ted…

Souhlasím, že iterátor by měl vracet jen to, co je vyselektované. Tato změna sama o sobě však nejspíš neřeší problém, na který jsem upozornil (tj. zbytečný dotaz navíc). Nicméně je možné, že způsob, kterým to chceš řešit, vyřeší i tento problém.

Jde o to, aby se v případě nespecifikování selectu po iterování iterátoru do cache uložila informace, že pro daný dotaz je třeba selectovat všechny sloupce.

enumag

@duke: Řeší. Ten dotaz navíc se provádí protože se dotahují sloupce které v danou chvíli řádek nemá.