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

@@ -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',

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 <field> = <scalar> or <field> = :<variable>
@@ -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();

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."<br/>\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 "<p>Parent class: $sParentClass... let's call MakeQuerySingleTable()</p>";
+
 			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');
 

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 "<p>in privRenderSingleTable this->m_aValues<pre>".print_r($this->m_aValues, true)."</pre></p>\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;
+	}
+}
 ?>

test/replay_query_log.php

@@ -244,7 +244,7 @@ default:
 	}
 	$oP->add("</ol>\n");
 	
-	$oP->add("<form action=\"?operation=benchmark?repeat=3\" method=\"post\">\n");
+	$oP->add("<form action=\"?operation=benchmark&repeat=3\" method=\"post\">\n");
 	$oP->add("<input type=\"submit\" value=\"Benchmark (3 repeats)!\">\n");
 	$oP->add("</form>\n");