db-derby-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From abr...@apache.org
Subject svn commit: r520188 - in /db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile: BinaryRelationalOperatorNode.java FromBaseTable.java HashJoinStrategy.java NestedLoopJoinStrategy.java Predicate.java PredicateList.java ValueNode.java
Date Tue, 20 Mar 2007 00:01:19 GMT
Author: abrown
Date: Mon Mar 19 17:01:17 2007
New Revision: 520188

URL: http://svn.apache.org/viewvc?view=rev&rev=520188
Log:
DERBY-47 (cleanup): Cleanup patch which does the following:

  1 - Changes Predicate.isRelationalOpPredicate() so that it just calls
      the already existing method "isRelationalOperator()" on the left
      operand of the predicate's AND node.  This ultimately comes down
      to a simple check for a null variable in BinaryRelationalOperatorNode,
      which is cheaper than the old check.

  2 - Adds a new method, "isInListProbeNode()", to ValueNode. The default
      case returns "false", while BinaryRelationalOperatorNode returns true
      if it has a source IN-list associated with it.

      Also adds a corresponding method called "isInListProbePredicate()"
      to Predicate.java. This method allows for simple (and relatively
      cheap) checking of a predicate to see if it is an IN-list probe
      predicate.

  3 - Modifies Predicate.getSourceInList() to return the underlying
      InListOperatorNode for probe predicates AND for "normal"
      IN-list predicates (i.e. an IN-list that could not be
      transformed into a "probe predicate" because it contains
      one or more non-parameter, non-constant values)

      This then allowed for some cleanup of other related code in
      PredicateList.java.

      Also adds a second version of getSourceInList() that takes a
      boolean argument; if true, then it will only return the source
      IN list for a predicate *if* that predicate is an IN-list
      probe predicate.

  4 - Changes PredicateList.generateInListValues() to account for the
      fact that it only ever gets called when we know that there is
      a probe predicate in the list.

  5 - Shortens a couple of lines in FromBaseTable that were added with
      earlier patches but were longer than 80 chars. Also rewrites
      one Sanity check in that class to avoid construction of strings
      when no error occurs (per recent discussions on derby-dev). 

Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryRelationalOperatorNode.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/FromBaseTable.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/HashJoinStrategy.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/NestedLoopJoinStrategy.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/Predicate.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/PredicateList.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ValueNode.java

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryRelationalOperatorNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryRelationalOperatorNode.java?view=diff&rev=520188&r1=520187&r2=520188
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryRelationalOperatorNode.java
(original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryRelationalOperatorNode.java
Mon Mar 19 17:01:17 2007
@@ -1230,6 +1230,12 @@
 			(operatorType == RelationalOperator.EQUALS_RELOP);
 	}
 
