Author: shawkins Date: 2012-06-15 13:53:09 -0400 (Fri, 15 Jun 2012) New Revision: 4182 Modified: trunk/engine/src/main/java/org/teiid/query/processor/relational/DependentCriteriaProcessor.java trunk/engine/src/main/java/org/teiid/query/processor/relational/DependentValueSource.java trunk/engine/src/test/java/org/teiid/query/processor/TestDependentJoins.java Log: TEIID-2074 fix for npe with dep join processing Modified: trunk/engine/src/main/java/org/teiid/query/processor/relational/DependentCriteriaProcessor.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/processor/relational/DependentCriteriaProcessor.java 2012-06-15 13:43:29 UTC (rev 4181) +++ trunk/engine/src/main/java/org/teiid/query/processor/relational/DependentCriteriaProcessor.java 2012-06-15 17:53:09 UTC (rev 4182) @@ -28,6 +28,7 @@ import org.teiid.common.buffer.BlockedException; import org.teiid.core.TeiidComponentException; import org.teiid.core.TeiidProcessingException; +import org.teiid.core.types.DataTypeManager; import org.teiid.logging.LogConstants; import org.teiid.logging.LogManager; import org.teiid.query.QueryPlugin; @@ -100,18 +101,43 @@ setState.valueIterator = dvs.getValueIterator(setState.valueExpression); int distinctCount = dvs.getTupleBuffer().getRowCount(); if (setState.maxNdv > 0 && setState.maxNdv < distinctCount) { - if (dvs.getTupleBuffer().getSchema().size() >= 1) { + if (dvs.getTupleBuffer().getSchema().size() > 1) { distinctCount = 0; ValueIterator vi = dvs.getValueIterator(setState.valueExpression); - Object last = null; - distinctCount = 0; - while (vi.hasNext()) { - Object next = vi.next(); - if (last == null || Constant.COMPARATOR.compare(next, last) != 0) { - distinctCount++; - } - last = next; - } + if (dvs.getTupleBuffer().getSchema().indexOf(setState.valueExpression) == 0) { + Object last = null; + while (vi.hasNext()) { + Object next = vi.next(); + if (next != null && (last == null || Constant.COMPARATOR.compare(next, last) != 0)) { + distinctCount++; + } + last = next; + } + } else { + //secondary attributes are not in sorted order, so we use an approximate count + Set<Object> set = null; + int maxSize = Math.min(10000, dvs.getTupleBuffer().getRowCount()); + List<Object> buffer = Arrays.asList(new Object[maxSize]); + if (!DataTypeManager.isHashable(setState.valueExpression.getType())) { + set = new TreeSet<Object>(Constant.COMPARATOR); + } else { + set = new HashSet<Object>(); + } + int i = 0; + while (vi.hasNext()) { + Object next = vi.next(); + if (next == null) { + continue; + } + if (set.add(next)) { + distinctCount++; + } + Object old = buffer.set(i++%maxSize, next); + if (set.size() > maxSize) { + set.remove(old); + } + } + } } if (!setState.overMax && distinctCount > setState.maxNdv) { LogManager.logWarning(LogConstants.CTX_DQP, QueryPlugin.Util.gs(QueryPlugin.Event.TEIID30011, valueSource, setState.valueExpression, setState.maxNdv)); Modified: trunk/engine/src/main/java/org/teiid/query/processor/relational/DependentValueSource.java =================================================================== --- trunk/engine/src/main/java/org/teiid/query/processor/relational/DependentValueSource.java 2012-06-15 13:43:29 UTC (rev 4181) +++ trunk/engine/src/main/java/org/teiid/query/processor/relational/DependentValueSource.java 2012-06-15 17:53:09 UTC (rev 4182) @@ -34,6 +34,7 @@ import org.teiid.core.TeiidProcessingException; import org.teiid.core.types.DataTypeManager; import org.teiid.core.util.Assertion; +import org.teiid.query.sql.symbol.Constant; import org.teiid.query.sql.symbol.Expression; import org.teiid.query.sql.util.ValueIterator; import org.teiid.query.sql.util.ValueIteratorSource; @@ -89,7 +90,7 @@ Assertion.assertTrue(index != -1); Class<?> type = ((Expression)buffer.getSchema().get(index)).getType(); if (!DataTypeManager.isHashable(type)) { - result = new TreeSet<Object>(); + result = new TreeSet<Object>(Constant.COMPARATOR); } else { result = new HashSet<Object>(); } Modified: trunk/engine/src/test/java/org/teiid/query/processor/TestDependentJoins.java =================================================================== --- trunk/engine/src/test/java/org/teiid/query/processor/TestDependentJoins.java 2012-06-15 13:43:29 UTC (rev 4181) +++ trunk/engine/src/test/java/org/teiid/query/processor/TestDependentJoins.java 2012-06-15 17:53:09 UTC (rev 4182) @@ -569,15 +569,16 @@ dataMgr.registerTuples( metadata, "pm1.g1", new List[] { - Arrays.asList(new Object[] { "a", 0, Boolean.FALSE, new Double(2.0) }), //$NON-NLS-1$ - Arrays.asList(new Object[] { "b", 1, Boolean.TRUE, null }), //$NON-NLS-1$ + Arrays.asList(new Object[] { "a", 0, Boolean.FALSE, new Double(2.0) }), //$NON-NLS-1$ + Arrays.asList(new Object[] { "q", null, Boolean.FALSE, new Double(0.0) }), //$NON-NLS-1$ + Arrays.asList(new Object[] { "b", 1, Boolean.TRUE, null }), //$NON-NLS-1$ Arrays.asList(new Object[] { "c", 2, Boolean.FALSE, new Double(0.0) }), //$NON-NLS-1$ } ); dataMgr.registerTuples( metadata, "pm6.g1", new List[] { - Arrays.asList(new Object[] { "b", 0 }), //$NON-NLS-1$ + Arrays.asList(new Object[] { "b", 1 }), //$NON-NLS-1$ Arrays.asList(new Object[] { "d", 3 }), //$NON-NLS-1$ Arrays.asList(new Object[] { "e", 1 }), //$NON-NLS-1$ } ); @@ -840,7 +841,7 @@ FakeDataManager dataManager = helpTestBackoff(true); //note that the dependent join was not actually performed - assertEquals(new HashSet<String>(Arrays.asList("SELECT pm1.g1.e1 FROM pm1.g1", "SELECT pm6.g1.e1 FROM pm6.g1 ORDER BY pm6.g1.e1")), + assertEquals(new HashSet<String>(Arrays.asList("SELECT pm6.g1.e1, pm6.g1.e2 FROM pm6.g1 ORDER BY pm6.g1.e1, pm6.g1.e2", "SELECT pm1.g1.e1, pm1.g1.e2 FROM pm1.g1")), new HashSet<String>(dataManager.getQueries())); } @@ -895,7 +896,7 @@ QueryMetadataException, TeiidComponentException, TeiidProcessingException { // Create query - String sql = "SELECT pm1.g1.e1 FROM pm1.g1, pm6.g1 WHERE pm1.g1.e1=pm6.g1.e1"; //$NON-NLS-1$ + String sql = "SELECT pm1.g1.e1 FROM pm1.g1, pm6.g1 WHERE pm1.g1.e1=pm6.g1.e1 and pm1.g1.e2=pm6.g1.e2"; //$NON-NLS-1$ // Construct data manager with data FakeDataManager dataManager = new FakeDataManager(); @@ -906,10 +907,12 @@ RealMetadataFactory.setCardinality("pm1.g1", 1, fakeMetadata); if (setNdv) { fakeMetadata.getElementID("pm1.g1.e1").setDistinctValues(1); + fakeMetadata.getElementID("pm1.g1.e2").setDistinctValues(1); } RealMetadataFactory.setCardinality("pm6.g1", 1000, fakeMetadata); if (setNdv) { fakeMetadata.getElementID("pm6.g1.e1").setDistinctValues(1000); + fakeMetadata.getElementID("pm6.g1.e2").setDistinctValues(1000); } // Plan query FakeCapabilitiesFinder capFinder = new FakeCapabilitiesFinder(); @@ -929,8 +932,8 @@ new String("b")})}; //$NON-NLS-1$ ProcessorPlan plan = TestOptimizer.helpPlan(sql, fakeMetadata, new String[] { - "SELECT pm6.g1.e1 FROM pm6.g1 WHERE pm6.g1.e1 IN (<dependent values>) ORDER BY pm6.g1.e1", - "SELECT pm1.g1.e1 FROM pm1.g1" + "SELECT pm6.g1.e1, pm6.g1.e2 FROM pm6.g1 WHERE (pm6.g1.e1 IN (<dependent values>)) AND (pm6.g1.e2 IN (<dependent values>)) ORDER BY pm6.g1.e1, pm6.g1.e2", + "SELECT pm1.g1.e1, pm1.g1.e2 FROM pm1.g1" }, capFinder, ComparisonMode.EXACT_COMMAND_STRING); // Run query