N.718 Audit failing with message "Attempting to merge a filter of class A with a filter of class B" (regression introduced in branch 2.3). There are circumstances for which the queries cannot (yet) be optimized (fallback to the original algorithm)

git-svn-id: http://svn.code.sf.net/p/itop/code/trunk@4611 a333f486-631f-4898-b8df-5754b55c2be0

core/dbobjectsearch.class.php

@@ -1,5 +1,5 @@
 <?php
-// Copyright (C) 2010-2016 Combodo SARL
+// Copyright (C) 2010-2017 Combodo SARL
 //
 //   This file is part of iTop.
 //
@@ -20,7 +20,7 @@
 /**
  * Define filters for a given class of objects (formerly named "filter") 
  *
- * @copyright   Copyright (C) 2010-2016 Combodo SARL
+ * @copyright   Copyright (C) 2010-2017 Combodo SARL
  * @license     http://opensource.org/licenses/AGPL-3.0
  */
 
@@ -749,13 +749,18 @@ class DBObjectSearch extends DBSearch
 		{
 			if (isset($oFilter->m_aReferencedBy[$this->GetClass()][$sExtKeyAttCode][$iOperatorCode]))
 			{
-				// Optimization - fold sibling query
-				$oRemoteFilter = $oFilter->m_aReferencedBy[$this->GetClass()][$sExtKeyAttCode][$iOperatorCode][0];
-				$aAliasTranslation = array();
-				$this->MergeWith_InNamespace($oRemoteFilter, $this->m_aClasses, $aAliasTranslation);
-				unset($oFilter->m_aReferencedBy[$this->GetClass()][$sExtKeyAttCode][$iOperatorCode]);
-				$this->m_oSearchCondition  = $this->m_oSearchCondition->Translate($aAliasTranslation, false, false);
-				$this->UpdateRealiasingMap($aRealiasingMap, $aAliasTranslation);
+				foreach ($oFilter->m_aReferencedBy[$this->GetClass()][$sExtKeyAttCode][$iOperatorCode] as $oRemoteFilter)
+				{
+					if ($this->GetClass() == $oRemoteFilter->GetClass())
+					{
+						// Optimization - fold sibling query
+						$aAliasTranslation = array();
+						$this->MergeWith_InNamespace($oRemoteFilter, $this->m_aClasses, $aAliasTranslation);
+						unset($oFilter->m_aReferencedBy[$this->GetClass()][$sExtKeyAttCode][$iOperatorCode]);
+						$this->m_oSearchCondition = $this->m_oSearchCondition->Translate($aAliasTranslation, false, false);
+						$this->UpdateRealiasingMap($aRealiasingMap, $aAliasTranslation);
+					}
+				}
 			}
 		}
 		$this->RecomputeClassList($this->m_aClasses);
@@ -767,12 +772,20 @@ class DBObjectSearch extends DBSearch
 		// Find the node on which the new tree must be attached (most of the time it is "this")
 		$oReceivingFilter = $this->GetNode($this->GetClassAlias());
 
+		$bMerged = false;
 		if (ENABLE_OPT && isset($oReceivingFilter->m_aPointingTo[$sExtKeyAttCode][$iOperatorCode]))
 		{
-			$oExisting = $oReceivingFilter->m_aPointingTo[$sExtKeyAttCode][$iOperatorCode][0];
-			$oExisting->MergeWith_InNamespace($oFilter, $oExisting->m_aClasses, $aAliasTranslation);
+			foreach ($oReceivingFilter->m_aPointingTo[$sExtKeyAttCode][$iOperatorCode] as $oExisting)
+			{
+				if ($oExisting->GetClass() == $oFilter->GetClass())
+				{
+					$oExisting->MergeWith_InNamespace($oFilter, $oExisting->m_aClasses, $aAliasTranslation);
+					$bMerged = true;
+					break;
+				}
+			}
 		}
