Author: shawkins Date: 2012-03-12 13:53:35 -0400 (Mon, 12 Mar 2012) New Revision: 3933 Modified: branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/RelationalPlanner.java branches/7.7.x/engine/src/main/java/org/teiid/query/rewriter/QueryRewriter.java branches/7.7.x/engine/src/main/java/org/teiid/query/sql/symbol/Reference.java branches/7.7.x/engine/src/main/java/org/teiid/query/validator/ValidationVisitor.java branches/7.7.x/engine/src/test/java/org/teiid/query/processor/TestSQLXMLProcessing.java branches/7.7.x/engine/src/test/java/org/teiid/query/validator/TestValidator.java Log: TEIID-1968 fix for NPE in grouping initialization Modified: branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/RelationalPlanner.java =================================================================== --- branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/RelationalPlanner.java 2012-03-09 20:18:08 UTC (rev 3932) +++ branches/7.7.x/engine/src/main/java/org/teiid/query/optimizer/relational/RelationalPlanner.java 2012-03-12 17:53:35 UTC (rev 3933) @@ -22,17 +22,7 @@ package org.teiid.query.optimizer.relational; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.LinkedHashSet; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; import org.teiid.api.exception.query.QueryMetadataException; import org.teiid.api.exception.query.QueryPlannerException; @@ -81,51 +71,11 @@ import org.teiid.query.sql.LanguageObject; import org.teiid.query.sql.LanguageVisitor; import org.teiid.query.sql.LanguageObject.Util; -import org.teiid.query.sql.lang.CacheHint; -import org.teiid.query.sql.lang.Command; -import org.teiid.query.sql.lang.Criteria; -import org.teiid.query.sql.lang.Delete; -import org.teiid.query.sql.lang.ExistsCriteria; -import org.teiid.query.sql.lang.From; -import org.teiid.query.sql.lang.FromClause; -import org.teiid.query.sql.lang.GroupBy; -import org.teiid.query.sql.lang.Insert; -import org.teiid.query.sql.lang.JoinPredicate; -import org.teiid.query.sql.lang.JoinType; -import org.teiid.query.sql.lang.Limit; -import org.teiid.query.sql.lang.Option; -import org.teiid.query.sql.lang.OrderBy; -import org.teiid.query.sql.lang.ProcedureContainer; -import org.teiid.query.sql.lang.Query; -import org.teiid.query.sql.lang.QueryCommand; -import org.teiid.query.sql.lang.Select; -import org.teiid.query.sql.lang.SetQuery; -import org.teiid.query.sql.lang.SourceHint; -import org.teiid.query.sql.lang.StoredProcedure; -import org.teiid.query.sql.lang.SubqueryContainer; -import org.teiid.query.sql.lang.SubqueryFromClause; -import org.teiid.query.sql.lang.TableFunctionReference; -import org.teiid.query.sql.lang.TargetedCommand; -import org.teiid.query.sql.lang.TranslatableProcedureContainer; -import org.teiid.query.sql.lang.UnaryFromClause; -import org.teiid.query.sql.lang.Update; -import org.teiid.query.sql.lang.WithQueryCommand; +import org.teiid.query.sql.lang.*; import org.teiid.query.sql.navigator.PreOrPostOrderNavigator; import org.teiid.query.sql.proc.CreateUpdateProcedureCommand; import org.teiid.query.sql.proc.TriggerAction; -import org.teiid.query.sql.symbol.AggregateSymbol; -import org.teiid.query.sql.symbol.AliasSymbol; -import org.teiid.query.sql.symbol.Constant; -import org.teiid.query.sql.symbol.ElementSymbol; -import org.teiid.query.sql.symbol.Expression; -import org.teiid.query.sql.symbol.ExpressionSymbol; -import org.teiid.query.sql.symbol.GroupSymbol; -import org.teiid.query.sql.symbol.MultipleElementSymbol; -import org.teiid.query.sql.symbol.Reference; -import org.teiid.query.sql.symbol.ScalarSubquery; -import org.teiid.query.sql.symbol.SelectSymbol; -import org.teiid.query.sql.symbol.SingleElementSymbol; -import org.teiid.query.sql.symbol.WindowFunction; +import org.teiid.query.sql.symbol.*; import org.teiid.query.sql.util.SymbolMap; import org.teiid.query.sql.visitor.AggregateSymbolCollectorVisitor; import org.teiid.query.sql.visitor.CorrelatedReferenceCollectorVisitor; @@ -317,6 +267,22 @@ Command subCommand = (Command)container.getCommand().clone(); ArrayList<Reference> correlatedReferences = new ArrayList<Reference>(); CorrelatedReferenceCollectorVisitor.collectReferences(subCommand, localGroupSymbols, correlatedReferences); + if (node.getType() != NodeConstants.Types.JOIN) { + PlanNode grouping = NodeEditor.findNodePreOrder(node, NodeConstants.Types.GROUP, NodeConstants.Types.SOURCE | NodeConstants.Types.JOIN); + if (grouping != null && !correlatedReferences.isEmpty()) { + SymbolMap map = (SymbolMap) grouping.getProperty(Info.SYMBOL_MAP); + Map<Expression, ElementSymbol> reverseMap = new HashMap<Expression, ElementSymbol>(); + for (Map.Entry<ElementSymbol, Expression> entry : map.asMap().entrySet()) { + reverseMap.put(entry.getValue(), entry.getKey()); + } + for (Reference reference : correlatedReferences) { + ElementSymbol correlatedGroupingCol = reverseMap.get(reference.getExpression()); + if (correlatedGroupingCol != null) { + reference.setExpression(correlatedGroupingCol); + } + } + } + } ProcessorPlan procPlan = QueryOptimizer.optimizePlan(subCommand, metadata, idGenerator, capFinder, analysisRecord, context); container.getCommand().setProcessorPlan(procPlan); setCorrelatedReferences(container, correlatedReferences); Modified: branches/7.7.x/engine/src/main/java/org/teiid/query/rewriter/QueryRewriter.java =================================================================== --- branches/7.7.x/engine/src/main/java/org/teiid/query/rewriter/QueryRewriter.java 2012-03-09 20:18:08 UTC (rev 3932) +++ branches/7.7.x/engine/src/main/java/org/teiid/query/rewriter/QueryRewriter.java 2012-03-12 17:53:35 UTC (rev 3933) @@ -1485,7 +1485,7 @@ * @return CompareCriteria */ private CompareCriteria simplifyMathematicalCriteria(CompareCriteria criteria) - throws TeiidComponentException, TeiidProcessingException{ + throws TeiidProcessingException{ Expression leftExpr = criteria.getLeftExpression(); Expression rightExpr = criteria.getRightExpression(); Modified: branches/7.7.x/engine/src/main/java/org/teiid/query/sql/symbol/Reference.java =================================================================== --- branches/7.7.x/engine/src/main/java/org/teiid/query/sql/symbol/Reference.java 2012-03-09 20:18:08 UTC (rev 3932) +++ branches/7.7.x/engine/src/main/java/org/teiid/query/sql/symbol/Reference.java 2012-03-12 17:53:35 UTC (rev 3933) @@ -78,7 +78,7 @@ this.positional = ref.positional; this.type = ref.type; if (ref.expression != null) { - this.expression = (ElementSymbol)ref.expression.clone(); + this.expression = ref.expression.clone(); } this.constraint = ref.constraint; } @@ -191,4 +191,9 @@ return this.positional; } + public void setExpression(ElementSymbol expression) { + assert this.expression != null && !this.positional; + this.expression = expression; + } + } Modified: branches/7.7.x/engine/src/main/java/org/teiid/query/validator/ValidationVisitor.java =================================================================== --- branches/7.7.x/engine/src/main/java/org/teiid/query/validator/ValidationVisitor.java 2012-03-09 20:18:08 UTC (rev 3932) +++ branches/7.7.x/engine/src/main/java/org/teiid/query/validator/ValidationVisitor.java 2012-03-12 17:53:35 UTC (rev 3933) @@ -57,83 +57,15 @@ import org.teiid.query.sql.LanguageObject; import org.teiid.query.sql.LanguageVisitor; import org.teiid.query.sql.ProcedureReservedWords; -import org.teiid.query.sql.lang.AlterProcedure; -import org.teiid.query.sql.lang.AlterTrigger; -import org.teiid.query.sql.lang.AlterView; -import org.teiid.query.sql.lang.BatchedUpdateCommand; -import org.teiid.query.sql.lang.BetweenCriteria; -import org.teiid.query.sql.lang.Command; -import org.teiid.query.sql.lang.CompareCriteria; -import org.teiid.query.sql.lang.CompoundCriteria; -import org.teiid.query.sql.lang.Create; -import org.teiid.query.sql.lang.Criteria; -import org.teiid.query.sql.lang.Delete; -import org.teiid.query.sql.lang.DependentSetCriteria; -import org.teiid.query.sql.lang.Drop; -import org.teiid.query.sql.lang.DynamicCommand; -import org.teiid.query.sql.lang.ExistsCriteria; -import org.teiid.query.sql.lang.GroupBy; -import org.teiid.query.sql.lang.Insert; -import org.teiid.query.sql.lang.Into; -import org.teiid.query.sql.lang.IsNullCriteria; -import org.teiid.query.sql.lang.Limit; -import org.teiid.query.sql.lang.MatchCriteria; -import org.teiid.query.sql.lang.NotCriteria; -import org.teiid.query.sql.lang.Option; -import org.teiid.query.sql.lang.OrderBy; -import org.teiid.query.sql.lang.OrderByItem; -import org.teiid.query.sql.lang.Query; -import org.teiid.query.sql.lang.QueryCommand; -import org.teiid.query.sql.lang.SPParameter; -import org.teiid.query.sql.lang.Select; -import org.teiid.query.sql.lang.SetClause; -import org.teiid.query.sql.lang.SetClauseList; -import org.teiid.query.sql.lang.SetCriteria; -import org.teiid.query.sql.lang.SetQuery; -import org.teiid.query.sql.lang.StoredProcedure; -import org.teiid.query.sql.lang.SubqueryCompareCriteria; -import org.teiid.query.sql.lang.SubqueryContainer; -import org.teiid.query.sql.lang.SubqueryFromClause; -import org.teiid.query.sql.lang.SubquerySetCriteria; -import org.teiid.query.sql.lang.TextTable; -import org.teiid.query.sql.lang.Update; -import org.teiid.query.sql.lang.WithQueryCommand; -import org.teiid.query.sql.lang.XMLTable; +import org.teiid.query.sql.lang.*; import org.teiid.query.sql.lang.SetQuery.Operation; import org.teiid.query.sql.lang.XMLTable.XMLColumn; +import org.teiid.query.sql.navigator.PreOrPostOrderNavigator; import org.teiid.query.sql.navigator.PreOrderNavigator; -import org.teiid.query.sql.proc.AssignmentStatement; -import org.teiid.query.sql.proc.Block; -import org.teiid.query.sql.proc.BranchingStatement; -import org.teiid.query.sql.proc.CommandStatement; -import org.teiid.query.sql.proc.CreateUpdateProcedureCommand; -import org.teiid.query.sql.proc.CriteriaSelector; -import org.teiid.query.sql.proc.DeclareStatement; -import org.teiid.query.sql.proc.HasCriteria; -import org.teiid.query.sql.proc.LoopStatement; -import org.teiid.query.sql.proc.TranslateCriteria; -import org.teiid.query.sql.proc.WhileStatement; +import org.teiid.query.sql.proc.*; import org.teiid.query.sql.proc.BranchingStatement.BranchingMode; import org.teiid.query.sql.proc.Statement.Labeled; -import org.teiid.query.sql.symbol.AggregateSymbol; -import org.teiid.query.sql.symbol.Constant; -import org.teiid.query.sql.symbol.DerivedColumn; -import org.teiid.query.sql.symbol.ElementSymbol; -import org.teiid.query.sql.symbol.Expression; -import org.teiid.query.sql.symbol.Function; -import org.teiid.query.sql.symbol.GroupSymbol; -import org.teiid.query.sql.symbol.QueryString; -import org.teiid.query.sql.symbol.Reference; -import org.teiid.query.sql.symbol.ScalarSubquery; -import org.teiid.query.sql.symbol.SingleElementSymbol; -import org.teiid.query.sql.symbol.TextLine; -import org.teiid.query.sql.symbol.WindowFunction; -import org.teiid.query.sql.symbol.XMLAttributes; -import org.teiid.query.sql.symbol.XMLElement; -import org.teiid.query.sql.symbol.XMLForest; -import org.teiid.query.sql.symbol.XMLNamespaces; -import org.teiid.query.sql.symbol.XMLParse; -import org.teiid.query.sql.symbol.XMLQuery; +import org.teiid.query.sql.symbol.*; import org.teiid.query.sql.symbol.AggregateSymbol.Type; import org.teiid.query.sql.visitor.AggregateSymbolCollectorVisitor; import org.teiid.query.sql.visitor.ElementCollectorVisitor; @@ -761,12 +693,14 @@ GroupBy groupBy = query.getGroupBy(); Criteria having = query.getHaving(); validateNoAggsInClause(groupBy); + List<GroupSymbol> correlationGroups = null; validateNoAggsInClause(query.getCriteria()); if (query.getFrom() == null) { validateNoAggsInClause(select); validateNoAggsInClause(query.getOrderBy()); } else { validateNoAggsInClause(query.getFrom()); + correlationGroups = query.getFrom().getGroups(); } Set<Expression> groupSymbols = null; @@ -779,10 +713,14 @@ LinkedHashSet<Expression> invalidWindowFunctions = new LinkedHashSet<Expression>(); LinkedList<AggregateSymbol> aggs = new LinkedList<AggregateSymbol>(); if (having != null) { + validateCorrelatedReferences(query, correlationGroups, groupSymbols, having, invalid); AggregateSymbolCollectorVisitor.getAggregates(having, aggs, invalid, null, invalidWindowFunctions, groupSymbols); hasAgg = true; } for (SingleElementSymbol symbol : select.getProjectedSymbols()) { + if (hasAgg || !aggs.isEmpty()) { + validateCorrelatedReferences(query, correlationGroups, groupSymbols, symbol, invalid); + } AggregateSymbolCollectorVisitor.getAggregates(symbol, aggs, invalid, null, null, groupSymbols); } if ((!aggs.isEmpty() || hasAgg) && !invalid.isEmpty()) { @@ -793,6 +731,26 @@ } } + /** + * This validation is more convoluted than needed since it is being run before rewrite/planning. + * Ideally we would already have correlated references set on the subqueries. + */ + private void validateCorrelatedReferences(Query query, + final List<GroupSymbol> correlationGroups, final Set<Expression> groupingSymbols, LanguageObject object, LinkedHashSet<Expression> invalid) { + if (query.getFrom() == null) { + return; + } + ElementCollectorVisitor ecv = new ElementCollectorVisitor(invalid) { + public void visit(ElementSymbol obj) { + if (obj.isExternalReference() && correlationGroups.contains(obj.getGroupSymbol()) + && (groupingSymbols == null || !groupingSymbols.contains(obj))) { + super.visit(obj); + } + } + }; + PreOrPostOrderNavigator.doVisit(object, ecv, PreOrPostOrderNavigator.PRE_ORDER, true); + } + private void validateNoAggsInClause(LanguageObject clause) { if (clause == null) { return; Modified: branches/7.7.x/engine/src/test/java/org/teiid/query/processor/TestSQLXMLProcessing.java =================================================================== --- branches/7.7.x/engine/src/test/java/org/teiid/query/processor/TestSQLXMLProcessing.java 2012-03-09 20:18:08 UTC (rev 3932) +++ branches/7.7.x/engine/src/test/java/org/teiid/query/processor/TestSQLXMLProcessing.java 2012-03-12 17:53:35 UTC (rev 3933) @@ -504,6 +504,15 @@ process(sql, expected); } + + @Test public void testXmlAggNested() throws Exception { + String sql = "SELECT XMLELEMENT(NAME metadata, XMLFOREST(e1), (SELECT XMLAGG(XMLELEMENT(NAME subTypes, XMLFOREST(e1))) FROM pm1.g2 AS b WHERE b.e1 = a.e1)) FROM pm1.g1 AS a where e1 = 'a' GROUP BY e1"; //$NON-NLS-1$ + + List<?>[] expected = new List<?>[] { + Arrays.asList("<metadata><gcol0>a</gcol0><subTypes><e1>a</e1></subTypes><subTypes><e1>a</e1></subTypes><subTypes><e1>a</e1></subTypes></metadata>"), + }; + + process(sql, expected); + } - } Modified: branches/7.7.x/engine/src/test/java/org/teiid/query/validator/TestValidator.java =================================================================== --- branches/7.7.x/engine/src/test/java/org/teiid/query/validator/TestValidator.java 2012-03-09 20:18:08 UTC (rev 3932) +++ branches/7.7.x/engine/src/test/java/org/teiid/query/validator/TestValidator.java 2012-03-12 17:53:35 UTC (rev 3933) @@ -1957,4 +1957,8 @@ helpValidate("select count(distinct e1) over (order by e2) as y from pm1.g1", new String[] {"COUNT(DISTINCT e1) OVER (ORDER BY e2)"}, RealMetadataFactory.example1Cached()); } + @Test public void testInvalidCorrelation() { + helpValidate("SELECT XMLELEMENT(NAME metadata, XMLFOREST(e1 AS objectName), (SELECT XMLAGG(XMLELEMENT(NAME subTypes, XMLFOREST(e1))) FROM pm1.g2 AS b WHERE b.e2 = a.e2)) FROM pm1.g1 AS a GROUP BY e1", new String[] {"a.e2"}, RealMetadataFactory.example1Cached()); + } + }