+	/** @see ValueNode#isInListProbeNode */
+	public boolean isInListProbeNode()
+	{
+		return (inListProbeSource != null);
+	}
+
 	/** @see ValueNode#optimizableEqualityNode */
 	public boolean optimizableEqualityNode(Optimizable optTable, 
 										   int columnNumber, 

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/FromBaseTable.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/FromBaseTable.java?view=diff&rev=520188&r1=520187&r2=520188
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/FromBaseTable.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/FromBaseTable.java Mon
Mar 19 17:01:17 2007
@@ -1331,23 +1331,29 @@
 					/* A probe predicate is only useful if it can be used as
 					 * as a start/stop key for _first_ column in an index
 					 * (i.e. if the column position is 0).  That said, we only
-					 * allow a single start/stop key per column position in the
-					 * index (see orderUsefulPredicates() in PredicateList.java).
+					 * allow a single start/stop key per column position in
+					 * the index (see PredicateList.orderUsefulPredicates()).
 					 * Those two facts combined mean that we should never have
 					 * more than one probe predicate start/stop key for a given
 					 * conglomerate.
 					 */
 					if (SanityManager.DEBUG)
 					{
-						SanityManager.ASSERT(
-							(ssKeySourceInList == null) ||
-								(((Predicate)pred).getSourceInList() == null),
+						if ((ssKeySourceInList != null) &&
+							((Predicate)pred).isInListProbePredicate())
+						{
+							SanityManager.THROWASSERT(
 							"Found multiple probe predicate start/stop keys" +
 							" for conglomerate '" + cd.getConglomerateName() +
 							"' when at most one was expected.");
+						}
 					}
 
-					ssKeySourceInList = ((Predicate)pred).getSourceInList();
+					/* By passing "true" in the next line we indicate that we
+					 * should only retrieve the underlying InListOpNode *if*
+					 * the predicate is a "probe predicate".
+					 */
+					ssKeySourceInList = ((Predicate)pred).getSourceInList(true);
 					boolean knownConstant = pred.compareWithKnownConstant(this, true);
 
 					if (startKey)
@@ -2736,7 +2742,7 @@
 		for (int i = 0; i < restrictionList.size(); i++)
 		{
 			Predicate pred = (Predicate)restrictionList.elementAt(i);
-			if ((pred.getSourceInList() != null) && pred.isStartKey())
+			if (pred.isInListProbePredicate() && pred.isStartKey())
 			{
 				disableBulkFetch();
 				multiProbing = true;

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/HashJoinStrategy.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/HashJoinStrategy.java?view=diff&rev=520188&r1=520187&r2=520188
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/HashJoinStrategy.java
(original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/HashJoinStrategy.java
Mon Mar 19 17:01:17 2007
@@ -357,7 +357,7 @@
 			for (int i = storeRestrictionList.size() - 1; i >= 0; i--)
 			{
 				pred = (Predicate)storeRestrictionList.getOptPredicate(i);
-				if (pred.getSourceInList() != null)
+				if (pred.isInListProbePredicate())
 				{
 					SanityManager.THROWASSERT("Found IN-list probing " +
 						"(" + pred.binaryRelOpColRefsToString() +

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/NestedLoopJoinStrategy.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/NestedLoopJoinStrategy.java?view=diff&rev=520188&r1=520187&r2=520188
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/NestedLoopJoinStrategy.java
(original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/NestedLoopJoinStrategy.java
Mon Mar 19 17:01:17 2007
@@ -266,7 +266,7 @@
 				for (int i = storeRestrictionList.size() - 1; i >= 0; i--)
 				{
 					pred = (Predicate)storeRestrictionList.getOptPredicate(i);
-					if (pred.getSourceInList() != null)
+					if (pred.isInListProbePredicate())
 					{
 						SanityManager.THROWASSERT("Found IN-list probing " +
 							"predicate (" + pred.binaryRelOpColRefsToString() +

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/Predicate.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/Predicate.java?view=diff&rev=520188&r1=520187&r2=520188
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/Predicate.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/Predicate.java Mon Mar
19 17:01:17 2007
@@ -1309,19 +1309,80 @@
 	 */
 	protected boolean isRelationalOpPredicate()
 	{
-		return ((getRelop() != null) && (getSourceInList() == null));
+		/* The isRelationalOperator() method on the ValueNode
+		 * interface tells us what we need to know, so all we have
+		 * to do is call that method on the left child of our AND node.
+		 * Note that BinaryRelationalOperatorNode.isRelationalOperator()
+		 * includes logic to determine whether or not it (the BRON) is
+		 * really a disguised IN-list operator--and if so, it will
+		 * return false (which is what we want).
+		 */
+		return andNode.getLeftOperand().isRelationalOperator();
 	}
 
 	/**
-	 * If this predicate is an IN-list "probe predicate" then return
-	 * the InListOperatorNode from which it was built.  Otherwise
-	 * return null.
+	 * Return whether or not this predicate is an IN-list probe
+	 * predicate.
+	 */
+	protected boolean isInListProbePredicate()
+	{
+		/* The isInListProbeNode() method on the ValueNode interface
+		 * tells us what we need to know, so all we have to do is call
+		 * that method on the left child of our AND node.
+		 */
+		return andNode.getLeftOperand().isInListProbeNode();
+	}
+
+	/**
+	 * If this predicate corresponds to an IN-list, return the underlying
+	 * InListOperatorNode from which it was built.  There are two forms
+	 * to check for:
+	 *
+	 *  1. This predicate is an IN-list "probe predicate", in which case
+	 *     the underlying InListOpNode is stored within the binary relational
+	 *     operator that is the left operand of this predicate's AND node.
+	 *
+	 *  2. This predicate corresponds to an IN-list that could _not_ be
+	 *     transformed into a "probe predicate" (i.e. the IN-list contains
+	 *     one or more non-parameter, non-constant values). In that case
+	 *     the underlying InListOpNode is simply the left operand of
+	 *     this predicate's AND node.
+	 *
+	 * If this predicate does not correspond to an IN-list in any way,
+	 * this method will return null.
 	 */
 	protected InListOperatorNode getSourceInList()
 	{
-		RelationalOperator relop = getRelop();
-		if (relop instanceof BinaryRelationalOperatorNode)
-			return ((BinaryRelationalOperatorNode)relop).getInListOp();
+		return getSourceInList(false);
+	}
+
+	/**
+	 * Does the work of getSourceInList() above, but can also be called
+	 * directly with an argument to indicate whether or not we should
+	 * limit ourselves to probe predicates.
+	 *
+	 * @param probePredOnly If true, only get the source IN list for this
+	 *   predicate *if* it is an IN-list probe predicate.  If false,
+	 *   return the underlying InListOperatorNode (if it exists) regardless
+	 *   of whether this is a probe predicate or an un-transformed IN-list
+	 *   pred.
+	 * 
+	 * @return Underlying InListOp for this predicate (depending on
+	 *   the value of probePredOnly), or null if this predicate does
+	 *   not correspond to an IN-list in any way.
+	 */
+	protected InListOperatorNode getSourceInList(boolean probePredOnly)
+	{
+		ValueNode vn = andNode.getLeftOperand();
+		if (isInListProbePredicate())
+			return ((BinaryRelationalOperatorNode)vn).getInListOp();
+
+		if (probePredOnly)
+			return null;
+
+		if (vn instanceof InListOperatorNode)
+			return (InListOperatorNode)vn;
+
 		return null;
 	}
 }

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/PredicateList.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/PredicateList.java?view=diff&rev=520188&r1=520187&r2=520188
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/PredicateList.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/PredicateList.java Mon
Mar 19 17:01:17 2007
@@ -195,29 +195,25 @@
 		for (int index = 0; index < size; index++)
 		{
 			Predicate	pred = (Predicate) elementAt(index);
-
-			/*
-			** Skip over it if it's not a relational operator (this includes
-			** BinaryComparisonOperators and IsNullNodes.
-			*/
 			RelationalOperator relop = pred.getRelop();
+
+			/* InListOperatorNodes, while not relational operators, may still
+			 * be useful.  There are two cases: a) we transformed the IN-list
+			 * into a probe predicate of the form "col = ?", which can then be
+			 * optimized/generated as a start/stop key and used for "multi-
+			 * probing" at execution; or b) we did *not* transform the IN-list,
+			 * in which case we'll generate _dynamic_ start and stop keys in
+			 * order to improve scan performance (beetle 3858).  In either case
+			 * the IN-list may still prove "useful".
+			 */
 			InListOperatorNode inNode = pred.getSourceInList();
 			boolean isIn = (inNode != null);
 
-			if (relop == null)
-			{
-				/* if it's "in" operator, we generate dynamic start and stop key
-				 * to improve index scan performance, beetle 3858.
-				 */
-				if (pred.getAndNode().getLeftOperand() instanceof InListOperatorNode &&
-					! ((InListOperatorNode)pred.getAndNode().getLeftOperand()).getTransformed())
-				{
-					isIn = true;
-					inNode = (InListOperatorNode) pred.getAndNode().getLeftOperand();
-				}
-				else
-					continue;
-			}
+			/* If it's not a relational operator and it's not "in", then it's
+			 * not useful.
+			 */
+			if (!isIn && (relop == null))
+				continue;
 
 			/*
 			** If the relational operator is neither a useful start key
@@ -564,7 +560,7 @@
 
 				if (SanityManager.DEBUG)
 				{
-					if (pred.getSourceInList() != null)
+					if (pred.isInListProbePredicate())
 					{
 						SanityManager.THROWASSERT("Found an IN-list probe " +
 							"predicate (" + pred.binaryRelOpColRefsToString() +
@@ -609,34 +605,29 @@
 			Predicate pred = (Predicate) elementAt(index);
 			ColumnReference indexCol = null;
 			int			indexPosition;
-
-			/*
-			** Skip over it if it's not a relational operator (this includes
-			** BinaryComparisonOperators and IsNullNodes.
-			*/
 			RelationalOperator relop = pred.getRelop();
 
-			/* if it's "in" operator, we generate dynamic start and stop key
-			 * to improve index scan performance, beetle 3858.
+			/* InListOperatorNodes, while not relational operators, may still
+			 * be useful.  There are two cases: a) we transformed the IN-list
+			 * into a probe predicate of the form "col = ?", which can then be
+			 * optimized/generated as a start/stop key and used for "multi-
+			 * probing" at execution; or b) we did *not* transform the IN-list,
+			 * in which case we'll generate _dynamic_ start and stop keys in
+			 * order to improve scan performance (beetle 3858).  In either case
+			 * the IN-list may still prove "useful".
 			 */
 			InListOperatorNode inNode = pred.getSourceInList();
 			boolean isIn = (inNode != null);
-			boolean isInListProbePred = isIn;
 
-			if (relop == null)
+			/* If it's not an "in" operator and either a) it's not a relational
+			 * operator or b) it's not a qualifier, then it's not useful for
+			 * limiting the scan, so skip it.
+			 */
+			if (!isIn &&
+				((relop == null) || !relop.isQualifier(optTable, pushPreds)))
 			{
-				if (pred.getAndNode().getLeftOperand() instanceof InListOperatorNode &&
-					! ((InListOperatorNode)pred.getAndNode().getLeftOperand()).getTransformed())
-				{
-					isIn = true;
-					inNode = (InListOperatorNode) pred.getAndNode().getLeftOperand();
-				}
-				else
-					continue;
-			}
-
-			if ( !isIn && ! relop.isQualifier(optTable, pushPreds))
 				continue;
+			}
 
 			/* Look for an index column on one side of the relop */
 			for (indexPosition = 0;
@@ -652,7 +643,8 @@
 								(indexCol.getColumnNumber() != baseColumnPositions[indexPosition]) ||
 								inNode.selfReference(indexCol))
 							indexCol = null;
-						else if (isInListProbePred && (indexPosition > 0))
+						else if (pred.isInListProbePredicate()
+								&& (indexPosition > 0))
 						{
 							/* If the predicate is an IN-list probe predicate
 							 * then we only consider it to be useful if the
@@ -753,20 +745,10 @@
 			RelationalOperator	relop             = thisPred.getRelop();
 			int                 thisOperator      = -1;
 
-			InListOperatorNode  inNode            = thisPred.getSourceInList();
-			boolean             isIn              = (inNode != null);
-			boolean             isInListProbePred     = isIn;
-
-			if (relop == null)
-			{
-				isIn = true;
-				inNode = (InListOperatorNode) 
-                    thisPred.getAndNode().getLeftOperand();
-			}
-			else
-            {
+			boolean isIn = (thisPred.getSourceInList() != null);
+
+			if (relop != null)
 				thisOperator = relop.getOperator();
-            }
 
 			/* Allow only one start and stop position per index column */
 			if (currentStartPosition != thisIndexPosition)
@@ -911,7 +893,7 @@
 				 * predicate down to the base table for special handling.
 				 */
 				Predicate predToPush;
-				if (isIn && !isInListProbePred)
+				if (isIn && !thisPred.isInListProbePredicate())
                 {
 					AndNode andCopy = (AndNode) getNodeFactory().getNode(
 										C_NodeTypes.AND_NODE,
@@ -941,7 +923,7 @@
 					 * via execution-time index probes (for more see
 					 * execute/MultiProbeTableScanResultSet.java).
 					 */
-					if (!isIn || isInListProbePred)
+					if (!isIn || thisPred.isInListProbePredicate())
 						removeOptPredicate(thisPred);
 				}
 				else if (SanityManager.DEBUG)
@@ -1506,11 +1488,11 @@
 					 * Reference node).  Then we pass that copy into the new
 					 * relational operator node.
 					 */
-					InListOperatorNode ilon = opNode.getInListOp();
-					if (ilon != null)
+					inNode = opNode.getInListOp();
+					if (inNode != null)
 					{
-						ilon = ilon.shallowCopy();
-						ilon.setLeftOperand(newCRNode);
+						inNode = inNode.shallowCopy();
+						inNode.setLeftOperand(newCRNode);
 					}
 
 					BinaryRelationalOperatorNode newRelop = (BinaryRelationalOperatorNode)
@@ -1518,7 +1500,7 @@
 										opNode.getNodeType(),
 										newCRNode,
 										opNode.getRightOperand(),
-										ilon,
+										inNode,
 										getContextManager());
 					newRelop.bindComparisonOperator();
 					leftOperand = newRelop;
@@ -2854,15 +2836,12 @@
 	protected void generateInListValues(ExpressionClassBuilder acb,
 		MethodBuilder mb) throws StandardException
 	{
-		int size = size();
-		InListOperatorNode ilon = null;
-		for (int index = size - 1; index >= 0; index--)
+		for (int index = size() - 1; index >= 0; index--)
 		{
 			Predicate pred = (Predicate)elementAt(index);
-			ilon = pred.getSourceInList();
 
 			// Don't do anything if it's not an IN-list probe predicate.
-			if (ilon == null)
+			if (!pred.isInListProbePredicate())
 				continue;
 
 			/* We're going to generate the relevant code for the probe
@@ -2882,8 +2861,7 @@
 			{
 				for (int i = 0; i < index; i++)
 				{
-					pred = (Predicate)elementAt(i);
-					if (pred.getSourceInList() != null)
+					if (((Predicate)elementAt(i)).isInListProbePredicate())
 					{
 						SanityManager.THROWASSERT("Found multiple probe " +
 							"predicates for IN-list when only one was " +
@@ -2892,18 +2870,20 @@
 				}
 			}
 
-			break;
-		}
-
-		if (ilon != null)
-		{
+			InListOperatorNode ilon = pred.getSourceInList();
 			mb.getField(ilon.generateListAsArray(acb, mb));
 			mb.push(ilon.isOrdered());
+			return;
 		}
-		else
+
+		/* If we get here then we didn't find any probe predicates.  But
+		 * if that's true then we shouldn't have made it to this method
+		 * to begin with.
+		 */
+		if (SanityManager.DEBUG)
 		{
-			mb.pushNull(ClassName.DataValueDescriptor + "[]");
-			mb.push(false);
+			SanityManager.THROWASSERT("Attempted to generate IN-list values" +
+				"for multi-probing but no probe predicates were found.");
 		}
 	}
 
@@ -3467,8 +3447,8 @@
 		// second arg
 		if (isIn)
 		{
-			InListOperatorNode inNode = (InListOperatorNode) pred.getAndNode().getLeftOperand();
-			inNode.generateStartStopKey(isAscending[columnNumber], isStartKey, acb, mb);
+			pred.getSourceInList().generateStartStopKey(
+				isAscending[columnNumber], isStartKey, acb, mb);
 		}
 		else
 			pred.generateExpressionOperand(optTable, baseColumns[columnNumber], acb, mb);

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ValueNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ValueNode.java?view=diff&rev=520188&r1=520187&r2=520188
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ValueNode.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ValueNode.java Mon Mar
19 17:01:17 2007
@@ -1281,6 +1281,19 @@
 		return false;
 	}
 
+	/**
+	 * Returns true if this value node is an operator created
+	 * for optimized performance of an IN list.
+	 *
+	 * Or more specifically, returns true if this value node is
+	 * an equals operator of the form "col = ?" that we generated
+	 * during preprocessing to allow index multi-probing.
+	 */
+	public boolean isInListProbeNode()
+	{
+		return false;
+	}
+
 	/** Return true if the predicate represents an optimizable equality node.
 	 * an expression is considered to be an optimizable equality node if all the
 	 * following conditions are met:



Mime
View raw message