Opened 12 years ago
Last modified 2 years ago
#164 new Erweiterung
"strpos(strtoupper(" in /includes/classes/split_page_results.php
| Reported by: | Owned by: | somebody | |
|---|---|---|---|
| Priority: | niedrig | Milestone: | |
| Component: | Shop | Version: | 2.0.1.0 |
| Keywords: | Cc: | ||
| Blocked By: | Blocking: |
Description
Gibt es einen Grund dafür dass strpos(strtoupper statt einfach stripos( verwendet wird? Wenn nicht, würde ich vorschlagen, das der Übersichtlichkeit zu liebe auszutauschen.
Außerdem würde ich für diese drei Zeilen statt str(i)pos( vorschlagen strr(i)pos( zu verwenden:
$pos_group_by = strpos(strtoupper($this->sql_query), ' GROUP BY', $pos_from);
$pos_having = strpos(strtoupper($this->sql_query), ' HAVING', $pos_from);
$pos_order_by = strpos(strtoupper($this->sql_query), ' ORDER BY', $pos_from);
Also stattdessen:
$pos_group_by = strripos($this->sql_query, ' GROUP BY', $pos_from);
$pos_having = strripos($this->sql_query, ' HAVING', $pos_from);
$pos_order_by = strripos($this->sql_query, ' ORDER BY', $pos_from);
Der Grund: Wenn man in einer Produktlisting mal einen Subquery verwendet, der z.B. ein "ORDER BY" enthält, gibt das einen SQL-Fehler...
Attachments (0)
Change History (8)
comment:1 by , 12 years ago
| Milestone: | modified-shop-2.00 → modified-shop-2.10 |
|---|---|
| Version: | 1.06 → 2.10 |
comment:2 by , 8 years ago
| Milestone: | modified-shop-2.1.0.0 |
|---|
comment:3 by , 2 years ago
comment:4 by , 2 years ago
*NACHTRAG*
Zur Verdeutlichung: Die paginierte komplette Produkt-Liste besteht aus über 600.000 Artikeln.
comment:5 by , 2 years ago
Unabhängig vom ursprünglichen Ticket, bei dessen Umsetzung ich verstehen könnte, daß man sie erstmal ausgiebig testen wollen würde.
Das Cachen der Count-Query sollte imo in die nächste Version einfließen, da es nach meinen Tests eine erhebliche Performance-Verbesserung darstellen kann, nämlich wenn sich viele Artikel in einer Kat befinden.
Für's Backend könnte das ebenfalls gut sein, dort dann für all die Seiten die ein Paging haben.
Gruß,
noRiddle
comment:6 by , 2 years ago
wenn du den DB Cache anmachst, hast du fast das gleiche Ergebnis.
Deine Lösung Vorteil:
- es steht direkt in der Session und muss nicht aus dem Cache geholt werden
Deine Lösung Nachteil:
- bei Erstaufruf einer Kategorie oder Kategoriewechsel muss der Query immer ausgeführt werden
- das funktioniert immer nur auf Kundenebene
SVN Lösung Vorteil:
- sobald der Cache geschrieben wurde steht es für ALLE Kunden bzw. die komplette Kundengruppe zur Verfügung
- ein Kategoriewechsel ist irrelevant, da es im Cache und nicht in der Session steht
SVN Lösung Nachteil:
- auslesen aus dem Cache dauert minimal länger als aus der Session auslesen
Generell:
es gibt ein sehr gutes Cachingsystem im Shop. Wenn hier ein zusätzliches Cachingsystem unabhängig vom DB Cache gemacht werden soll, dann auf dessen Basis.
comment:7 by , 2 years ago
Nachtrag: ein Umbau mit einem zusätzlichen Caching macht keinen Sinn. Es ist bereits ein Caching vorhanden und es bringt an der Stelle keinen Vorteil.
comment:8 by , 2 years ago
Meine Lösung Nachteil:
- "bei Erstaufruf einer Kategorie oder Kategoriewechsel muss der Query immer ausgeführt werden": Das war vorher ja auch so, und das bei jedem Blättern.
- "das funktioniert immer nur auf Kundenebene": Korrekt
SVN Lösung Vorteil:
- Gut. Hatte vergessen, daß es ja den DB-Cache gibt.
(Daß ich in allen Shops die ich bisher gesehen habe im Betrieb keinen sichtbaren Unterschied gibt zwischen Cache on und off ist wohl subjektiv.)
strripos() anstatt strpos() und strtoupper() funktioniert übrigens hervorragend. Habe ich schon länger in Live-Umgebung am laufen.
Den Vorschlag könnte man soweit ich das sehe problemlos übernehmen (zusätzlich für $pos_from),
abgesehen davon, daß die split_page_results ohnehin einer Überarbeitung bedarf (siehe Ticket #764 und dazugehörigen Thread (split_page_results.php) und GTBs Aussage dazu (Antwort #4).
Sub-Queries dürfte man mit Marcus Kreuschs Vorschlag allerdings abgefangen haben.
Außerdem, habe ich gerade erfolgreich getestet (bei ausgeschaltetem DB-Cache), würde ein "Cachen" der COUNT-Query beim Weiterblättern das Paging erheblich verschnellern, denn dann würde sie nicht bei jedem Weiterblättern erneut ausgeführt werden:
Statt
$count_query = xtDBquery("SELECT count(" . $count_string . ") as total " . substr($query, $pos_from, ($pos_to - $pos_from))); $count = xtc_db_fetch_array($count_query, true); $this->number_of_rows = $count['total'];das
if(!isset($_SESSION['split_page_res_qu']) || (isset($_SESSION['split_page_res_qu']) && $_SESSION['split_page_res_qu'] != $query)) { $_SESSION['split_page_res_qu'] = $query; $count_query = xtDBquery("SELECT count(" . $count_string . ") as total " . substr($query, $pos_from, ($pos_to - $pos_from))); $count = xtc_db_fetch_array($count_query, true); $_SESSION['split_page_res_cnt'] = $reviews_count['total']; } $this->number_of_rows = $_SESSION['split_page_res_cnt';Was das andere, oben verlinkte, Ticket betrifft, dürfte eine Lösung damit gegeben sein, daß die COUNT-Query mit als Parameter an new splitPageResults(...) übergeben wird und somit immer im aufrufenden Skript definiert werden muß.
Das spart die ganzen "$pos_from"- und "$pos_to"-Berechnungen und somit auch für die COUNT-Query unnötige JOINS.
Habe ich ebenfalls mittels (echter) Klassenerweiterung mit einem weiteren Parameter $count_qu im Constructor getestet.
Mit dem Cachen des Count und dem letzen Vorschlag hier konnte ich die Ausgabe einer paginierten kompletten Produkt-Liste so stark beschleunigen, daß es schon unheimlich ist.
Gruß,
noRiddle