From 78cb9f793a8eaf61d6435fb87dfb68c95dc65556 Mon Sep 17 00:00:00 2001 From: Romain Quetiez Date: Fri, 30 Nov 2012 13:34:46 +0000 Subject: [PATCH] Optimization of SQL queries: reduce the number of JOINS, assuming that data are consistent. Can be disabled with config setting query_optimization_enabled => 0. Also fixed caching issue (reproduced when replaying a query log) SVN:trunk[2485] --- core/config.class.inc.php | 16 ++++++ core/expression.class.inc.php | 51 ++++++++++++++++- core/metamodel.class.php | 41 ++++++++++---- core/sqlquery.class.inc.php | 103 +++++++++++++++++++++++++++++++--- test/replay_query_log.php | 2 +- 5 files changed, 192 insertions(+), 21 deletions(-) diff --git a/core/config.class.inc.php b/core/config.class.inc.php index f485d6122..e8be33615 100644 --- a/core/config.class.inc.php +++ b/core/config.class.inc.php @@ -133,6 +133,22 @@ class Config 'source_of_value' => '', 'show_in_conf_sample' => false, ), + 'query_optimization_enabled' => array( + 'type' => 'bool', + 'description' => 'The queries are optimized based on the assumption that the DB integrity has been preserved. By disabling the optimization one can ensure that the fetched data is clean... but this can be really slower or not usable at all (some queries will exceed the allowed number of joins in MySQL: 61!)', + 'default' => true, + 'value' => true, + 'source_of_value' => '', + 'show_in_conf_sample' => false, + ), + 'query_indentation_enabled' => array( + 'type' => 'bool', + 'description' => 'For developpers: format the SQL queries for human analysis', + 'default' => false, + 'value' => true, + 'source_of_value' => '', + 'show_in_conf_sample' => false, + ), 'graphviz_path' => array( 'type' => 'string', 'description' => 'Path to the Graphviz "dot" executable for graphing objects lifecycle', diff --git a/core/expression.class.inc.php b/core/expression.class.inc.php index 83a284fc0..7aeb3dc59 100644 --- a/core/expression.class.inc.php +++ b/core/expression.class.inc.php @@ -42,6 +42,9 @@ abstract class Expression // recursively builds an array of class => fieldname abstract public function ListRequiredFields(); + // recursively list field parents ($aTable = array of sParent => dummy) + abstract public function CollectUsedParents(&$aTable); + abstract public function IsTrue(); // recursively builds an array of [classAlias][fieldName] => value @@ -144,6 +147,10 @@ class SQLExpression extends Expression { return array(); } + + public function CollectUsedParents(&$aTable) + { + } public function ListConstantFields() { @@ -260,7 +267,12 @@ class BinaryExpression extends Expression $aRight = $this->GetRightExpr()->ListRequiredFields(); return array_merge($aLeft, $aRight); } - + + public function CollectUsedParents(&$aTable) + { + $this->GetLeftExpr()->CollectUsedParents($aTable); + $this->GetRightExpr()->CollectUsedParents($aTable); + } /** * List all constant expression of the form = or = : @@ -351,6 +363,10 @@ class UnaryExpression extends Expression return array(); } + public function CollectUsedParents(&$aTable) + { + } + public function ListConstantFields() { return array(); @@ -452,6 +468,11 @@ class FieldExpression extends UnaryExpression return array($this->m_sParent.'.'.$this->m_sName); } + public function CollectUsedParents(&$aTable) + { + $aTable[$this->m_sParent] = true; + } + public function GetUnresolvedFields($sAlias, &$aUnresolved) { if ($this->m_sParent == $sAlias) @@ -711,6 +732,14 @@ class ListExpression extends Expression return $aRes; } + public function CollectUsedParents(&$aTable) + { + foreach ($this->m_aExpressions as $oExpr) + { + $oExpr->CollectUsedParents($aTable); + } + } + public function ListConstantFields() { $aRes = array(); @@ -814,6 +843,14 @@ class FunctionExpression extends Expression return $aRes; } + public function CollectUsedParents(&$aTable) + { + foreach ($this->m_aArgs as $oExpr) + { + $oExpr->CollectUsedParents($aTable); + } + } + public function ListConstantFields() { $aRes = array(); @@ -932,6 +969,10 @@ class IntervalExpression extends Expression return array(); } + public function CollectUsedParents(&$aTable) + { + } + public function ListConstantFields() { return array(); @@ -1020,6 +1061,14 @@ class CharConcatExpression extends Expression return $aRes; } + public function CollectUsedParents(&$aTable) + { + foreach ($this->m_aExpressions as $oExpr) + { + $oExpr->CollectUsedParents($aTable); + } + } + public function ListConstantFields() { $aRes = array(); diff --git a/core/metamodel.class.php b/core/metamodel.class.php index f3d0eff24..dc1f49065 100644 --- a/core/metamodel.class.php +++ b/core/metamodel.class.php @@ -240,6 +240,8 @@ abstract class MetaModel private static $m_bQueryCacheEnabled = false; private static $m_bTraceQueries = false; + private static $m_bIndentQueries = false; + private static $m_bOptimizeQueries = false; private static $m_aQueriesLog = array(); private static $m_bLogIssue = false; @@ -2082,7 +2084,8 @@ abstract class MetaModel $aScalarArgs = array_merge(self::PrepareQueryArguments($aArgs), $oFilter->GetInternalParams()); try { - $sRes = $oSelect->RenderGroupBy($aScalarArgs); + $bBeautifulSQL = self::$m_bTraceQueries || self::$m_bDebugQuery || self::$m_bIndentQueries; + $sRes = $oSelect->RenderGroupBy($aScalarArgs, $bBeautifulSQL); } catch (MissingQueryArgument $e) { @@ -2135,13 +2138,14 @@ abstract class MetaModel } $oSelect = self::MakeSelectStructure($oFilter, $aOrderBy, $aArgs, $aAttToLoad, $aExtendedDataSpec, $iLimitCount, $iLimitStart, $bGetCount); + $oSelect = unserialize(serialize($oSelect)); $aScalarArgs = array_merge(self::PrepareQueryArguments($aArgs), $oFilter->GetInternalParams()); try { - $bBeautifulSQL = self::$m_bTraceQueries || self::$m_bDebugQuery; + $bBeautifulSQL = self::$m_bTraceQueries || self::$m_bDebugQuery || self::$m_bIndentQueries; $sRes = $oSelect->RenderSelect($aOrderSpec, $aScalarArgs, $iLimitCount, $iLimitStart, $bGetCount, $bBeautifulSQL); - if ($sClassAlias == 'itop') + if ($sClassAlias == '_itop_') { echo $sRes."
\n"; } @@ -2185,6 +2189,8 @@ abstract class MetaModel // $aModifierProperties = self::MakeModifierProperties($oFilter); + // Create a unique cache id + // if (self::$m_bQueryCacheEnabled || self::$m_bTraceQueries) { // Need to identify the query @@ -2203,18 +2209,16 @@ abstract class MetaModel $sRawId = $sOqlQuery.$sModifierProperties; if (!is_null($aAttToLoad)) { - foreach($aAttToLoad as $sAlias => $aAttributes) - { - $sRawId = $sOqlQuery.'|'.implode(',', array_keys($aAttributes)); - } + $sRawId .= json_encode($aAttToLoad); } if (!is_null($aGroupByExpr)) { foreach($aGroupByExpr as $sAlias => $oExpr) { - $sRawId = 'g:'.$sAlias.'!'.$oExpr->Render(); + $sRawId .= 'g:'.$sAlias.'!'.$oExpr->Render(); } } + $sRawId .= $bGetCount; $sOqlId = md5($sRawId); } else @@ -2242,6 +2246,7 @@ abstract class MetaModel { // hit! $oSelect = clone self::$m_aQueryStructCache[$sOqlId]; + // Note: cloning is not enough... should be replaced by unserialize(serialize()) !!! } elseif (self::$m_bUseAPCCache) { @@ -2292,6 +2297,17 @@ abstract class MetaModel self::$m_aQueryStructCache[$sOqlId] = clone $oSelect; } + + if (self::$m_bOptimizeQueries) + { + // Simplify the query if just getting the count + // + if ($bGetCount) + { + $oSelect->SetSelect(array()); + } + $oSelect->OptimizeJoins(); + } } // Join to an additional table, if required... @@ -2668,7 +2684,7 @@ abstract class MetaModel foreach(self::EnumParentClasses($sClass) as $sParentClass) { if (!self::HasTable($sParentClass)) continue; -//echo "

Parent class: $sParentClass... let's call MakeQuerySingleTable()

"; + self::DbgTrace("Parent class: $sParentClass... let's call MakeQuerySingleTable()"); $oSelectParentTable = self::MakeQuerySingleTable($oBuild, $oFilter, $sParentClass, $aExtKeys, $aValues); if (is_null($oSelectBase)) @@ -2785,10 +2801,9 @@ abstract class MetaModel // 1/a - Get the key and friendly name // // We need one pkey to be the key, let's take the one corresponding to the root class - // (used to be based on the leaf, but it may happen that this one has no table defined) - $sRootClass = self::GetRootClass($sTargetClass); + // (used to be based on the leaf, then moved to the root class... now back to the leaf for optimization concerns) $oSelectedIdField = null; - if ($sTableClass == $sRootClass) + if ($sTableClass == $sTargetClass) { $oIdField = new FieldExpressionResolved(self::DBGetKey($sTableClass), $sTableAlias); $aTranslation[$sTargetAlias]['id'] = $oIdField; @@ -4491,8 +4506,10 @@ abstract class MetaModel } self::$m_bTraceQueries = self::$m_oConfig->GetLogQueries(); + self::$m_bIndentQueries = self::$m_oConfig->Get('query_indentation_enabled'); self::$m_bQueryCacheEnabled = self::$m_oConfig->GetQueryCacheEnabled(); + self::$m_bOptimizeQueries = self::$m_oConfig->Get('query_optimization_enabled'); self::$m_bSkipCheckToWrite = self::$m_oConfig->Get('skip_check_to_write'); self::$m_bSkipCheckExtKeys = self::$m_oConfig->Get('skip_check_ext_keys'); diff --git a/core/sqlquery.class.inc.php b/core/sqlquery.class.inc.php index 762f78cdf..aff3811a5 100644 --- a/core/sqlquery.class.inc.php +++ b/core/sqlquery.class.inc.php @@ -298,6 +298,7 @@ class SQLQuery { $this->m_bBeautifulQuery = $bBeautifulQuery; $sLineSep = $this->m_bBeautifulQuery ? "\n" : ''; + $sIndent = $this->m_bBeautifulQuery ? " " : null; // The goal will be to complete the lists as we build the Joins $aFrom = array(); @@ -309,9 +310,7 @@ class SQLQuery $aSelectedIdFields = array(); $this->privRender($aFrom, $aFields, $aGroupBy, $oCondition, $aDelTables, $aSetValues, $aSelectedIdFields); - $sIndent = $this->m_bBeautifulQuery ? " " : null; $sFrom = self::ClauseFrom($aFrom, $sIndent); - $sWhere = self::ClauseWhere($oCondition, $aArgs); if ($bGetCount) { @@ -352,8 +351,12 @@ class SQLQuery } // Interface, build the SQL query - public function RenderGroupBy($aArgs = array()) + public function RenderGroupBy($aArgs = array(), $bBeautifulQuery = false) { + $this->m_bBeautifulQuery = $bBeautifulQuery; + $sLineSep = $this->m_bBeautifulQuery ? "\n" : ''; + $sIndent = $this->m_bBeautifulQuery ? " " : null; + // The goal will be to complete the lists as we build the Joins $aFrom = array(); $aFields = array(); @@ -365,10 +368,10 @@ class SQLQuery $this->privRender($aFrom, $aFields, $aGroupBy, $oCondition, $aDelTables, $aSetValues, $aSelectedIdFields); $sSelect = self::ClauseSelect($aFields); - $sFrom = self::ClauseFrom($aFrom); + $sFrom = self::ClauseFrom($aFrom, $sIndent); $sWhere = self::ClauseWhere($oCondition, $aArgs); $sGroupBy = self::ClauseGroupBy($aGroupBy); - $sSQL = "SELECT $sSelect, COUNT(*) AS _itop_count_ FROM $sFrom WHERE $sWhere GROUP BY $sGroupBy"; + $sSQL = "SELECT $sSelect,$sLineSep COUNT(*) AS _itop_count_$sLineSep FROM $sFrom$sLineSep WHERE $sWhere$sLineSep GROUP BY $sGroupBy"; return $sSQL; } @@ -570,7 +573,6 @@ class SQLQuery { $aDelTables[] = "`{$this->m_sTableAlias}`"; } -//echo "

in privRenderSingleTable this->m_aValues

".print_r($this->m_aValues, true)."

\n"; foreach($this->m_aValues as $sFieldName=>$value) { $aSetValues["`{$this->m_sTableAlias}`.`$sFieldName`"] = $value; // quoted further! @@ -595,6 +597,93 @@ class SQLQuery return $this->m_sTableAlias; } -} + public function OptimizeJoins($aUsedTables = null) + { + if (is_null($aUsedTables)) + { + // Top call: build the list of tables absolutely required to perform the query + $aUsedTables = $this->CollectUsedTables(); + } + $aToDiscard = array(); + foreach ($this->m_aJoinSelects as $i => $aJoinInfo) + { + $oSQLQuery = $aJoinInfo["select"]; + $sTableAlias = $oSQLQuery->GetTableAlias(); + if ($oSQLQuery->OptimizeJoins($aUsedTables) && !array_key_exists($sTableAlias, $aUsedTables)) + { + $aToDiscard[] = $i; + } + } + foreach ($aToDiscard as $i) + { + unset($this->m_aJoinSelects[$i]); + } + + return (count($this->m_aJoinSelects) == 0); + } + + protected function CollectUsedTables(&$aTables = null) + { + if (is_null($aTables)) + { + $aTables = array(); + + $this->m_oConditionExpr->CollectUsedParents($aTables); + foreach($this->m_aFields as $sFieldAlias => $oField) + { + $oField->CollectUsedParents($aTables); + } + if ($this->m_aGroupBy) + { + foreach($this->m_aGroupBy as $sAlias => $oExpression) + { + $oExpression->CollectUsedParents($aTables); + } + } + if (!is_null($this->m_oSelectedIdField)) + { + $this->m_oSelectedIdField->CollectUsedParents($aTables); + } + } + + foreach ($this->m_aJoinSelects as $i => $aJoinInfo) + { + $oSQLQuery = $aJoinInfo["select"]; + if ($oSQLQuery->HasRequiredTables($aTables)) + { + // There is something required in the branch, then this node is a MUST + if (isset($aJoinInfo['righttablealias'])) + { + $aTables[$aJoinInfo['righttablealias']] = true; + } + if (isset($aJoinInfo["on_expression"])) + { + $sJoinCond = $aJoinInfo["on_expression"]->CollectUsedParents($aTables); + } + } + } + + return $aTables; + } + + // Is required in the JOIN, and therefore we must ensure that the join expression will be valid + protected function HasRequiredTables($aTables) + { + if (array_key_exists($this->m_sTableAlias, $aTables)) + { + return true; + } + foreach ($this->m_aJoinSelects as $i => $aJoinInfo) + { + $oSQLQuery = $aJoinInfo["select"]; + if ($oSQLQuery->HasRequiredTables($aTables)) + { + return true; + } + } + // None of the tables is in the list of required tables + return false; + } +} ?> diff --git a/test/replay_query_log.php b/test/replay_query_log.php index 380e8b5a5..bdef4fb73 100644 --- a/test/replay_query_log.php +++ b/test/replay_query_log.php @@ -244,7 +244,7 @@ default: } $oP->add("\n"); - $oP->add("
\n"); + $oP->add("\n"); $oP->add("\n"); $oP->add("
\n");