Skip to content

Commit

Permalink
TEIID-5331: LEAD/LAG ignores ORDER BY in the OVER clause (fixing lead…
Browse files Browse the repository at this point in the history 
…/lag)
  • Loading branch information
@shawkins @johnathonlee
shawkins authored and johnathonlee committed May 3, 2018
1 parent d8b9e55 commit 99af117
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 32 deletions.
3 changes: 3 additions & 0 deletions build/kits/jboss-as7/overlay/docs/teiid/teiid-releasenotes.html
View file
Expand Up @@ -294,6 +294,9 @@ <h4 class="western">from ${project.version}</h4>
<p style="margin-bottom: 0in">
<a href='https://issues.jboss.org/browse/TEIID-5330'>TEIID-5330</a> - FIRST_VALUE/LAST_VALUE/LEAD/LAG functions always try to return integer (fixing analytical resolving
)
<li/>
<p style="margin-bottom: 0in">
<a href='https://issues.jboss.org/browse/TEIID-5331'>TEIID-5331</a> - LEAD/LAG ignores ORDER BY in the OVER clause (fixing lead/lag)
</ul>

<h4 class="western">from 8.12.12.6_4</h4>
Expand Down
45 changes: 23 additions & 22 deletions engine/src/main/java/org/teiid/query/processor/relational/WindowFunctionProjectNode.java
View file
Expand Up @@ -256,26 +256,41 @@ protected TupleBatch nextBatchDirect() throws BlockedException,
List<WindowFunctionInfo> functions = entry.getValue().rowValuefunctions;
if (!functions.isEmpty()) {
List<?> valueRow = rowValueMapping[specIndex].find(idRow);
for (int i = 0; i < functions.size(); i++) {
WindowFunctionInfo wfi = functions.get(i);
Object value = valueRow.get(i+1);
outputRow.set(wfi.outputIndex, value);
}
}
functions = entry.getValue().functions;
if (!functions.isEmpty()) {
if (partitionMapping[specIndex] != null) {
idRow = partitionMapping[specIndex].find(idRow);
idRow = idRow.subList(1, 2);
} else {
idRow = SINGLE_VALUE_ID;
}
List<?> valueRow = valueMapping[specIndex].find(idRow);
for (int i = 0; i < functions.size(); i++) {
WindowFunctionInfo wfi = functions.get(i);
Object value = valueRow.get(i+1);
//special handling for lead lag
//an array value encodes what we need to know about
//the offset, default, and partition
if (wfi.function.getFunction().getAggregateFunction() == Type.LEAD
|| wfi.function.getFunction().getAggregateFunction() == Type.LAG) {
|| wfi.function.getFunction().getAggregateFunction() == Type.LAG) {
ArrayImpl array = (ArrayImpl)value;
Object[] args = array.getValues();
int offset = 1;
Object defaultValue = null;
if (args.length > 2) {
offset = (Integer) args[1];
if (args.length > 3) {
defaultValue = args[2];
}
offset = (Integer) args[1];
if (args.length > 3) {
defaultValue = args[2];
}
}
List<?> newIdRow = Arrays.asList(rowId+(wfi.function.getFunction().getAggregateFunction() == Type.LAG?-offset:offset));
List<?> newValueRow = rowValueMapping[specIndex].find(newIdRow);
List<?> newIdRow = Arrays.asList((Integer)idRow.get(0)+(wfi.function.getFunction().getAggregateFunction() == Type.LAG?-offset:offset));
List<?> newValueRow = valueMapping[specIndex].find(newIdRow);
if (newValueRow == null) {
value = defaultValue;
} else {
Expand All @@ -288,21 +303,7 @@ protected TupleBatch nextBatchDirect() throws BlockedException,
}
}
}
outputRow.set(wfi.outputIndex, value);
}
}
functions = entry.getValue().functions;
if (!functions.isEmpty()) {
if (partitionMapping[specIndex] != null) {
idRow = partitionMapping[specIndex].find(idRow);
idRow = idRow.subList(1, 2);
} else {
idRow = SINGLE_VALUE_ID;
}
List<?> valueRow = valueMapping[specIndex].find(idRow);
for (int i = 0; i < functions.size(); i++) {
WindowFunctionInfo wfi = functions.get(i);
outputRow.set(wfi.outputIndex, valueRow.get(i+1));
outputRow.set(wfi.outputIndex, value);
}
}
}
Expand Down
2 changes: 0 additions & 2 deletions engine/src/main/java/org/teiid/query/sql/symbol/AggregateSymbol.java
View file
Expand Up @@ -177,8 +177,6 @@ public Type getAggregateFunction() {
public boolean isRowValueFunction() {
switch (aggregate) {
case ROW_NUMBER:
case LEAD:
case LAG:
return true;
default:
return false;
Expand Down
58 changes: 50 additions & 8 deletions engine/src/test/java/org/teiid/query/processor/TestWindowFunctions.java
View file
Expand Up @@ -304,6 +304,25 @@ public class TestWindowFunctions {
helpProcess(plan, dataManager, expected);
}

@Test public void testPartitionedRowNumber1() throws Exception {
String sql = "select e1, e3, row_number() over (partition by e3 order by e1) as r from pm1.g1 order by r, e1";

List<?>[] expected = new List[] {
Arrays.asList(null, Boolean.FALSE, 1),
Arrays.asList("a", Boolean.TRUE, 1),
Arrays.asList("a", Boolean.FALSE, 2),
Arrays.asList("c", Boolean.TRUE, 2),
Arrays.asList("a", Boolean.FALSE, 3),
Arrays.asList("b", Boolean.FALSE, 4),
};

FakeDataManager dataManager = new FakeDataManager();
sampleData1(dataManager);
ProcessorPlan plan = helpGetPlan(sql, RealMetadataFactory.example1Cached(), TestOptimizer.getGenericFinder());

helpProcess(plan, dataManager, expected);
}

@Test public void testPartitionedDistinctCount() throws Exception {
String sql = "select e1, e3, count(distinct e1) over (partition by e3) as r from pm1.g1 order by e1, e3";

Expand Down Expand Up @@ -525,7 +544,7 @@ public class TestWindowFunctions {
@Test public void testLag() throws Exception {
BasicSourceCapabilities bsc = TestOptimizer.getTypicalCapabilities();

String sql = "SELECT e1, LAG(e1, 2, 'd') over (partition by e3 order by e2) from pm1.g1";
String sql = "SELECT e1, e3, e2, LAG(e1, 2, 'd') over (partition by e3 order by e2) from pm1.g1";

ProcessorPlan plan = TestOptimizer.getPlan(helpGetCommand(sql, RealMetadataFactory.example1Cached(), null),
RealMetadataFactory.example1Cached(), new DefaultCapabilitiesFinder(bsc),
Expand All @@ -537,20 +556,43 @@ public class TestWindowFunctions {
Arrays.asList("a", false, 1), Arrays.asList("b", false, 2), Arrays.asList("c", false, 0));

List<?>[] expected = new List<?>[] {
Arrays.asList("a", "d"),
Arrays.asList("b", "d"),
Arrays.asList("c", "a"),
Arrays.asList("a", "d"),
Arrays.asList("b", "d"),
Arrays.asList("c", "a"),
Arrays.asList("a", true, 1, "d"),
Arrays.asList("b", true, 2, "c"),
Arrays.asList("c", true, 0, "d"),
Arrays.asList("a", false, 1, "d"),
Arrays.asList("b", false, 2, "c"),
Arrays.asList("c", false, 0, "d"),
};

helpProcess(plan, dataMgr, expected);

bsc.setCapabilitySupport(Capability.ELEMENTARY_OLAP, true);
bsc.setCapabilitySupport(Capability.QUERY_AGGREGATES, true);

TestOptimizer.helpPlan(sql, RealMetadataFactory.example1Cached(), new String[] {"SELECT g_0.e1, LAG(g_0.e1, 2, 'd') OVER (PARTITION BY g_0.e3 ORDER BY g_0.e2) FROM pm1.g1 AS g_0"}, new DefaultCapabilitiesFinder(bsc), ComparisonMode.EXACT_COMMAND_STRING);
TestOptimizer.helpPlan(sql, RealMetadataFactory.example1Cached(), new String[] {"SELECT g_0.e1, g_0.e3, g_0.e2, LAG(g_0.e1, 2, 'd') OVER (PARTITION BY g_0.e3 ORDER BY g_0.e2) FROM pm1.g1 AS g_0"}, new DefaultCapabilitiesFinder(bsc), ComparisonMode.EXACT_COMMAND_STRING);
}

@Test public void testSimpleLead() throws Exception {
BasicSourceCapabilities bsc = TestOptimizer.getTypicalCapabilities();

String sql = "select stringkey, lead(stringkey) over (order by stringkey) l from bqt1.smalla order by l";

ProcessorPlan plan = TestOptimizer.getPlan(helpGetCommand(sql, RealMetadataFactory.exampleBQTCached(), null),
RealMetadataFactory.example1Cached(), new DefaultCapabilitiesFinder(bsc),
null, true, new CommandContext()); //$NON-NLS-1$

HardcodedDataManager dataMgr = new HardcodedDataManager();
dataMgr.addData("SELECT g_0.StringKey FROM BQT1.SmallA AS g_0",
Arrays.asList("b"), Arrays.asList("a"), Arrays.asList("c"), Arrays.asList("d"));

List<?>[] expected = new List<?>[] {
Arrays.asList("d", null),
Arrays.asList("a", "b"),
Arrays.asList("b", "c"),
Arrays.asList("c", "d"),
};

helpProcess(plan, dataMgr, expected);
}

}

0 comments on commit 99af117

Please sign in to comment.