[teiid-commits] teiid SVN: r3337 - in branches/7.4.x/engine/src: main/java/org/teiid/query/optimizer/relational/rules and 4 other directories.
teiid-commits at lists.jboss.org
teiid-commits at lists.jboss.org
Wed Jul 27 06:59:11 EDT 2011
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;
}
More information about the teiid-commits
mailing list