Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

MultiJoin - correct bug when multiple columns are supplied #4279

Merged
merged 24 commits into from
Aug 7, 2023
Merged

MultiJoin - correct bug when multiple columns are supplied #4279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
44938c0
Initial commit, added major chunk-hashing files, now compiles.
lbooker42 Jun 15, 2023
6996754
Added tests and fillChunk fixes.
lbooker42 Jun 22, 2023
e22673a
MultiJoin static OA Hashing implemented and tests passing.
lbooker42 Jun 30, 2023
8d8a40f
MultiJoin incremental OA Hashing implemented and tests passing.
lbooker42 Jul 11, 2023
d697b54
Pre-review cleanup
lbooker42 Jul 11, 2023
e6e9f07
Ready for review.
lbooker42 Jul 11, 2023
fb0f9eb
JoinMatch and JoinAddition conversion. Improved tests.
lbooker42 Jul 12, 2023
175c540
Refactored to return MultiJoinTable and expose table() interface.
lbooker42 Jul 13, 2023
d481933
Re-implemented as factory interface and added to engine-api.
lbooker42 Jul 19, 2023
e06a520
More and better documentation.
lbooker42 Jul 20, 2023
0ae0b18
Addressed PR comments, added tests for null-fill behavior.
lbooker42 Jul 28, 2023
1c23393
Large commit addressing multiple PR comments.
lbooker42 Aug 1, 2023
5fc733e
Another large commit addressing multiple PR comments.
lbooker42 Aug 2, 2023
760cdec
Finishing up round 1 PR comments.
lbooker42 Aug 2, 2023
e678bfb
Bug-fix for last commit.
lbooker42 Aug 2, 2023
32ecb5b
Documentation nd explanations.
lbooker42 Aug 2, 2023
b05ec11
Self-review, minor harmonizing changes.
lbooker42 Aug 2, 2023
cc5dd2f
Round 2 PR comments. Getting closer.
lbooker42 Aug 3, 2023
eae04c5
Another round of PR comments, hopefully final.
lbooker42 Aug 3, 2023
5bb26b7
Bug fix and test for multiple columns in MuliJoinInput.
lbooker42 Aug 7, 2023
5bf0b4a
Merged with main
lbooker42 Aug 7, 2023
61d1a13
Rewrite test, assert whitespace handled correctly.
lbooker42 Aug 7, 2023
9a5b868
Rewrite test, assert whitespace handled correctly.
lbooker42 Aug 7, 2023
d15774a
Assert `==` for JoinMatch is handled.
lbooker42 Aug 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions engine/api/src/main/java/io/deephaven/engine/table/MultiJoinInput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,13 +83,14 @@ public static MultiJoinInput of(@NotNull final Table inputTable, @NotNull final
* or "SourceColumnToAddWithSameName"); empty for all columns
*/
public static MultiJoinInput of(@NotNull final Table inputTable, String columnsToMatch, String columnsToAdd) {
// Need to split the columnsToMatch and columnsToAdd by commas.
return of(inputTable,
columnsToMatch == null || columnsToMatch.isEmpty()
? Collections.emptyList()
: JoinMatch.from(columnsToMatch),
: JoinMatch.from(columnsToMatch.split(",")),
columnsToAdd == null || columnsToAdd.isEmpty()
? Collections.emptyList()
: JoinAddition.from(columnsToAdd));
: JoinAddition.from(columnsToAdd.split(",")));
}

/**
Expand Down
34 changes: 30 additions & 4 deletions engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableMultiJoinTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,7 @@
import io.deephaven.engine.rowset.RowSet;
import io.deephaven.engine.rowset.RowSetFactory;
import io.deephaven.engine.rowset.RowSetShiftData;
import io.deephaven.engine.table.ModifiedColumnSet;
import io.deephaven.engine.table.MultiJoinFactory;
import io.deephaven.engine.table.MultiJoinInput;
import io.deephaven.engine.table.Table;
import io.deephaven.engine.table.*;
import io.deephaven.engine.testutil.*;
import io.deephaven.engine.testutil.generator.*;
import io.deephaven.engine.util.PrintListener;
Expand Down Expand Up @@ -763,6 +760,35 @@ public void testColumnConflicts() {
}
}

@Test
public void testMultiJoinInputColumnParsing() {
final Table dummyTable = emptyTable(0);

MultiJoinInput mji = MultiJoinInput.of(dummyTable, "Key1=A,Key2=B", "C1=C,D1=D");
Assert.assertEquals(mji.inputTable(), dummyTable);
Assert.assertEquals(mji.columnsToMatch()[0].left().name(), "Key1");
Assert.assertEquals(mji.columnsToMatch()[0].right().name(), "A");
Assert.assertEquals(mji.columnsToMatch()[1].left().name(), "Key2");
Assert.assertEquals(mji.columnsToMatch()[1].right().name(), "B");
Assert.assertEquals(mji.columnsToAdd()[0].newColumn().name(), "C1");
Assert.assertEquals(mji.columnsToAdd()[0].existingColumn().name(), "C");
Assert.assertEquals(mji.columnsToAdd()[1].newColumn().name(), "D1");
Assert.assertEquals(mji.columnsToAdd()[1].existingColumn().name(), "D");

// Assert whitespace and '==' is handled properly.
mji = MultiJoinInput.of(dummyTable, "\tKey1 = A, \tKey2 ==B ", "C1 =C, D1=D");
Assert.assertEquals(mji.inputTable(), dummyTable);
Assert.assertEquals(mji.columnsToMatch()[0].left().name(), "Key1");
Assert.assertEquals(mji.columnsToMatch()[0].right().name(), "A");
Assert.assertEquals(mji.columnsToMatch()[1].left().name(), "Key2");
Assert.assertEquals(mji.columnsToMatch()[1].right().name(), "B");
Assert.assertEquals(mji.columnsToAdd()[0].newColumn().name(), "C1");
Assert.assertEquals(mji.columnsToAdd()[0].existingColumn().name(), "C");
Assert.assertEquals(mji.columnsToAdd()[1].newColumn().name(), "D1");
Assert.assertEquals(mji.columnsToAdd()[1].existingColumn().name(), "D");
}


private Table doIterativeMultiJoin(String[] keyColumns, List<? extends Table> inputTables) {
final List<Table> keyTables = inputTables.stream()
.map(t -> keyColumns.length == 0 ? t.dropColumns(t.getDefinition().getColumnNames()).view("Dummy=1")
Expand Down
Loading