Skip to content

Commit

Permalink
JS API should return nulls rather than undefined when reading table d…
Browse files Browse the repository at this point in the history 
…ata (#5437)

While Java arrays are initialized to nulls, GWT normalizes undefined
and null to be equivalent, so we need to explicitly assign null values
when building arrays.

Adds tests to ensure this doesn't break as part of #188.

Fixes #5400
  • Loading branch information
@niloc132 @stanbrub
niloc132 authored and stanbrub committed May 3, 2024
1 parent c544222 commit ed7d643
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 5 deletions.
7 changes: 7 additions & 0 deletions web/client-api/src/main/java/io/deephaven/web/client/api/barrage/WebBarrageUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -404,6 +404,8 @@ private static ColumnData readArrowBuffer(ByteBuffer data, Iter<FieldNode> nodes
for (int i = 0; i < size; ++i) {
if (!hasNulls || valid.get(i)) {
boolArray[i] = wireValues.get(i);
} else {
boolArray[i] = null;
}
}
return new BooleanArrayColumnData(boolArray);
Expand Down Expand Up @@ -486,6 +488,7 @@ private static ColumnData readArrowBuffer(ByteBuffer data, Iter<FieldNode> nodes

for (int i = 0; i < size; i++) {
if (hasNulls && !valid.get(i)) {
strArrArr[i] = null;
continue;
}
int arrayStart = offsets.get(i);
Expand All @@ -497,6 +500,7 @@ private static ColumnData readArrowBuffer(ByteBuffer data, Iter<FieldNode> nodes
if (innerHasNulls && !innerValid.get(inner)) {
assert innerOffsets.get(inner) == innerOffsets.get(inner + 1)
: innerOffsets.get(inner) + " == " + innerOffsets.get(inner + 1);
strArr[j] = null;
continue;
}
// might be cheaper to do views on the underlying bb (which will be copied anyway
Expand Down Expand Up @@ -525,6 +529,7 @@ private static ColumnData readArrowBuffer(ByteBuffer data, Iter<FieldNode> nodes
byte[] buf = new byte[32];
for (int i = 0; i < size; i++) {
if (hasNulls && !valid.get(i)) {
stringArray[i] = null;
continue;
}
int ioff = offsets.get(i);
Expand All @@ -544,6 +549,7 @@ private static ColumnData readArrowBuffer(ByteBuffer data, Iter<FieldNode> nodes
byte[] buf = null;
for (int i = 0; i < size; i++) {
if (hasNulls && !valid.get(i)) {
bigDecArray[i] = null;
continue;
}
int ioff = offsets.get(i);
Expand All @@ -563,6 +569,7 @@ private static ColumnData readArrowBuffer(ByteBuffer data, Iter<FieldNode> nodes
byte[] buf = null;
for (int i = 0; i < size; i++) {
if (hasNulls && !valid.get(i)) {
bigIntArray[i] = null;
continue;
}
int ioff = offsets.get(i);
Expand Down
86 changes: 81 additions & 5 deletions web/client-api/src/test/java/io/deephaven/web/client/api/NullValueTestGwt.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import elemental2.core.JsArray;
import elemental2.promise.Promise;
import io.deephaven.web.client.api.subscription.ViewportRow;
import jsinterop.base.Js;

import java.math.BigDecimal;
import java.math.BigInteger;
Expand All @@ -16,7 +17,7 @@
public class NullValueTestGwt extends AbstractAsyncGwtTestCase {
private final TableSourceBuilder tables = new TableSourceBuilder()
.script("from deephaven import empty_table")
.script("nulltable", "empty_table(2).update([\n" +
.script("some_null_table", "empty_table(2).update([\n" +
" \"MyInt=i==0?null:i\",\n" +
" \"MyLong=i==0?null:(long)i\",\n" +
" \"MyDouble=i==0?null:(double)i\",\n" +
Expand All @@ -28,12 +29,28 @@ public class NullValueTestGwt extends AbstractAsyncGwtTestCase {
" \"MyString=i==0?null:``+i\",\n" +
" \"MyDate=i==0?null:epochNanosToInstant(i)\",\n" +
" \"MyBigInteger=i==0?null:java.math.BigInteger.valueOf(i)\",\n" +
" \"MyBigDecimal=i==0?null:java.math.BigDecimal.valueOf(i, 4)\"\n" +
" \"MyBigDecimal=i==0?null:java.math.BigDecimal.valueOf(i, 4)\",\n" +
" \"MyStringArray1=i==0?null:new String[] {`A`, `B`, `C`}\"\n" +
"])")
.script("all_null_table", "empty_table(20).update([\n" +
" \"MyInt=(Integer)null\",\n" +
" \"MyLong=(Long)null\",\n" +
" \"MyDouble=(Double)null\",\n" +
" \"MyShort=(Short)null\",\n" +
" \"MyFloat=(Float)null\",\n" +
" \"MyChar=(Character)null\",\n" +
" \"MyByte=(Byte)null\",\n" +
" \"MyBoolean=(Boolean)null\",\n" +
" \"MyString=(String)null\",\n" +
" \"MyDate=(java.time.Instant) null\",\n" +
" \"MyBigInteger=(java.math.BigInteger)null\",\n" +
" \"MyBigDecimal=(java.math.BigDecimal)null\",\n" +
" \"MyStringArray1=(String[])null\"\n" +
"])");

public void testNullTable() {
public void testTableWithSomeNulls() {
connect(tables)
.then(table("nulltable"))
.then(table("some_null_table"))
.then(table -> {
delayTestFinish(5000);

Expand Down Expand Up @@ -61,6 +78,8 @@ public void testNullTable() {
assertEquals("java.time.Instant", table.findColumn("MyDate").getType());
assertEquals("java.math.BigInteger", table.findColumn("MyBigInteger").getType());
assertEquals("java.math.BigDecimal", table.findColumn("MyBigDecimal").getType());
// TODO restore after #188
// assertEquals("java.lang.String[]", table.findColumn("MyStringArray1").getType());

return Promise.resolve(table);
})
Expand All @@ -72,7 +91,8 @@ public void testNullTable() {

JsArray<Column> columns = table.getColumns();
for (int i = 0; i < columns.length; i++) {
assertEquals(null, nullRow.get(columns.getAt(i)));
assertFalse(Js.isTripleEqual(Js.undefined(), nullRow.get(columns.getAt(i))));
assertNull(nullRow.get(columns.getAt(i)));
}

ViewportRow valueRow = rows.getAt(1);
Expand All @@ -91,6 +111,62 @@ public void testNullTable() {
valueRow.get(table.findColumn("MyBigInteger")).<BigIntegerWrapper>cast().getWrapped());
assertEquals(BigDecimal.valueOf(1, 4),
valueRow.get(table.findColumn("MyBigDecimal")).<BigDecimalWrapper>cast().getWrapped());
// TODO restore after #188
// assertEquals(JsArray.of("A", "B", "C"),
// valueRow.get(table.findColumn("MyStringArray1")).<JsArray<String>>cast());
}, 1000);
})
.then(this::finish).catch_(this::report);
}

public void testTableWithAllNulls() {
connect(tables)
.then(table("all_null_table"))
.then(table -> {
delayTestFinish(5000);

assertFalse(table.isRefreshing());
assertFalse(table.isClosed());
assertFalse(table.isBlinkTable());
assertFalse(table.hasInputTable());
assertFalse((table.isUncoalesced()));

assertEquals(20., table.getSize(), 0);
assertEquals(20., table.getTotalSize(), 0);

return Promise.resolve(table);
})
.then(table -> {
assertEquals("int", table.findColumn("MyInt").getType());
assertEquals("long", table.findColumn("MyLong").getType());
assertEquals("double", table.findColumn("MyDouble").getType());
assertEquals("short", table.findColumn("MyShort").getType());
assertEquals("float", table.findColumn("MyFloat").getType());
assertEquals("char", table.findColumn("MyChar").getType());
assertEquals("byte", table.findColumn("MyByte").getType());
assertEquals("java.lang.Boolean", table.findColumn("MyBoolean").getType());
assertEquals("java.lang.String", table.findColumn("MyString").getType());
assertEquals("java.time.Instant", table.findColumn("MyDate").getType());
assertEquals("java.math.BigInteger", table.findColumn("MyBigInteger").getType());
assertEquals("java.math.BigDecimal", table.findColumn("MyBigDecimal").getType());
// TODO restore after #188
// assertEquals("java.lang.String[]", table.findColumn("MyStringArray1").getType());

return Promise.resolve(table);
})
.then(table -> {
table.setViewport(0, 1, null);
return assertUpdateReceived(table, viewport -> {
JsArray<ViewportRow> rows = viewport.getRows();

JsArray<Column> columns = table.getColumns();
for (int i = 0; i < columns.length; i++) {
for (int j = 0; j < rows.length; j++) {
ViewportRow row = rows.getAt(j);
assertFalse(Js.isTripleEqual(Js.undefined(), row.get(columns.getAt(i))));
assertNull(columns.getAt(i).getName(), row.get(columns.getAt(i)));
}
}
}, 1000);
})
.then(this::finish).catch_(this::report);
Expand Down

0 comments on commit ed7d643

Please sign in to comment.