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.

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

Assert for non-empty nulls #13071

Merged
merged 20 commits into from
Apr 24, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
2 changes: 2 additions & 0 deletions java/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@
<maven.compiler.target>1.8</maven.compiler.target>
<junit.version>5.4.2</junit.version>
<ai.rapids.refcount.debug>false</ai.rapids.refcount.debug>
<ai.rapids.allow.nonempty.nulls>true</ai.rapids.allow.nonempty.nulls>
ttnghia marked this conversation as resolved.
Show resolved Hide resolved
<ai.rapids.cudf.nvtx.enabled>false</ai.rapids.cudf.nvtx.enabled>
<native.build.path>${basedir}/target/cmake-build</native.build.path>
<skipNativeCopy>false</skipNativeCopy>
Expand Down Expand Up @@ -550,6 +551,7 @@
<redirectTestOutputToFile>true</redirectTestOutputToFile>
<systemPropertyVariables>
<ai.rapids.refcount.debug>${ai.rapids.refcount.debug}</ai.rapids.refcount.debug>
<ai.rapids.allow.nonempty.nulls>${ai.rapids.allow.nonempty.nulls}</ai.rapids.allow.nonempty.nulls>
<ai.rapids.cudf.nvtx.enabled>${ai.rapids.cudf.nvtx.enabled}</ai.rapids.cudf.nvtx.enabled>
</systemPropertyVariables>
</configuration>
Expand Down
18 changes: 18 additions & 0 deletions java/src/main/java/ai/rapids/cudf/AssertEmptyNulls.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package ai.rapids.cudf;

public class AssertEmptyNulls {
/*
We have to read the system property to see if we should by-pass this assert. Ideally we should
just use `-da:AssertNonEmptyNulls` as per Java standard, but the reason for doing this is because
razajafri marked this conversation as resolved.
Show resolved Hide resolved
surefire-plugin doesn't expose anyway to disable assertions at the class level or package level
instead we can only disable asserts at the ClassLoader level. Therefore, in order for the tests to
pass we need to set a flag from the pom to know we are running unit-tests to allow non-empty nulls
*/
private static final boolean ALLOW_NON_EMPTY_NULLS =
Boolean.getBoolean("ai.rapids.allow.nonempty.nulls");
public static void assertEmptyNulls(ColumnView cv) {
razajafri marked this conversation as resolved.
Show resolved Hide resolved
if (cv.type.isNestedType() && !ALLOW_NON_EMPTY_NULLS) {
assert !cv.hasNonEmptyNulls() : "Column has non-empty nulls";
ttnghia marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
2 changes: 2 additions & 0 deletions java/src/main/java/ai/rapids/cudf/ColumnView.java
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public class ColumnView implements AutoCloseable, BinaryOperable {
this.rows = ColumnView.getNativeRowCount(viewHandle);
this.nullCount = ColumnView.getNativeNullCount(viewHandle);
this.offHeap = null;
AssertEmptyNulls.assertEmptyNulls(this);
}


Expand All @@ -67,6 +68,7 @@ protected ColumnView(ColumnVector.OffHeapState state) {
type = DType.fromNative(ColumnView.getNativeTypeId(viewHandle), ColumnView.getNativeTypeScale(viewHandle));
rows = ColumnView.getNativeRowCount(viewHandle);
nullCount = ColumnView.getNativeNullCount(viewHandle);
AssertEmptyNulls.assertEmptyNulls(this);
}

/**
Expand Down
6 changes: 6 additions & 0 deletions java/src/test/java/ai/rapids/cudf/ColumnVectorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@
import static org.junit.jupiter.api.Assumptions.assumeTrue;

public class ColumnVectorTest extends CudfTestBase {
private static final boolean ALLOW_NON_EMPTY_NULLS =
Boolean.getBoolean("ai.rapids.allow.nonempty.nulls");

public static final double PERCENTAGE = 0.0001;

Expand Down Expand Up @@ -742,6 +744,7 @@ void testSpark32BitMurmur3HashListsAndNestedLists() {

@Test
void testAndNullReconfigureNulls() {
assumeTrue(ALLOW_NON_EMPTY_NULLS);
try (ColumnVector v0 = ColumnVector.fromBoxedInts(0, 100, null, null, Integer.MIN_VALUE, null);
ColumnVector v1 = ColumnVector.fromBoxedInts(0, 100, 1, 2, Integer.MIN_VALUE, null);
ColumnVector intResult = v1.mergeAndSetValidity(BinaryOp.BITWISE_AND, v0);
Expand All @@ -757,6 +760,7 @@ void testAndNullReconfigureNulls() {

@Test
void testOrNullReconfigureNulls() {
assumeTrue(ALLOW_NON_EMPTY_NULLS);
try (ColumnVector v0 = ColumnVector.fromBoxedInts(0, 100, null, null, Integer.MIN_VALUE, null);
ColumnVector v1 = ColumnVector.fromBoxedInts(0, 100, 1, 2, Integer.MIN_VALUE, null);
ColumnVector v2 = ColumnVector.fromBoxedInts(0, 100, 1, 2, Integer.MIN_VALUE, Integer.MAX_VALUE);
Expand Down Expand Up @@ -6746,6 +6750,7 @@ private ColumnView[] getColumnViewWithNonEmptyNulls() {

@Test
void testPurgeNonEmptyNullsList() {
assumeTrue(ALLOW_NON_EMPTY_NULLS);
ColumnView[] values = getColumnViewWithNonEmptyNulls();
try (ColumnView colWithNonEmptyNulls = values[1];
ColumnView input = values[0];
Expand All @@ -6761,6 +6766,7 @@ void testPurgeNonEmptyNullsList() {

@Test
void testPurgeNonEmptyNullsStruct() {
assumeTrue(ALLOW_NON_EMPTY_NULLS);
ColumnView[] values = getColumnViewWithNonEmptyNulls();
try (ColumnView listCol = values[1];
ColumnView input = values[0];
Expand Down