-		else
+		if (!$bMerged)
 		{
 			$oFilter->AddToNamespace($aClassAliases, $aAliasTranslation);
 			$oReceivingFilter->m_aPointingTo[$sExtKeyAttCode][$iOperatorCode][] = $oFilter;
@@ -814,13 +827,18 @@ class DBObjectSearch extends DBSearch
 		{
 			if (isset($oFilter->m_aPointingTo[$sForeignExtKeyAttCode][$iOperatorCode]))
 			{
-				// Optimization - fold sibling query
-				$oRemoteFilter = $oFilter->m_aPointingTo[$sForeignExtKeyAttCode][$iOperatorCode][0];
-				$aAliasTranslation = array();
-				$this->MergeWith_InNamespace($oRemoteFilter, $this->m_aClasses, $aAliasTranslation);
-				unset($oFilter->m_aPointingTo[$sForeignExtKeyAttCode][$iOperatorCode]);
-				$this->m_oSearchCondition  = $this->m_oSearchCondition->Translate($aAliasTranslation, false, false);
-				$this->UpdateRealiasingMap($aRealiasingMap, $aAliasTranslation);
+				foreach ($oFilter->m_aPointingTo[$sForeignExtKeyAttCode][$iOperatorCode] as $oRemoteFilter)
+				{
+					if ($this->GetClass() == $oRemoteFilter->GetClass())
+					{
+						// Optimization - fold sibling query
+						$aAliasTranslation = array();
+						$this->MergeWith_InNamespace($oRemoteFilter, $this->m_aClasses, $aAliasTranslation);
+						unset($oFilter->m_aPointingTo[$sForeignExtKeyAttCode][$iOperatorCode]);
+						$this->m_oSearchCondition  = $this->m_oSearchCondition->Translate($aAliasTranslation, false, false);
+						$this->UpdateRealiasingMap($aRealiasingMap, $aAliasTranslation);
+					}
+				}
 			}
 		}
 		$this->RecomputeClassList($this->m_aClasses);
@@ -834,12 +852,20 @@ class DBObjectSearch extends DBSearch
 		// Find the node on which the new tree must be attached (most of the time it is "this")
 		$oReceivingFilter = $this->GetNode($this->GetClassAlias());
 
+		$bMerged = false;
 		if (ENABLE_OPT && isset($oReceivingFilter->m_aReferencedBy[$sForeignClass][$sForeignExtKeyAttCode][$iOperatorCode]))
 		{
-			$oExisting = $oReceivingFilter->m_aReferencedBy[$sForeignClass][$sForeignExtKeyAttCode][$iOperatorCode][0];
-			$oExisting->MergeWith_InNamespace($oFilter, $oExisting->m_aClasses, $aAliasTranslation);
+			foreach ($oReceivingFilter->m_aReferencedBy[$sForeignClass][$sForeignExtKeyAttCode][$iOperatorCode] as $oExisting)
+			{
+				if ($oExisting->GetClass() == $oFilter->GetClass())
+				{
+					$oExisting->MergeWith_InNamespace($oFilter, $oExisting->m_aClasses, $aAliasTranslation);
+					$bMerged = true;
+					break;
+				}
+			}
 		}
-		else
+		if (!$bMerged)
 		{
 			$oFilter->AddToNamespace($aClassAliases, $aAliasTranslation);
 			$oReceivingFilter->m_aReferencedBy[$sForeignClass][$sForeignExtKeyAttCode][$iOperatorCode][] = $oFilter;

test/testlist.inc.php

@@ -5309,7 +5309,7 @@ class TestIntersectOptimization5 extends TestBizModel
 
 	protected function DoExecute()
 	{
-		echo "<h4>Here we are (conluding a long series of tests)</h4>\n";
+		echo "<h4>Here we are (concluding a long series of tests)</h4>\n";
 		$sQueryA = 'SELECT Organization AS o JOIN UserRequest AS r ON r.org_id = o.id JOIN Person AS p ON r.caller_id = p.id WHERE o.name LIKE "Company" AND r.service_id = 123 AND p.employee_number LIKE "007"';
 		$sQueryB = 'SELECT UserRequest AS ur JOIN Person AS p ON ur.agent_id = p.id WHERE p.status != "terminated"';
 
@@ -5318,7 +5318,7 @@ class TestIntersectOptimization5 extends TestBizModel
 		$oSearchA = DBSearch::FromOQL($sQueryA);
 		$oSearchB = DBSearch::FromOQL($sQueryB);
 		$oSearchB->AddCondition_PointingTo($oSearchA, 'org_id');
-		echo "<p>Referenced by...: ".htmlentities($oSearchB->ToOQL(), ENT_QUOTES, 'UTF-8')."</p>\n";
+		echo "<p>Pointing to...: ".htmlentities($oSearchB->ToOQL(), ENT_QUOTES, 'UTF-8')."</p>\n";
 		CMDBSource::TestQuery($oSearchB->MakeSelectQuery());
 		echo "<p>Successfully tested the SQL query.</p>\n";
 	}
@@ -5454,3 +5454,67 @@ class TestImplicitAlias extends TestBizModel
 		}
 	}
 }
