8:37 p.m.
Author: shawkins
Date: 2009-05-12 21:37:34 -0400 (Tue, 12 May 2009)
New Revision: 923
Modified:
trunk/engine/src/main/java/com/metamatrix/query/optimizer/relational/rules/FrameUtil.java
trunk/engine/src/main/java/com/metamatrix/query/rewriter/QueryRewriter.java
trunk/engine/src/main/java/com/metamatrix/query/sql/visitor/ExpressionMappingVisitor.java
trunk/engine/src/test/java/com/metamatrix/query/sql/visitor/TestExpressionMappingVisitor.java
Log:
TEIID-582 fixing the expression mapping visitor to prevent recursive mappings.
Modified:
trunk/engine/src/main/java/com/metamatrix/query/optimizer/relational/rules/FrameUtil.java
===================================================================
---
trunk/engine/src/main/java/com/metamatrix/query/optimizer/relational/rules/FrameUtil.java 2009-05-13
01:01:53 UTC (rev 922)
+++
trunk/engine/src/main/java/com/metamatrix/query/optimizer/relational/rules/FrameUtil.java 2009-05-13
01:37:34 UTC (rev 923)
@@ -55,7 +55,6 @@
import com.metamatrix.query.sql.lang.QueryCommand;
import com.metamatrix.query.sql.lang.Select;
import com.metamatrix.query.sql.lang.StoredProcedure;
-import com.metamatrix.query.sql.symbol.AggregateSymbol;
import com.metamatrix.query.sql.symbol.Constant;
import com.metamatrix.query.sql.symbol.ElementSymbol;
import com.metamatrix.query.sql.symbol.Expression;
@@ -264,24 +263,14 @@
return expression;
}
- if(expression instanceof ElementSymbol) {
+ if(expression instanceof SingleElementSymbol) {
Expression mappedSymbol = (Expression) symbolMap.get(expression);
if (mappedSymbol != null) {
return mappedSymbol;
}
return expression;
}
-
- if(expression instanceof AggregateSymbol) {
- AggregateSymbol aggSymbol = (AggregateSymbol) expression;
-
- // First try to replace the entire aggregate
- SingleElementSymbol replacement = (SingleElementSymbol)
symbolMap.get(aggSymbol);
- if(replacement != null) {
- return replacement;
- }
- }
-
+
ExpressionMappingVisitor.mapExpressions(expression, symbolMap);
return expression;
@@ -507,7 +496,7 @@
if (project != null) {
return
(List<SingleElementSymbol>)project.getProperty(NodeConstants.Info.PROJECT_COLS);
}
- Assertion.failed("no top cols in frame");
+ Assertion.failed("no top cols in frame"); //$NON-NLS-1$
return null;
}
Modified: trunk/engine/src/main/java/com/metamatrix/query/rewriter/QueryRewriter.java
===================================================================
--- trunk/engine/src/main/java/com/metamatrix/query/rewriter/QueryRewriter.java 2009-05-13
01:01:53 UTC (rev 922)
+++ trunk/engine/src/main/java/com/metamatrix/query/rewriter/QueryRewriter.java 2009-05-13
01:37:34 UTC (rev 923)
@@ -751,14 +751,14 @@
for (SingleElementSymbol symbol :
(List<SingleElementSymbol>)query.getSelect().getProjectedSymbols()) {
expressionMap.put((Expression)SymbolMap.getExpression(symbol).clone(),
(SingleElementSymbol)iter.next());
}
- ExpressionMappingVisitor.mapExpressions(groupBy, expressionMap, true);
+ ExpressionMappingVisitor.mapExpressions(groupBy, expressionMap);
outerQuery.setGroupBy(groupBy);
- ExpressionMappingVisitor.mapExpressions(having, expressionMap, true);
+ ExpressionMappingVisitor.mapExpressions(having, expressionMap);
outerQuery.setHaving(having);
- ExpressionMappingVisitor.mapExpressions(orderBy, expressionMap, true);
+ ExpressionMappingVisitor.mapExpressions(orderBy, expressionMap);
outerQuery.setOrderBy(orderBy);
outerQuery.setLimit(limit);
- ExpressionMappingVisitor.mapExpressions(select, expressionMap, true);
+ ExpressionMappingVisitor.mapExpressions(select, expressionMap);
outerQuery.setSelect(select);
outerQuery.setInto(into);
outerQuery.setOption(query.getOption());
Modified:
trunk/engine/src/main/java/com/metamatrix/query/sql/visitor/ExpressionMappingVisitor.java
===================================================================
---
trunk/engine/src/main/java/com/metamatrix/query/sql/visitor/ExpressionMappingVisitor.java 2009-05-13
01:01:53 UTC (rev 922)
+++
trunk/engine/src/main/java/com/metamatrix/query/sql/visitor/ExpressionMappingVisitor.java 2009-05-13
01:37:34 UTC (rev 923)
@@ -24,9 +24,11 @@
import java.util.ArrayList;
import java.util.Collection;
+import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import com.metamatrix.query.sql.LanguageObject;
import com.metamatrix.query.sql.LanguageVisitor;
@@ -47,7 +49,6 @@
import com.metamatrix.query.sql.lang.StoredProcedure;
import com.metamatrix.query.sql.lang.SubqueryCompareCriteria;
import com.metamatrix.query.sql.lang.SubquerySetCriteria;
-import com.metamatrix.query.sql.navigator.PostOrderNavigator;
import com.metamatrix.query.sql.navigator.PreOrderNavigator;
import com.metamatrix.query.sql.proc.AssignmentStatement;
import com.metamatrix.query.sql.symbol.AggregateSymbol;
@@ -74,7 +75,7 @@
public ExpressionMappingVisitor(Map symbolMap) {
this.symbolMap = symbolMap;
}
-
+
protected boolean createAliases() {
return true;
}
@@ -293,27 +294,25 @@
}
/**
- * Take a language object (currently only criteria and expressions will be
- * properly mapped) and use the map to swap out expressions with new expressions.
- * If an expression is not found in the map, it is not swapped. The object
- * is modified in place, so is not returned.
+ * The object is modified in place, so is not returned.
* @param obj Language object
* @param exprMap Expression map, Expression to Expression
*/
public static void mapExpressions(LanguageObject obj, Map exprMap) {
- mapExpressions(obj, exprMap, false);
- }
-
- public static void mapExpressions(LanguageObject obj, Map exprMap, boolean preOrder)
{
if(obj == null || exprMap == null || exprMap.isEmpty()) {
return;
}
- ExpressionMappingVisitor visitor = new ExpressionMappingVisitor(exprMap);
- if (preOrder) {
- PreOrderNavigator.doVisit(obj, visitor);
- } else {
- PostOrderNavigator.doVisit(obj, visitor);
- }
+ final Set reverseSet = new HashSet(exprMap.values());
+ final ExpressionMappingVisitor visitor = new ExpressionMappingVisitor(exprMap);
+ PreOrderNavigator pon = new PreOrderNavigator(visitor) {
+ @Override
+ protected void visitNode(LanguageObject obj) {
+ if (!(obj instanceof Expression) || !reverseSet.contains(obj)) {
+ super.visitNode(obj);
+ }
+ }
+ };
+ obj.acceptVisitor(pon);
}
protected void setVariableValues(Map variableValues) {
Modified:
trunk/engine/src/test/java/com/metamatrix/query/sql/visitor/TestExpressionMappingVisitor.java
===================================================================
---
trunk/engine/src/test/java/com/metamatrix/query/sql/visitor/TestExpressionMappingVisitor.java 2009-05-13
01:01:53 UTC (rev 922)
+++
trunk/engine/src/test/java/com/metamatrix/query/sql/visitor/TestExpressionMappingVisitor.java 2009-05-13
01:37:34 UTC (rev 923)
@@ -22,18 +22,23 @@
package com.metamatrix.query.sql.visitor;
+import static org.junit.Assert.*;
+
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import junit.framework.TestCase;
+import org.junit.Test;
+import org.teiid.connector.api.SourceSystemFunctions;
import com.metamatrix.query.sql.LanguageObject;
+import com.metamatrix.query.sql.ReservedWords;
import com.metamatrix.query.sql.lang.CompareCriteria;
import com.metamatrix.query.sql.lang.Select;
import com.metamatrix.query.sql.lang.SetCriteria;
+import com.metamatrix.query.sql.symbol.AggregateSymbol;
import com.metamatrix.query.sql.symbol.CaseExpression;
import com.metamatrix.query.sql.symbol.Constant;
import com.metamatrix.query.sql.symbol.ElementSymbol;
@@ -44,19 +49,15 @@
import com.metamatrix.query.sql.symbol.TestSearchedCaseExpression;
-public class TestExpressionMappingVisitor extends TestCase {
+public class TestExpressionMappingVisitor {
- public TestExpressionMappingVisitor(String arg0) {
- super(arg0);
- }
-
public void helpTest(LanguageObject original, Map map, LanguageObject expected) {
ExpressionMappingVisitor.mapExpressions(original, map);
assertEquals("Did not get expected mapped expression", expected,
original); //$NON-NLS-1$
}
- public void testCompareCriteria1() {
+ @Test public void testCompareCriteria1() {
ElementSymbol e1 = new ElementSymbol("e1"); //$NON-NLS-1$
Function f = new Function("+", new Expression[] { new Constant(new
Integer(2)), new Constant(new Integer(5)) }); //$NON-NLS-1$
Map map = new HashMap();
@@ -66,7 +67,7 @@
helpTest(before, map, after);
}
- public void testCompareCriteria2() {
+ @Test public void testCompareCriteria2() {
ElementSymbol e1 = new ElementSymbol("e1"); //$NON-NLS-1$
Function f = new Function("+", new Expression[] { new Constant(new
Integer(2)), new Constant(new Integer(5)) }); //$NON-NLS-1$
Map map = new HashMap();
@@ -76,7 +77,7 @@
helpTest(before, map, after);
}
- public void testFunction1() {
+ @Test public void testFunction1() {
ElementSymbol e1 = new ElementSymbol("e1"); //$NON-NLS-1$
ElementSymbol e2 = new ElementSymbol("e2"); //$NON-NLS-1$
ElementSymbol e3 = new ElementSymbol("e3"); //$NON-NLS-1$
@@ -100,7 +101,7 @@
helpTest(f3, map, f7);
}
- public void testSetCriteria() {
+ @Test public void testSetCriteria() {
ElementSymbol e1 = new ElementSymbol("e1"); //$NON-NLS-1$
ElementSymbol e2 = new ElementSymbol("e2"); //$NON-NLS-1$
Constant c1 = new Constant("xyz"); //$NON-NLS-1$
@@ -122,7 +123,7 @@
helpTest(before, map, after);
}
- public void testCaseExpression1() {
+ @Test public void testCaseExpression1() {
ElementSymbol x = new ElementSymbol("x"); //$NON-NLS-1$
ElementSymbol y = new ElementSymbol("y"); //$NON-NLS-1$
Constant a = new Constant(String.valueOf('a'));
@@ -145,7 +146,7 @@
helpTest(TestCaseExpression.example(3), map, mapped);
}
- public void testCaseExpression2() {
+ @Test public void testCaseExpression2() {
ElementSymbol x = new ElementSymbol("x"); //$NON-NLS-1$
ElementSymbol y = new ElementSymbol("y"); //$NON-NLS-1$
@@ -168,7 +169,7 @@
/**
* We do not need to create an alias if the canonical short names match
*/
- public void testSelectAlias() {
+ @Test public void testSelectAlias() {
ElementSymbol x = new ElementSymbol("y.x"); //$NON-NLS-1$
ElementSymbol y = new ElementSymbol("z.X"); //$NON-NLS-1$
@@ -181,6 +182,26 @@
assertEquals("Did not get expected mapped expression", "SELECT
z.X", toMap.toString()); //$NON-NLS-1$ //$NON-NLS-2$
}
-
+ /**
+ * we are not careful about ensuring that that every symbol is
+ * unique in a plan, so there's a chance mapping the expression in an
+ * aggregate in a project node will cause the same symbol to be
+ * updated in a sort node. to ensure that we don't get into
+ * recursion trouble we detect if we're replacing an expression
+ * that already exists as a mapping.
+ *
+ * we simulate that situation here using the same aggregate twice in
+ * a function.
+ */
+ @Test public void testRecursionDetection() {
+ ElementSymbol e1 = new ElementSymbol("e1"); //$NON-NLS-1$
+ AggregateSymbol a1 = new AggregateSymbol("x", ReservedWords.SUM, false,
e1); //$NON-NLS-1$
+ Function f = new Function(SourceSystemFunctions.ADD_OP, new Expression[] {a1, a1});
+ HashMap<AggregateSymbol, AggregateSymbol> map = new
HashMap<AggregateSymbol, AggregateSymbol>();
+ map.put(a1, new AggregateSymbol("x", ReservedWords.SUM, false, a1));
//$NON-NLS-1$
+ ExpressionMappingVisitor.mapExpressions(f, map);
+ assertEquals("(SUM(SUM(e1)) + SUM(SUM(e1)))", f.toString());
//$NON-NLS-1$
+ }
+
}