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

Fix incorrect assertion in Java concat [skip ci] #8258

Merged
merged 2 commits into from
May 19, 2021
Merged

Fix incorrect assertion in Java concat [skip ci] #8258

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
af497e8
fix incorrect assertion in concat
sperlingxx May 17, 2021
e6110dc
fix comments
sperlingxx May 17, 2021
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 java/src/main/java/ai/rapids/cudf/ColumnVector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,8 @@ public static ColumnVector stringConcatenate(ColumnView[] columns) {
* @return A new java column vector containing the concatenated strings.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to update the comment here to match the behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed! Thanks for catching that.

*/
public static ColumnVector stringConcatenate(Scalar separator, Scalar narep, ColumnView[] columns) {
assert columns.length >= 2 : ".stringConcatenate() operation requires at least 2 columns";
assert columns != null : "input columns should not be null";
assert columns.length > 0 : "input columns should not be empty";
assert separator != null : "separator scalar provided may not be null";
assert separator.getType().equals(DType.STRING) : "separator scalar must be a string scalar";
assert narep != null : "narep scalar provided may not be null";
Expand Down Expand Up @@ -548,7 +549,7 @@ public static ColumnVector listConcatenateByRow(ColumnView... columns) {
*/
public static ColumnVector listConcatenateByRow(boolean ignoreNull, ColumnView... columns) {
assert columns != null : "input columns should not be null";
assert columns.length >= 2 : "listConcatenateByRow requires at least 2 columns";
assert columns.length > 0 : "input columns should not be empty";

long[] columnViews = new long[columns.length];
for(int i = 0; i < columns.length; i++) {
Expand Down
36 changes: 26 additions & 10 deletions java/src/test/java/ai/rapids/cudf/ColumnVectorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2037,9 +2037,8 @@ void testStringConcat() {
}
});
assertThrows(AssertionError.class, () -> {
try (ColumnVector sv = ColumnVector.fromStrings("a", "B", "cd");
Scalar emptyString = Scalar.fromString("");
ColumnVector concat = ColumnVector.stringConcatenate(emptyString, emptyString, new ColumnView[]{sv})) {
try (Scalar emptyString = Scalar.fromString("");
ColumnVector concat = ColumnVector.stringConcatenate(emptyString, emptyString, new ColumnView[]{})) {
}
});
assertThrows(CudfException.class, () -> {
Expand Down Expand Up @@ -2085,6 +2084,16 @@ void testStringConcatWithNulls() {
ColumnVector concat = ColumnVector.stringConcatenate(emptyString, nullSubstitute, new ColumnView[]{v, v})) {
assertColumnsAreEqual(concat, e_concat);
}

try (ColumnVector v = ColumnVector.fromStrings("a", "B", "cd", "\u0480\u0481", "E\tf",
"g\nH", "IJ\"\u0100\u0101\u0500\u0501",
"kl m", "Nop1", "\\qRs2", null,
"3tuV\'", "wX4Yz", "\ud720\ud721");
Scalar emptyString = Scalar.fromString("");
wbo4958 marked this conversation as resolved.
Show resolved Hide resolved
Scalar nullSubstitute = Scalar.fromString("NULL");
ColumnVector concat = ColumnVector.stringConcatenate(emptyString, nullSubstitute, new ColumnView[]{v})) {
assertColumnsAreEqual(v, concat);
}
}

@Test
Expand All @@ -2102,6 +2111,13 @@ void testStringConcatSeparators() {

@Test
void testListConcatByRow() {
try (ColumnVector cv = ColumnVector.fromLists(new HostColumnVector.ListType(true,
new HostColumnVector.BasicType(true, DType.INT32)),
Arrays.asList(0), Arrays.asList(1, 2, 3), null, Arrays.asList(), Arrays.asList());
ColumnVector result = ColumnVector.listConcatenateByRow(cv)) {
assertColumnsAreEqual(cv, result);
}

try (ColumnVector cv1 = ColumnVector.fromLists(new HostColumnVector.ListType(true,
new HostColumnVector.BasicType(true, DType.INT32)),
Arrays.asList(0), Arrays.asList(1, 2, 3), null, Arrays.asList(), Arrays.asList());
Expand Down Expand Up @@ -2148,13 +2164,6 @@ void testListConcatByRow() {
assertColumnsAreEqual(expect, result);
}

assertThrows(AssertionError.class, () -> {
try (ColumnVector cv = ColumnVector.fromLists(new HostColumnVector.ListType(true,
new HostColumnVector.BasicType(true, DType.INT32)), Arrays.asList(1, 2, 3));
ColumnVector result = ColumnVector.listConcatenateByRow(cv)) {
}
});

assertThrows(CudfException.class, () -> {
try (ColumnVector cv = ColumnVector.fromInts(1, 2, 3);
ColumnVector result = ColumnVector.listConcatenateByRow(cv, cv)) {
Expand Down Expand Up @@ -2190,6 +2199,13 @@ void testListConcatByRow() {

@Test
void testListConcatByRowIgnoreNull() {
try (ColumnVector cv = ColumnVector.fromLists(new HostColumnVector.ListType(true,
new HostColumnVector.BasicType(true, DType.INT32)),
Arrays.asList(0), Arrays.asList(1, 2, 3), null, Arrays.asList(), Arrays.asList());
ColumnVector result = ColumnVector.listConcatenateByRow(true, cv)) {
assertColumnsAreEqual(cv, result);
}

try (ColumnVector cv1 = ColumnVector.fromLists(new HostColumnVector.ListType(true,
new HostColumnVector.BasicType(true, DType.INT32)),
Arrays.asList((Integer) null), Arrays.asList(1, 2, 3), null, Arrays.asList(), null);
Expand Down