[teiid-commits] teiid SVN: r3337 - in branches/7.4.x/engine/src: main/java/org/teiid/query/optimizer/relational/rules and 4 other directories.

Author: shawkins Date: 2011-07-27 06:59:10 -0400 (Wed, 27 Jul 2011) New Revision: 3337 Modified: branches/7.4.x/engine/src/main/java/org/teiid/query/optimizer/relational/PlanToProcessConverter.java branches/7.4.x/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePlanSorts.java branches/7.4.x/engine/src/main/java/org/teiid/query/processor/relational/GroupingNode.java branches/7.4.x/engine/src/main/java/org/teiid/query/processor/relational/SortUtility.java branches/7.4.x/engine/src/main/java/org/teiid/query/sql/symbol/AggregateSymbol.java branches/7.4.x/engine/src/main/java/org/teiid/query/sql/symbol/TextLine.java branches/7.4.x/engine/src/test/java/org/teiid/query/processor/TestAggregateProcessing.java branches/7.4.x/engine/src/test/java/org/teiid/query/processor/relational/TestGroupingNode.java Log: TEIID-1682 fixing duplicate removal when merged with group by Modified: branches/7.4.x/engine/src/main/java/org/teiid/query/optimizer/relational/PlanToProcessConverter.java =================================================================== --- branches/7.4.x/engine/src/main/java/org/teiid/query/optimizer/relational/PlanToProcessConverter.java 2011-07-26 20:20:59 UTC (rev 3336) +++ branches/7.4.x/engine/src/main/java/org/teiid/query/optimizer/relational/PlanToProcessConverter.java 2011-07-27 10:59:10 UTC (rev 3337) @@ -379,7 +379,6 @@ case NodeConstants.Types.GROUP: GroupingNode gnode = new GroupingNode(getID()); List<SingleElementSymbol> gCols = (List) node.getProperty(NodeConstants.Info.GROUP_COLS); - gnode.setGroupingElements( gCols ); gnode.setRemoveDuplicates(node.hasBooleanProperty(NodeConstants.Info.IS_DUP_REMOVAL)); orderBy = (OrderBy) node.getProperty(Info.SORT_ORDER); if (orderBy == null) { Modified: branches/7.4.x/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePlanSorts.java =================================================================== --- branches/7.4.x/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePlanSorts.java 2011-07-26 20:20:59 UTC (rev 3336) +++ branches/7.4.x/engine/src/main/java/org/teiid/query/optimizer/relational/rules/RulePlanSorts.java 2011-07-27 10:59:10 UTC (rev 3337) @@ -166,8 +166,28 @@ if (!node.hasCollectionProperty(NodeConstants.Info.GROUP_COLS)) { break; } + List<SingleElementSymbol> output = (List<SingleElementSymbol>)node.getProperty(Info.OUTPUT_COLS); + boolean cardinalityDependent = false; + for (SingleElementSymbol singleElementSymbol : output) { + if (singleElementSymbol instanceof AggregateSymbol) { + AggregateSymbol agg = (AggregateSymbol)singleElementSymbol; + if (agg.isCardinalityDependent()) { + cardinalityDependent = true; + break; + } + } + } if (mergeSortWithDupRemovalAcrossSource(node)) { node.setProperty(NodeConstants.Info.IS_DUP_REMOVAL, true); + if (cardinalityDependent) { + PlanNode source = NodeEditor.findNodePreOrder(node, NodeConstants.Types.SOURCE); + List<SingleElementSymbol> sourceOutput = (List<SingleElementSymbol>)source.getProperty(Info.OUTPUT_COLS); + PlanNode child = node.getFirstChild(); + while (child != source) { + child.setProperty(Info.OUTPUT_COLS, sourceOutput); + child = child.getFirstChild(); + } + } } //TODO: check the join interesting order parentBlocking = true; Modified: branches/7.4.x/engine/src/main/java/org/teiid/query/processor/relational/GroupingNode.java =================================================================== --- branches/7.4.x/engine/src/main/java/org/teiid/query/processor/relational/GroupingNode.java 2011-07-26 20:20:59 UTC (rev 3336) +++ branches/7.4.x/engine/src/main/java/org/teiid/query/processor/relational/GroupingNode.java 2011-07-27 10:59:10 UTC (rev 3337) @@ -40,7 +40,7 @@ import org.teiid.core.TeiidComponentException; import org.teiid.core.TeiidProcessingException; import org.teiid.language.SQLConstants.NonReserved; -import org.teiid.query.analysis.AnalysisRecord; +import org.teiid.language.SortSpecification.NullOrdering; import org.teiid.query.eval.Evaluator; import org.teiid.query.function.aggregate.AggregateFunction; import org.teiid.query.function.aggregate.ArrayAgg; @@ -56,6 +56,7 @@ import org.teiid.query.processor.BatchCollector; import org.teiid.query.processor.ProcessorDataManager; import org.teiid.query.processor.relational.SortUtility.Mode; +import org.teiid.query.sql.lang.OrderBy; import org.teiid.query.sql.lang.OrderByItem; import org.teiid.query.sql.symbol.AggregateSymbol; import org.teiid.query.sql.symbol.ElementSymbol; @@ -63,13 +64,13 @@ import org.teiid.query.sql.symbol.SingleElementSymbol; import org.teiid.query.sql.symbol.TextLine; import org.teiid.query.sql.symbol.AggregateSymbol.Type; +import org.teiid.query.sql.util.SymbolMap; import org.teiid.query.util.CommandContext; public class GroupingNode extends RelationalNode { // Grouping columns set by the planner - private List sortElements; private List<OrderByItem> orderBy; private boolean removeDuplicates; @@ -77,6 +78,7 @@ private int phase = COLLECTION; private Map elementMap; // Map of incoming symbol to index in source elements private List<Expression> collectedExpressions; // Collected Expressions + private int distinctCols = -1; // Sort phase private SortUtility sortUtility; @@ -118,17 +120,6 @@ this.removeDuplicates = removeDuplicates; } - /** - * Called by the planner to initialize the grouping node. Set the list of grouping - * expressions - these may be either elements or expressions and the list itself may - * be null to indicate an implied grouping on all columns - * @param groupingElements - * @since 4.2 - */ - public void setGroupingElements(List groupingElements) { - this.sortElements = groupingElements; - } - public void setOrderBy(List<OrderByItem> orderBy) { this.orderBy = orderBy; } @@ -143,13 +134,22 @@ } // Incoming elements and lookup map for evaluating expressions - List sourceElements = this.getChildren()[0].getElements(); + List<SingleElementSymbol> sourceElements = this.getChildren()[0].getElements(); this.elementMap = createLookupMap(sourceElements); // List should contain all grouping columns / expressions as we need those for sorting - if(this.sortElements != null) { - this.collectedExpressions = new ArrayList<Expression>(this.sortElements.size() + getElements().size()); - this.collectedExpressions.addAll(sortElements); + if(this.orderBy != null) { + this.collectedExpressions = new ArrayList<Expression>(this.orderBy.size() + getElements().size()); + for (OrderByItem item : this.orderBy) { + Expression ex = SymbolMap.getExpression(item.getSymbol()); + this.collectedExpressions.add(ex); + } + if (removeDuplicates) { + for (SingleElementSymbol ses : sourceElements) { + collectExpression(ses); + } + } + distinctCols = collectedExpressions.size(); } else { this.collectedExpressions = new ArrayList<Expression>(getElements().size()); } @@ -299,13 +299,32 @@ } private void collectionPhase() { - if(this.sortElements == null) { + if(this.orderBy == null) { // No need to sort this.groupTupleSource = getCollectionTupleSource(); this.phase = GROUP; } else { - this.sortUtility = new SortUtility(getCollectionTupleSource(), orderBy, removeDuplicates?Mode.DUP_REMOVE_SORT:Mode.SORT, getBufferManager(), - getConnectionID(), collectedExpressions); + List<NullOrdering> nullOrdering = new ArrayList<NullOrdering>(orderBy.size()); + List<Boolean> sortTypes = new ArrayList<Boolean>(orderBy.size()); + int size = orderBy.size(); + if (this.removeDuplicates) { + //sort on all inputs + size = distinctCols; + } + int[] sortIndexes = new int[size]; + for (int i = 0; i < size; i++) { + if (i < this.orderBy.size()) { + OrderByItem item = this.orderBy.get(i); + nullOrdering.add(item.getNullOrdering()); + sortTypes.add(item.isAscending()); + } else { + nullOrdering.add(null); + sortTypes.add(OrderBy.ASC); + } + sortIndexes[i] = i; + } + this.sortUtility = new SortUtility(getCollectionTupleSource(), removeDuplicates?Mode.DUP_REMOVE_SORT:Mode.SORT, getBufferManager(), + getConnectionID(), collectedExpressions, sortTypes, nullOrdering, sortIndexes); this.phase = SORT; } } @@ -353,7 +372,7 @@ updateAggregates(currentGroupTuple); currentGroupTuple = null; } - if(lastRow != null || sortElements == null) { + if(lastRow != null || orderBy == null) { // Close last group List row = new ArrayList(functions.length); for(int i=0; i<functions.length; i++) { @@ -369,12 +388,12 @@ private boolean sameGroup(List newTuple, List oldTuple) { // Check for no grouping columns - if(sortElements == null) { + if(orderBy == null) { return true; } // Walk backwards through sort cols as the last columns are most likely to be different - for(int i=sortElements.size()-1; i>=0; i--) { + for(int i=orderBy.size()-1; i>=0; i--) { Object oldValue = oldTuple.get(i); Object newValue = newTuple.get(i); @@ -412,13 +431,12 @@ protected void getNodeString(StringBuffer str) { super.getNodeString(str); - str.append(sortElements); + str.append(orderBy); } public Object clone(){ GroupingNode clonedNode = new GroupingNode(super.getID()); super.copy(this, clonedNode); - clonedNode.sortElements = sortElements; clonedNode.removeDuplicates = removeDuplicates; clonedNode.orderBy = orderBy; return clonedNode; @@ -428,17 +446,14 @@ // Default implementation - should be overridden PlanNode props = super.getDescriptionProperties(); - if(sortElements != null) { - int elements = sortElements.size(); + if(orderBy != null) { + int elements = orderBy.size(); List<String> groupCols = new ArrayList<String>(elements); for(int i=0; i<elements; i++) { - groupCols.add(this.sortElements.get(i).toString()); + groupCols.add(this.orderBy.get(i).toString()); } props.addProperty(PROP_GROUP_COLS, groupCols); } - if (orderBy != null) { - props.addProperty(AnalysisRecord.PROP_SORT_COLS, orderBy.toString()); - } props.addProperty(PROP_SORT_MODE, String.valueOf(this.removeDuplicates)); return props; Modified: branches/7.4.x/engine/src/main/java/org/teiid/query/processor/relational/SortUtility.java =================================================================== --- branches/7.4.x/engine/src/main/java/org/teiid/query/processor/relational/SortUtility.java 2011-07-26 20:20:59 UTC (rev 3336) +++ branches/7.4.x/engine/src/main/java/org/teiid/query/processor/relational/SortUtility.java 2011-07-27 10:59:10 UTC (rev 3337) @@ -45,6 +45,7 @@ import org.teiid.query.sql.lang.OrderBy; import org.teiid.query.sql.lang.OrderByItem; import org.teiid.query.sql.symbol.Expression; +import org.teiid.query.sql.symbol.SingleElementSymbol; /** @@ -109,18 +110,11 @@ public SortUtility(TupleSource sourceID, List<OrderByItem> items, Mode mode, BufferManager bufferMgr, String groupName, List schema) { - this.source = sourceID; - this.mode = mode; - this.bufferManager = bufferMgr; - this.groupName = groupName; - this.schema = schema; - this.schemaSize = bufferManager.getSchemaSize(this.schema); - int distinctIndex = items != null? items.size() - 1:0; List<Expression> sortElements = null; List<Boolean> sortTypes = null; List<NullOrdering> nullOrderings = null; if (items == null) { - sortElements = (List<Expression>) this.schema; + sortElements = (List<Expression>) schema; sortTypes = Collections.nCopies(sortElements.size(), OrderBy.ASC); } else { sortElements = new ArrayList(items.size()); @@ -147,7 +141,29 @@ cols[iter.previousIndex()] = schema.indexOf(elem); Assertion.assertTrue(cols[iter.previousIndex()] != -1); } + init(sourceID, mode, bufferMgr, groupName, schema, sortTypes, + nullOrderings, cols); + } + + public SortUtility(TupleSource sourceID, Mode mode, BufferManager bufferMgr, + String groupName, List<? extends Expression> schema, + List<Boolean> sortTypes, List<NullOrdering> nullOrderings, + int[] cols) { + init(sourceID, mode, bufferMgr, groupName, schema, sortTypes, nullOrderings, cols); + } + + private void init(TupleSource sourceID, Mode mode, BufferManager bufferMgr, + String groupName, List<? extends Expression> schema, + List<Boolean> sortTypes, List<NullOrdering> nullOrderings, + int[] cols) { + this.source = sourceID; + this.mode = mode; + this.bufferManager = bufferMgr; + this.groupName = groupName; + this.schema = schema; + this.schemaSize = bufferManager.getSchemaSize(this.schema); this.comparator = new ListNestedSortComparator(cols, sortTypes); + int distinctIndex = cols.length - 1; this.comparator.setDistinctIndex(distinctIndex); this.comparator.setNullOrdering(nullOrderings); } Modified: branches/7.4.x/engine/src/main/java/org/teiid/query/sql/symbol/AggregateSymbol.java =================================================================== --- branches/7.4.x/engine/src/main/java/org/teiid/query/sql/symbol/AggregateSymbol.java 2011-07-26 20:20:59 UTC (rev 3336) +++ branches/7.4.x/engine/src/main/java/org/teiid/query/sql/symbol/AggregateSymbol.java 2011-07-27 10:59:10 UTC (rev 3337) @@ -179,6 +179,8 @@ return DataTypeManager.DefaultDataClasses.DOUBLE; } else if (this.aggregate == Type.ARRAY_AGG) { return DataTypeManager.DefaultDataClasses.OBJECT; + } else if (this.aggregate == Type.TEXTAGG) { + return DataTypeManager.DefaultDataClasses.BLOB; } else { return this.getExpression().getType(); } Modified: branches/7.4.x/engine/src/main/java/org/teiid/query/sql/symbol/TextLine.java =================================================================== --- branches/7.4.x/engine/src/main/java/org/teiid/query/sql/symbol/TextLine.java 2011-07-26 20:20:59 UTC (rev 3336) +++ branches/7.4.x/engine/src/main/java/org/teiid/query/sql/symbol/TextLine.java 2011-07-27 10:59:10 UTC (rev 3337) @@ -88,7 +88,7 @@ @Override public Class<?> getType() { - return DataTypeManager.DefaultDataClasses.BLOB; + return DataTypeManager.DefaultDataClasses.STRING; } @Override Modified: branches/7.4.x/engine/src/test/java/org/teiid/query/processor/TestAggregateProcessing.java =================================================================== --- branches/7.4.x/engine/src/test/java/org/teiid/query/processor/TestAggregateProcessing.java 2011-07-26 20:20:59 UTC (rev 3336) +++ branches/7.4.x/engine/src/test/java/org/teiid/query/processor/TestAggregateProcessing.java 2011-07-27 10:59:10 UTC (rev 3337) @@ -399,5 +399,22 @@ // Run query helpProcess(plan, cc, dataManager, expected); } + + @Test public void testDupGroupCombination() throws Exception { + String sql = "select count(e2), e1 from (select distinct e1, e2, e3 from pm1.g1) x group by e1"; //$NON-NLS-1$ + List[] expected = new List[] { + Arrays.asList(2, "a"), + }; + + HardcodedDataManager dataManager = new HardcodedDataManager(); + dataManager.addData("SELECT pm1.g1.e1, pm1.g1.e2, pm1.g1.e3 FROM pm1.g1", new List[] { + Arrays.asList("a", 0, Boolean.TRUE), + Arrays.asList("a", 0, Boolean.FALSE), + }); + + ProcessorPlan plan = helpGetPlan(sql, RealMetadataFactory.example1Cached()); + helpProcess(plan, dataManager, expected); + } + } Modified: branches/7.4.x/engine/src/test/java/org/teiid/query/processor/relational/TestGroupingNode.java =================================================================== --- branches/7.4.x/engine/src/test/java/org/teiid/query/processor/relational/TestGroupingNode.java 2011-07-26 20:20:59 UTC (rev 3336) +++ branches/7.4.x/engine/src/test/java/org/teiid/query/processor/relational/TestGroupingNode.java 2011-07-27 10:59:10 UTC (rev 3337) @@ -22,7 +22,7 @@ package org.teiid.query.processor.relational; -import static org.junit.Assert.assertEquals; +import static org.junit.Assert.*; import java.math.BigDecimal; import java.util.ArrayList; @@ -45,6 +45,7 @@ import org.teiid.query.processor.FakeDataManager; import org.teiid.query.processor.FakeTupleSource; import org.teiid.query.processor.ProcessorDataManager; +import org.teiid.query.sql.lang.OrderBy; import org.teiid.query.sql.symbol.AggregateSymbol; import org.teiid.query.sql.symbol.Constant; import org.teiid.query.sql.symbol.ElementSymbol; @@ -153,7 +154,7 @@ List groupingElements = new ArrayList(); groupingElements.add(col1); - node.setGroupingElements(groupingElements); + node.setOrderBy(new OrderBy(groupingElements).getOrderByItems()); CommandContext context = new CommandContext("pid", "test", null, null, 1); //$NON-NLS-1$ //$NON-NLS-2$ List[] expected = new List[] { @@ -231,8 +232,6 @@ outputElements.add(new AggregateSymbol("bigAvg", "AVG", false, bigDecimal)); //$NON-NLS-1$ //$NON-NLS-2$ node.setElements(outputElements); - // Set grouping elements to null - node.setGroupingElements(null); CommandContext context = new CommandContext("pid", "test", null, null, 1); //$NON-NLS-1$ //$NON-NLS-2$ List[] data = new List[] { @@ -272,7 +271,7 @@ // Set grouping elements to null List groupingElements = new ArrayList(); groupingElements.add(col1); - node.setGroupingElements(groupingElements); + node.setOrderBy(new OrderBy(groupingElements).getOrderByItems()); CommandContext context = new CommandContext("pid", "test", null, null, 1); //$NON-NLS-1$ //$NON-NLS-2$ List[] data = new List[] { @@ -320,7 +319,7 @@ List groupingElements = new ArrayList(); groupingElements.add(col1); - node.setGroupingElements(groupingElements); + node.setOrderBy(new OrderBy(groupingElements).getOrderByItems()); CommandContext context = new CommandContext("pid", "test", null, null, 1); //$NON-NLS-1$ //$NON-NLS-2$ FakeDataManager dataMgr = new FakeDataManager(); @@ -366,7 +365,7 @@ if (groupBy) { List groupingElements = new ArrayList(); groupingElements.add(new ElementSymbol("col1")); //$NON-NLS-1$ - node.setGroupingElements(groupingElements); + node.setOrderBy(new OrderBy(groupingElements).getOrderByItems()); } CommandContext context = new CommandContext("pid", "test", null, null, 1); //$NON-NLS-1$ //$NON-NLS-2$ @@ -434,7 +433,7 @@ List groupingElements = new ArrayList(); groupingElements.add(col1); - node.setGroupingElements(groupingElements); + node.setOrderBy(new OrderBy(groupingElements).getOrderByItems()); return node; }