+
+class TestIntersectNotOptimized extends TestBizModel
+{
+	static public function GetName()
+	{
+		return 'Internal query NOT optimized';
+	}
+
+	static public function GetDescription()
+	{
+		return '(N.718) Sometimes, the optimization CANNOT be performed because merging two different classes (same branch) is not implemented';
+	}
+
+	protected function DoExecute()
+	{
+		echo "<h4>Intersect NOT optimized on 'pointing to'</h4>\n";
+		$sBaseQuery = 'SELECT lnkContactToFunctionalCI AS l JOIN Contact AS c ON l.contact_id = c.id';
+		$sOQL = 'SELECT lnkContactToFunctionalCI AS l JOIN Person AS p ON l.contact_id = p.id';
+		echo "<h5>Left: ".htmlentities($sBaseQuery, ENT_QUOTES, 'UTF-8')."</h5>\n";
+		echo "<h5>Right: ".htmlentities($sOQL, ENT_QUOTES, 'UTF-8')."</h5>\n";
+		$oSearchA = DBSearch::FromOQL($sBaseQuery);
+		$oSearchB = DBSearch::FromOQL($sOQL);
+		$oIntersect = $oSearchA->Intersect($oSearchB);
+		echo "<p>Intersect: ".htmlentities($oIntersect->ToOQL(), ENT_QUOTES, 'UTF-8')."</p>\n";
+		CMDBSource::TestQuery($oIntersect->MakeSelectQuery());
+		echo "<p>Successfully tested the SQL query.</p>\n";
+
+		echo "<h4>Intersect NOT optimized on 'referenced by'</h4>\n";
+		$sBaseQuery = 'SELECT Organization AS o JOIN Contact AS c ON c.org_id = o.id WHERE c.id = 1';
+		$sOQL = 'SELECT Organization AS o JOIN Person AS p ON p.org_id = o.id WHERE p.id = 2';
+		echo "<h5>Left: ".htmlentities($sBaseQuery, ENT_QUOTES, 'UTF-8')."</h5>\n";
+		echo "<h5>Right: ".htmlentities($sOQL, ENT_QUOTES, 'UTF-8')."</h5>\n";
+		$oSearchA = DBSearch::FromOQL($sBaseQuery);
+		$oSearchB = DBSearch::FromOQL($sOQL);
+		$oIntersect = $oSearchA->Intersect($oSearchB);
+		echo "<p>Intersect: ".htmlentities($oIntersect->ToOQL(), ENT_QUOTES, 'UTF-8')."</p>\n";
+		CMDBSource::TestQuery($oIntersect->MakeSelectQuery());
+		echo "<p>Successfully tested the SQL query.</p>\n";
+
+		echo "<h4>NOT Folding on AddCondition_PointingTo</h4>\n";
+		$sQueryA = 'SELECT Organization AS o JOIN Contact AS c ON c.org_id = o.id';
+		$sQueryB = 'SELECT Person';
+		echo "<h5>A: ".htmlentities($sQueryA, ENT_QUOTES, 'UTF-8')."</h5>\n";
+		echo "<h5>B: ".htmlentities($sQueryB, ENT_QUOTES, 'UTF-8')."</h5>\n";
+		$oSearchA = DBSearch::FromOQL($sQueryA);
+		$oSearchB = DBSearch::FromOQL($sQueryB);
+		$oSearchB->AddCondition_PointingTo($oSearchA, 'org_id');
+		echo "<p>Pointing to...: ".htmlentities($oSearchB->ToOQL(), ENT_QUOTES, 'UTF-8')."</p>\n";
+		CMDBSource::TestQuery($oSearchB->MakeSelectQuery());
+		echo "<p>Successfully tested the SQL query.</p>\n";
+
+		echo "<h4>NOT Folding on AddCondition_ReferencedBy</h4>\n";
+		$sQueryA = 'SELECT lnkContactToFunctionalCI AS l JOIN Contact AS c ON l.contact_id = c.id';
+		$sQueryB = 'SELECT Person';
+		echo "<h5>A: ".htmlentities($sQueryA, ENT_QUOTES, 'UTF-8')."</h5>\n";
+		echo "<h5>B: ".htmlentities($sQueryB, ENT_QUOTES, 'UTF-8')."</h5>\n";
+		$oSearchA = DBSearch::FromOQL($sQueryA);
+		$oSearchB = DBSearch::FromOQL($sQueryB);
+		$oSearchB->AddCondition_ReferencedBy($oSearchA, 'contact_id');
+		echo "<p>Referenced by...: ".htmlentities($oSearchA->ToOQL(), ENT_QUOTES, 'UTF-8')."</p>\n";
+		CMDBSource::TestQuery($oSearchA->MakeSelectQuery());
+		echo "<p>Successfully tested the SQL query.</p>\n";
+	}
+}