Skip to content

Commit 7a9526c

Browse files
lbooker42lbooker42
authored
fix: correct OuterJoinTools.leftOuterJoin/fullOuterJoin() output when RHS initally empty (deephaven#6548)
1 parent 3b3661a commit 7a9526c

File tree

3 files changed

+233
-6
lines changed

3 files changed

+233
-6
lines changed

‎engine/table/src/main/java/io/deephaven/engine/table/impl/CrossJoinHelper.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import io.deephaven.engine.table.impl.sources.BitMaskingColumnSource;
1515
import io.deephaven.engine.table.impl.sources.BitShiftingColumnSource;
1616
import io.deephaven.engine.table.impl.sources.CrossJoinRightColumnSource;
17+
import io.deephaven.engine.table.impl.sources.NullValueColumnSource;
1718
import io.deephaven.util.SafeCloseableList;
1819
import io.deephaven.util.mutable.MutableInt;
1920
import io.deephaven.util.mutable.MutableLong;
@@ -1418,7 +1419,11 @@ private static <T extends ColumnSource<?>> QueryTable makeResult(
14181419
}
14191420

14201421
for (MatchPair mp : columnsToAdd) {
1421-
final T wrappedSource = newRightColumnSource.apply(rightTable.getColumnSource(mp.rightColumn()));
1422+
// If rhs is empty and static, can substitute with a NullValueColumnSource
1423+
final ColumnSource<?> rcs = rightTable.getColumnSource(mp.rightColumn());
1424+
final ColumnSource<?> wrappedSource = rightTable.isEmpty() && !rightTable.isRefreshing()
1425+
? NullValueColumnSource.getInstance(rcs.getType(), rcs.getComponentType())
1426+
: newRightColumnSource.apply(rcs);
14221427
columnSourceMap.put(mp.leftColumn(), wrappedSource);
14231428
}
14241429

‎engine/table/src/main/java/io/deephaven/engine/table/impl/sources/BitMaskingColumnSource.java

+3-1
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,11 @@ public class BitMaskingColumnSource<T> extends AbstractColumnSource<T> implement
3030
public static <T> ColumnSource<T> maybeWrap(
3131
final ZeroKeyCrossJoinShiftState shiftState,
3232
@NotNull final ColumnSource<T> innerSource) {
33-
if (innerSource instanceof RowKeyAgnosticChunkSource) {
33+
if (innerSource instanceof NullValueColumnSource) {
3434
return innerSource;
3535
}
36+
// We must wrap all other sources to leverage shiftState.rightEmpty() and shiftState.rightEmptyPrev()
37+
// before calling the inner source.
3638
return new BitMaskingColumnSource<>(shiftState, innerSource);
3739
}
3840

‎engine/table/src/test/java/io/deephaven/engine/util/TestOuterJoinTools.java

+224-4
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,22 @@
33
//
44
package io.deephaven.engine.util;
55

6+
import io.deephaven.engine.context.ExecutionContext;
7+
import io.deephaven.engine.rowset.RowSet;
8+
import io.deephaven.engine.rowset.RowSetFactory;
69
import io.deephaven.engine.table.Table;
10+
import io.deephaven.engine.table.impl.QueryTable;
711
import io.deephaven.engine.table.vectors.ColumnVectors;
12+
import io.deephaven.engine.testutil.ControlledUpdateGraph;
13+
import io.deephaven.engine.testutil.TstUtils;
814
import io.deephaven.engine.testutil.junit4.EngineCleanup;
915
import org.junit.Rule;
1016
import org.junit.Test;
1117

12-
import static io.deephaven.engine.testutil.TstUtils.testRefreshingTable;
13-
import static io.deephaven.engine.testutil.TstUtils.testTable;
14-
import static io.deephaven.engine.util.TableTools.col;
15-
import static io.deephaven.engine.util.TableTools.intCol;
18+
import java.util.Collections;
19+
20+
import static io.deephaven.engine.testutil.TstUtils.*;
21+
import static io.deephaven.engine.util.TableTools.*;
1622
import static io.deephaven.util.QueryConstants.NULL_INT;
1723
import static org.junit.Assert.assertArrayEquals;
1824
import static org.junit.Assert.assertEquals;
@@ -253,4 +259,218 @@ public void testFullOuterJoinIdentityMatchWithAddColumn() {
253259
assertArrayEquals(new int[] {0, 2, 4, 6}, ColumnVectors.ofInt(result, "b").toArray());
254260
assertArrayEquals(new int[] {0, 3, 6, 9}, ColumnVectors.ofInt(result, "c").toArray());
255261
}
262+
263+
@Test
264+
public void testStaticEmptyRightZeroKey() {
265+
final Table lhs = TableTools.emptyTable(5).update("I=i");
266+
final Table rhs = TableTools.emptyTable(0).update("J=`asdf`");
267+
268+
Table result = OuterJoinTools.leftOuterJoin(lhs, rhs, Collections.emptyList());
269+
Table expected = lhs.update("J=(String)(null)");
270+
assertTableEquals(expected, result);
271+
272+
result = OuterJoinTools.fullOuterJoin(lhs, rhs, Collections.emptyList());
273+
expected = lhs.update("J=(String)(null)");
274+
assertTableEquals(expected, result);
275+
}
276+
277+
@Test
278+
public void testDynamicRightLeftOuterJoinZeroKey() {
279+
final Table lhs = testRefreshingTable(intCol("I", 0, 1));
280+
final QueryTable rhsSource = testRefreshingTable(stringCol("J"));
281+
282+
// This update creates a SingleValueColumnSource for the RHS table, which causes problems if we query the
283+
// column source for a value that doesn't exist in the table.
284+
final Table rhs = rhsSource.update("J = `asdf`");
285+
286+
final Table result = OuterJoinTools.leftOuterJoin(lhs, rhs, Collections.emptyList());
287+
288+
assertEquals(2, result.numColumns());
289+
assertEquals("I", result.getDefinition().getColumnsArray()[0].getName());
290+
assertEquals("J", result.getDefinition().getColumnsArray()[1].getName());
291+
292+
assertEquals(2, result.size());
293+
assertArrayEquals(new int[] {0, 1}, ColumnVectors.ofInt(result, "I").toArray());
294+
assertArrayEquals(new String[] {null, null}, ColumnVectors.ofObject(result, "J", String.class).toArray());
295+
296+
ControlledUpdateGraph updateGraph = ExecutionContext.getContext().getUpdateGraph().cast();
297+
298+
// Add some matching and non-matching rows to RHS
299+
updateGraph.runWithinUnitTestCycle(() -> {
300+
final RowSet newKeys = i(10);
301+
TstUtils.addToTable(rhsSource, newKeys, stringCol("J", "anything"));
302+
rhsSource.notifyListeners(newKeys, RowSetFactory.empty(), RowSetFactory.empty());
303+
});
304+
305+
assertEquals(2, result.size());
306+
assertArrayEquals(new int[] {0, 1}, ColumnVectors.ofInt(result, "I").toArray());
307+
assertArrayEquals(new String[] {"asdf", "asdf"}, ColumnVectors.ofObject(result, "J", String.class).toArray());
308+
309+
// Remove all rows from RHS
310+
updateGraph.runWithinUnitTestCycle(() -> {
311+
final RowSet removedKeys = i(10);
312+
TstUtils.removeRows(rhsSource, removedKeys);
313+
rhsSource.notifyListeners(RowSetFactory.empty(), removedKeys, RowSetFactory.empty());
314+
});
315+
316+
assertEquals(2, result.size());
317+
assertArrayEquals(new int[] {0, 1}, ColumnVectors.ofInt(result, "I").toArray());
318+
assertArrayEquals(new String[] {null, null}, ColumnVectors.ofObject(result, "J", String.class).toArray());
319+
}
320+
321+
@Test
322+
public void testDynamicRightFullOuterJoinZeroKey() {
323+
final Table lhs = testRefreshingTable(intCol("I", 0, 1));
324+
final QueryTable rhsSource = testRefreshingTable(stringCol("J"));
325+
326+
// This update creates a SingleValueColumnSource for the RHS table, which causes problems if we query the
327+
// column source for a value that doesn't exist in the table.
328+
final Table rhs = rhsSource.update("J = `asdf`");
329+
330+
final Table result = OuterJoinTools.fullOuterJoin(lhs, rhs, Collections.emptyList());
331+
332+
assertEquals(2, result.numColumns());
333+
assertEquals("I", result.getDefinition().getColumnsArray()[0].getName());
334+
assertEquals("J", result.getDefinition().getColumnsArray()[1].getName());
335+
336+
assertEquals(2, result.size());
337+
assertArrayEquals(new int[] {0, 1}, ColumnVectors.ofInt(result, "I").toArray());
338+
assertArrayEquals(new String[] {null, null}, ColumnVectors.ofObject(result, "J", String.class).toArray());
339+
340+
ControlledUpdateGraph updateGraph = ExecutionContext.getContext().getUpdateGraph().cast();
341+
342+
// Add some matching and non-matching rows to RHS
343+
updateGraph.runWithinUnitTestCycle(() -> {
344+
final RowSet newKeys = i(10);
345+
TstUtils.addToTable(rhsSource, newKeys, stringCol("J", "anything"));
346+
rhsSource.notifyListeners(newKeys, RowSetFactory.empty(), RowSetFactory.empty());
347+
});
348+
349+
assertEquals(2, result.size());
350+
assertArrayEquals(new int[] {0, 1}, ColumnVectors.ofInt(result, "I").toArray());
351+
assertArrayEquals(new String[] {"asdf", "asdf"}, ColumnVectors.ofObject(result, "J", String.class).toArray());
352+
353+
// Remove all rows from RHS
354+
updateGraph.runWithinUnitTestCycle(() -> {
355+
final RowSet removedKeys = i(10);
356+
TstUtils.removeRows(rhsSource, removedKeys);
357+
rhsSource.notifyListeners(RowSetFactory.empty(), removedKeys, RowSetFactory.empty());
358+
});
359+
360+
assertEquals(2, result.size());
361+
assertArrayEquals(new int[] {0, 1}, ColumnVectors.ofInt(result, "I").toArray());
362+
assertArrayEquals(new String[] {null, null}, ColumnVectors.ofObject(result, "J", String.class).toArray());
363+
}
364+
365+
@Test
366+
public void testStaticEmptyRight() {
367+
final Table lhs = TableTools.emptyTable(5).update("I=i");
368+
final Table rhs = TableTools.emptyTable(0).update("J=i", "K=`asdf`");
369+
370+
Table result = OuterJoinTools.leftOuterJoin(lhs, rhs, "I=J");
371+
Table expected = lhs.update("J=(int)(null)", "K=(String)(null)");
372+
assertTableEquals(expected, result);
373+
374+
result = OuterJoinTools.fullOuterJoin(lhs, rhs, "I=J");
375+
expected = lhs.update("J=(int)(null)", "K=(String)(null)");
376+
assertTableEquals(expected, result);
377+
}
378+
379+
@Test
380+
public void testDynamicRightLeftOuterJoin() {
381+
final Table lhs = testRefreshingTable(intCol("I", 0, 1));
382+
final QueryTable rhsSource = testRefreshingTable(shortCol("J"));
383+
384+
// This update creates a SingleValueColumnSource for the RHS table, which causes problems if we query the
385+
// column source for a value that doesn't exist in the table.
386+
final Table rhs = rhsSource.update("J=(int)1", "K=`asdf`");
387+
388+
final Table result = OuterJoinTools.leftOuterJoin(lhs, rhs, "I=J");
389+
390+
assertEquals(3, result.numColumns());
391+
assertEquals("I", result.getDefinition().getColumnsArray()[0].getName());
392+
assertEquals("J", result.getDefinition().getColumnsArray()[1].getName());
393+
assertEquals("K", result.getDefinition().getColumnsArray()[2].getName());
394+
395+
assertEquals(2, result.size());
396+
assertArrayEquals(new int[] {0, 1}, ColumnVectors.ofInt(result, "I").toArray());
397+
assertArrayEquals(new int[] {NULL_INT, NULL_INT}, ColumnVectors.ofInt(result, "J").toArray());
398+
assertArrayEquals(new String[] {null, null}, ColumnVectors.ofObject(result, "K", String.class).toArray());
399+
400+
ControlledUpdateGraph updateGraph = ExecutionContext.getContext().getUpdateGraph().cast();
401+
402+
// Add some rows to RHS
403+
updateGraph.runWithinUnitTestCycle(() -> {
404+
final RowSet newKeys = i(10, 11);
405+
TstUtils.addToTable(rhsSource, newKeys, shortCol("J", (short) 100, (short) 200));
406+
rhsSource.notifyListeners(newKeys, RowSetFactory.empty(), RowSetFactory.empty());
407+
});
408+
409+
assertEquals(3, result.size());
410+
assertArrayEquals(new int[] {0, 1, 1}, ColumnVectors.ofInt(result, "I").toArray());
411+
assertArrayEquals(new int[] {NULL_INT, 1, 1}, ColumnVectors.ofInt(result, "J").toArray());
412+
assertArrayEquals(new String[] {null, "asdf", "asdf"},
413+
ColumnVectors.ofObject(result, "K", String.class).toArray());
414+
415+
// Remove all rows from RHS
416+
updateGraph.runWithinUnitTestCycle(() -> {
417+
final RowSet removedKeys = i(10, 11);
418+
TstUtils.removeRows(rhsSource, removedKeys);
419+
rhsSource.notifyListeners(RowSetFactory.empty(), removedKeys, RowSetFactory.empty());
420+
});
421+
422+
assertEquals(2, result.size());
423+
assertArrayEquals(new int[] {0, 1}, ColumnVectors.ofInt(result, "I").toArray());
424+
assertArrayEquals(new int[] {NULL_INT, NULL_INT}, ColumnVectors.ofInt(result, "J").toArray());
425+
assertArrayEquals(new String[] {null, null}, ColumnVectors.ofObject(result, "K", String.class).toArray());
426+
}
427+
428+
@Test
429+
public void testDynamicRightFullOuterJoin() {
430+
final Table lhs = testRefreshingTable(intCol("I", 0, 1));
431+
final QueryTable rhsSource = testRefreshingTable(shortCol("J"));
432+
433+
// This update creates a SingleValueColumnSource for the RHS table, which causes problems if we query the
434+
// column source for a value that doesn't exist in the table.
435+
final Table rhs = rhsSource.update("J=(int)1", "K=`asdf`");
436+
437+
final Table result = OuterJoinTools.fullOuterJoin(lhs, rhs, "I=J");
438+
439+
assertEquals(3, result.numColumns());
440+
assertEquals("I", result.getDefinition().getColumnsArray()[0].getName());
441+
assertEquals("J", result.getDefinition().getColumnsArray()[1].getName());
442+
assertEquals("K", result.getDefinition().getColumnsArray()[2].getName());
443+
444+
assertEquals(2, result.size());
445+
assertArrayEquals(new int[] {0, 1}, ColumnVectors.ofInt(result, "I").toArray());
446+
assertArrayEquals(new int[] {NULL_INT, NULL_INT}, ColumnVectors.ofInt(result, "J").toArray());
447+
assertArrayEquals(new String[] {null, null}, ColumnVectors.ofObject(result, "K", String.class).toArray());
448+
449+
ControlledUpdateGraph updateGraph = ExecutionContext.getContext().getUpdateGraph().cast();
450+
451+
// Add some rows to RHS
452+
updateGraph.runWithinUnitTestCycle(() -> {
453+
final RowSet newKeys = i(10, 11);
454+
TstUtils.addToTable(rhsSource, newKeys, shortCol("J", (short) 100, (short) 200));
455+
rhsSource.notifyListeners(newKeys, RowSetFactory.empty(), RowSetFactory.empty());
456+
});
457+
458+
assertEquals(3, result.size());
459+
assertArrayEquals(new int[] {0, 1, 1}, ColumnVectors.ofInt(result, "I").toArray());
460+
assertArrayEquals(new int[] {NULL_INT, 1, 1}, ColumnVectors.ofInt(result, "J").toArray());
461+
assertArrayEquals(new String[] {null, "asdf", "asdf"},
462+
ColumnVectors.ofObject(result, "K", String.class).toArray());
463+
464+
// Remove all rows from RHS
465+
updateGraph.runWithinUnitTestCycle(() -> {
466+
final RowSet removedKeys = i(10, 11);
467+
TstUtils.removeRows(rhsSource, removedKeys);
468+
rhsSource.notifyListeners(RowSetFactory.empty(), removedKeys, RowSetFactory.empty());
469+
});
470+
471+
assertEquals(2, result.size());
472+
assertArrayEquals(new int[] {0, 1}, ColumnVectors.ofInt(result, "I").toArray());
473+
assertArrayEquals(new int[] {NULL_INT, NULL_INT}, ColumnVectors.ofInt(result, "J").toArray());
474+
assertArrayEquals(new String[] {null, null}, ColumnVectors.ofObject(result, "K", String.class).toArray());
475+
}
256476
}

0 commit comments

Comments
 (0)