Skip to content

Commit af27e88

Browse files
texodustexodus
authored
Merge pull request #3115 from perspective-dev/fix-null-equality-bug
Fix `null` equality bug
2 parents f1c3af6 + 82f21a0 commit af27e88

File tree

9 files changed

+82
-10
lines changed

9 files changed

+82
-10
lines changed

‎rust/perspective-js/test/arrow/untitled.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"ENTITY_TYPE":[null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null]}

‎rust/perspective-js/test/arrow/untitled2.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"ENTITY_TYPE":[null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null]}

‎rust/perspective-js/test/arrow/untitled3.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"ENTITY_TYPE":[null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null]}

‎rust/perspective-js/test/arrow/untitled4.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"ENTITY_TYPE":[null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,null,"XXX","XXX",null,"XXX","XXX","YYY","XXX","YYY","ZZZ","ZZZ","YYY","YYY","ZZZ","ZZZ",null,"YYY",null,null,"XXX","YYY",null,"ZZZ","XXX","ZZZ","WWW","WWW","YYY",null,"WWW","ZZZ","YYY","YYY","XXX","XXX","ZZZ","ZZZ","ZZZ","XXX",null,null,"YYY","XXX",null,"XXX","WWW","YYY",null,null,"ZZZ",null,"YYY",null,null,"ZZZ",null,"YYY","XXX","XXX","YYY","YYY",null,"XXX","YYY","YYY",null,"YYY","YYY",null,"XXX","YYY","XXX","ZZZ",null,"XXX",null,null,"YYY","XXX","XXX","XXX","XXX","WWW","XXX","ZZZ","XXX","XXX","ZZZ","ZZZ",null,"XXX","YYY",null,"YYY","ZZZ","YYY",null,null,null,"YYY","ZZZ","XXX",null,"WWW","WWW"]}

‎rust/perspective-js/test/arrow/untitled5.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
{"ENTITY_TYPE":["","","","AAAA","AAAA","DDDD","CCCC","AAAA","","BBBB","AAAA","CCCC","","AAAA","CCCC","CCCC","CCCC","CCCC","AAAA","","DDDD","DDDD","AAAA","BBBB","AAAA","CCCC","AAAA","DDDD","CCCC","AAAA","CCCC","CCCC","CCCC","AAAA","","CCCC","CCCC","CCCC","CCCC","DDDD","","AAAA","AAAA","","DDDD","DDDD","AAAA","DDDD","CCCC","CCCC","CCCC","CCCC","AAAA","CCCC","DDDD","DDDD","CCCC","AAAA","CCCC","AAAA","CCCC","AAAA","DDDD","DDDD","DDDD","AAAA","CCCC","DDDD","DDDD","AAAA","","DDDD","BBBB","","AAAA","AAAA","CCCC","CCCC","DDDD","AAAA","CCCC","BBBB","","CCCC","DDDD","CCCC","","AAAA","CCCC","AAAA","DDDD","AAAA","CCCC","CCCC","DDDD","CCCC","AAAA","","DDDD","CCCC","DDDD","AAAA","AAAA","CCCC","DDDD","","AAAA","","AAAA","DDDD","AAAA","DDDD","","DDDD","","CCCC","AAAA","","BBBB","CCCC","AAAA","BBBB","BBBB","CCCC","AAAA","DDDD","BBBB","AAAA","DDDD","","","DDDD","CCCC","DDDD","CCCC","AAAA","AAAA","AAAA","CCCC","CCCC","","CCCC","CCCC","CCCC","","","","AAAA","CCCC","","","AAAA","","","CCCC","","CCCC","CCCC","","AAAA","CCCC","DDDD","AAAA","CCCC","DDDD","DDDD","CCCC","","","","","","","CCCC","","","CCCC","","","CCCC","CCCC","CCCC","DDDD","BBBB","","","AAAA","CCCC","AAAA","","","CCCC","","AAAA","CCCC","DDDD","AAAA","","","","DDDD","","CCCC","AAAA","","","","","CCCC","","","","","AAAA","","CCCC","","CCCC","CCCC","","CCCC","CCCC","DDDD","CCCC","DDDD","","","","CCCC","","","CCCC","","","","CCCC","CCCC","","","","","AAAA","","","","","","DDDD","","CCCC","AAAA","","","","","","AAAA","DDDD","CCCC","DDDD","CCCC","AAAA","","CCCC","","AAAA","CCCC","","CCCC","BBBB","AAAA","CCCC","CCCC","CCCC","DDDD","CCCC","CCCC","","CCCC","AAAA","","CCCC","AAAA","CCCC","AAAA","CCCC","BBBB","AAAA","","CCCC","CCCC","DDDD","DDDD","AAAA","DDDD","CCCC","DDDD","AAAA","CCCC","CCCC","AAAA","CCCC","","AAAA","AAAA","AAAA","CCCC","BBBB","","","AAAA","","CCCC","DDDD","","CCCC","CCCC","CCCC","BBBB","DDDD","CCCC","AAAA","","AAAA","CCCC","CCCC","DDDD","CCCC","","","BBBB","DDDD","CCCC","DDDD","DDDD","AAAA","BBBB","CCCC","DDDD","AAAA","AAAA","CCCC","BBBB","CCCC","","AAAA","DDDD","AAAA","CCCC","DDDD","","CCCC","AAAA","DDDD","","","CCCC","","","","","","","BBBB","","BBBB","","BBBB","CCCC","AAAA","CCCC","DDDD","","CCCC","","BBBB","","DDDD","AAAA","CCCC","CCCC","CCCC","AAAA","AAAA","BBBB","BBBB","CCCC","DDDD","BBBB","CCCC","AAAA","","DDDD","CCCC","AAAA","DDDD","AAAA","CCCC","","AAAA","CCCC","AAAA","AAAA","CCCC","CCCC","AAAA","AAAA","AAAA","CCCC","CCCC","","","","","DDDD","CCCC","DDDD","AAAA","CCCC","AAAA","CCCC","AAAA","AAAA","AAAA","AAAA","DDDD","CCCC","AAAA","CCCC","DDDD","CCCC","AAAA","DDDD","DDDD","CCCC","","AAAA","AAAA","AAAA","AAAA","AAAA","BBBB","CCCC","CCCC","BBBB","AAAA","AAAA","AAAA","BBBB","AAAA","CCCC","DDDD","CCCC","AAAA","AAAA","DDDD","AAAA","AAAA","DDDD","","AAAA","","","CCCC","","","","DDDD","AAAA","CCCC","AAAA","CCCC","CCCC","CCCC","BBBB","AAAA","","DDDD","AAAA","CCCC","AAAA","CCCC","DDDD","DDDD","","","CCCC","AAAA","","","DDDD","","AAAA","AAAA","","DDDD","AAAA","DDDD","AAAA","DDDD","DDDD","","AAAA","BBBB","CCCC","","","AAAA","AAAA","AAAA","","DDDD","AAAA","AAAA","DDDD","AAAA","AAAA","DDDD","CCCC","DDDD","","CCCC","","CCCC","CCCC","AAAA","DDDD","AAAA","BBBB","CCCC","CCCC","AAAA","DDDD","DDDD","CCCC","DDDD","AAAA","AAAA","CCCC","CCCC","BBBB","AAAA","DDDD","DDDD","AAAA","BBBB","AAAA","CCCC","BBBB","","","","","","","","","","CCCC","","","DDDD","DDDD","AAAA","","DDDD","AAAA","CCCC","CCCC","DDDD","CCCC","DDDD","DDDD","AAAA","","","","CCCC","AAAA","CCCC","AAAA","CCCC","CCCC","DDDD","CCCC","AAAA","AAAA","DDDD","DDDD","CCCC","DDDD","AAAA","DDDD","CCCC","AAAA","AAAA","CCCC","AAAA","DDDD","AAAA","CCCC","CCCC","DDDD","DDDD","DDDD","AAAA","DDDD","CCCC","CCCC","CCCC","CCCC","DDDD","","DDDD","CCCC","","DDDD","","CCCC","CCCC","DDDD","","AAAA","AAAA","CCCC","CCCC","CCCC","CCCC","CCCC","AAAA","CCCC","CCCC","CCCC","DDDD","AAAA","DDDD","DDDD","DDDD","CCCC","CCCC","DDDD","CCCC","CCCC","CCCC","DDDD","CCCC","DDDD","DDDD","DDDD","CCCC","DDDD","DDDD","CCCC","DDDD","CCCC","DDDD","DDDD","DDDD","DDDD","","DDDD","CCCC","AAAA","DDDD","CCCC","DDDD","CCCC","CCCC","BBBB","CCCC","AAAA","CCCC","AAAA","","","DDDD","DDDD","AAAA","CCCC","AAAA","CCCC","AAAA","CCCC","CCCC","CCCC","AAAA","AAAA","AAAA","AAAA","","CCCC","CCCC","CCCC","CCCC","CCCC","CCCC","DDDD","AAAA","AAAA","AAAA","AAAA","","CCCC","AAAA","CCCC","AAAA","CCCC","BBBB","AAAA","DDDD","AAAA","BBBB","CCCC","DDDD","AAAA","AAAA","","AAAA","CCCC","AAAA","CCCC","CCCC","AAAA","AAAA","CCCC","AAAA","DDDD","AAAA","AAAA","AAAA","AAAA","AAAA","DDDD","AAAA","BBBB","AAAA","DDDD","AAAA","CCCC","DDDD","BBBB","AAAA","CCCC","AAAA","DDDD","AAAA","BBBB","AAAA","CCCC","AAAA","BBBB","AAAA","DDDD","","CCCC","","CCCC","AAAA","DDDD","DDDD","","DDDD","CCCC","CCCC","CCCC","CCCC","AAAA","DDDD","CCCC","","CCCC","CCCC","AAAA","DDDD","CCCC","BBBB","AAAA","DDDD","AAAA","CCCC","AAAA","AAAA","CCCC","DDDD","BBBB","BBBB","CCCC","DDDD","DDDD","CCCC","AAAA","AAAA","BBBB","DDDD","CCCC","AAAA","CCCC","AAAA","AAAA","CCCC","AAAA","AAAA","AAAA","AAAA","CCCC","CCCC","CCCC","AAAA","CCCC","AAAA","DDDD","CCCC","AAAA","DDDD","DDDD","DDDD","CCCC","DDDD","DDDD","AAAA","DDDD","DDDD","DDDD","DDDD","CCCC","DDDD","AAAA","AAAA","CCCC","AAAA","CCCC","AAAA","AAAA","BBBB","AAAA","AAAA","AAAA","AAAA","AAAA","CCCC","DDDD","CCCC","AAAA","AAAA","AAAA","CCCC","BBBB","AAAA",null,"CCCC","CCCC","CCCC","CCCC","CCCC","CCCC","CCCC","DDDD","BBBB","DDDD","CCCC","CCCC","CCCC","CCCC","DDDD","CCCC","AAAA","AAAA","AAAA","CCCC",null,"AAAA","CCCC","AAAA","CCCC","CCCC","CCCC","DDDD","DDDD","DDDD","CCCC","AAAA","CCCC","CCCC","CCCC","AAAA","DDDD","CCCC","AAAA","CCCC","DDDD","CCCC","AAAA","CCCC","AAAA",null,"AAAA","CCCC","DDDD","DDDD","AAAA","CCCC","DDDD","AAAA","CCCC",null,"CCCC","DDDD",null,null,"CCCC",null,null,null,null,null,null,null,null,"CCCC",null,null,"CCCC",null,"DDDD","CCCC",null,"AAAA",null,"AAAA",null,null,null,null,null,null,null,null,null,null,"DDDD","BBBB","DDDD","AAAA","DDDD","DDDD","AAAA","CCCC","BBBB","CCCC","AAAA","CCCC","AAAA","AAAA","BBBB","AAAA","BBBB",null,null,null,null,"CCCC","AAAA","CCCC","CCCC","CCCC","AAAA","DDDD","CCCC",null,null,"CCCC",null,"AAAA","AAAA",null,null,null,null,null,"AAAA",null,null,"CCCC",null,"CCCC","CCCC","BBBB","CCCC","CCCC","CCCC",null,null,null,"AAAA",null,null,null,null,null,null,null,null,null,null,"AAAA",null,null,null,null,"DDDD",null,null,null,null,"AAAA","CCCC",null,null,null,null,null,null,"DDDD",null,null,null,null,null,null,null,"DDDD","AAAA",null,null,null,"AAAA",null,"AAAA","AAAA","AAAA",null,"AAAA","AAAA","CCCC","AAAA","CCCC","CCCC",null,"CCCC","BBBB","AAAA","AAAA","DDDD","BBBB","AAAA","CCCC","AAAA","AAAA","BBBB","AAAA","CCCC","DDDD","AAAA","AAAA","CCCC","DDDD","DDDD","AAAA","CCCC","CCCC","AAAA","CCCC","AAAA","BBBB","AAAA","AAAA","AAAA","AAAA","AAAA","AAAA","CCCC","CCCC","CCCC","CCCC","AAAA","AAAA","CCCC","CCCC","CCCC","DDDD","DDDD","DDDD","CCCC","CCCC","AAAA",null,"CCCC",null,"CCCC","DDDD","DDDD","DDDD","AAAA","DDDD","AAAA","CCCC","AAAA","DDDD","DDDD","CCCC","DDDD","DDDD","AAAA","DDDD","DDDD","DDDD","AAAA","CCCC","CCCC","CCCC","AAAA","AAAA","AAAA","AAAA","CCCC","BBBB","AAAA","AAAA","CCCC","AAAA","AAAA","DDDD","AAAA",null,"BBBB","CCCC","AAAA","AAAA","CCCC",null,"AAAA","AAAA","DDDD","CCCC","DDDD","DDDD","AAAA","CCCC","DDDD","AAAA","DDDD","AAAA","AAAA","AAAA","CCCC","CCCC","AAAA","DDDD","CCCC","DDDD",null,"AAAA","BBBB","DDDD","CCCC","DDDD","AAAA",null,"CCCC","DDDD","AAAA",null,"BBBB","AAAA",null,"BBBB","CCCC","DDDD","BBBB",null,null,"AAAA","CCCC","DDDD","CCCC","CCCC","DDDD",null,"CCCC","CCCC","DDDD"]}

‎rust/perspective-js/test/js/constructors/arrow.spec.ts‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@
1313
import * as arrow from "apache-arrow";
1414
import { test, expect } from "@perspective-dev/test";
1515
import perspective from "../perspective_client";
16+
import * as fs from "node:fs";
17+
18+
import * as url from "node:url";
19+
20+
const __dirname = url.fileURLToPath(new URL(".", import.meta.url)).slice(0, -1);
1621

1722
test.describe("Arrow", function () {
1823
test.describe("Date columns", function () {
@@ -68,4 +73,52 @@ test.describe("Arrow", function () {
6873
process.env.TZ = `UTC`;
6974
});
7075
});
76+
77+
test.describe("regressions", () => {
78+
test("null equality works correctly in updates", async function () {
79+
async function write_to_json(
80+
buffer: ArrayBuffer,
81+
filename: string,
82+
) {
83+
const table = await perspective.table(buffer);
84+
const view = await table.view({
85+
columns: ["ENTITY_TYPE"],
86+
});
87+
88+
const json = await view.to_columns_string();
89+
fs.writeFileSync(filename, json);
90+
await view.delete();
91+
await table.delete();
92+
}
93+
94+
const file = JSON.parse(
95+
fs.readFileSync(
96+
`${__dirname}/../../arrow/untitled.json`,
97+
"utf8",
98+
),
99+
);
100+
101+
const table = await perspective.table(file, {
102+
name: "arrow_null_test",
103+
});
104+
105+
const view = await table.view({ group_by: ["ENTITY_TYPE"] });
106+
for (let i = 2; i < 6; i++) {
107+
const file = JSON.parse(
108+
fs.readFileSync(
109+
`${__dirname}/../../arrow/untitled${i}.json`,
110+
"utf8",
111+
),
112+
);
113+
114+
await table.update(file);
115+
}
116+
117+
const cols = await view.to_columns({ end_row: 4 });
118+
expect(cols).toStrictEqual({
119+
ENTITY_TYPE: [2158, 985, 168, 311],
120+
__ROW_PATH__: [[], [null], [""], ["AAAA"]],
121+
});
122+
});
123+
});
71124
});

‎rust/perspective-js/test/js/expressions/string.spec.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -756,7 +756,7 @@ const random_string = (
756756
expect(result['"a" == "b"']).toEqual([
757757
false,
758758
true,
759-
false,
759+
true,
760760
true,
761761
true,
762762
]);
@@ -790,7 +790,7 @@ const random_string = (
790790
expect(result['"a" == "b"']).toEqual([
791791
false,
792792
false,
793-
false,
793+
true,
794794
false,
795795
false,
796796
]);

‎rust/perspective-server/cpp/perspective/src/cpp/scalar.cpp‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,11 @@ t_tscalar::operator==(const t_tscalar& rhs) const {
8585
return false;
8686
}
8787

88+
if ((m_status == STATUS_INVALID || m_status == STATUS_CLEAR)
89+
&& (rhs.m_status == STATUS_INVALID || rhs.m_status == STATUS_CLEAR)) {
90+
return true;
91+
}
92+
8893
if (m_type == DTYPE_BOOL) {
8994
return get<bool>() == rhs.get<bool>();
9095
}
@@ -1447,16 +1452,20 @@ repr(const t_tscalar& s) {
14471452
size_t
14481453
hash_value(const t_tscalar& s) {
14491454
std::size_t seed = 0;
1450-
if (s.m_type == DTYPE_STR) {
1451-
const char* c = s.get_char_ptr();
1452-
boost::hash_combine(seed, boost::hash_range(c, c + std::strlen(c)));
1453-
1455+
if (s.m_status == STATUS_VALID) {
1456+
boost::hash_combine(seed, s.m_status);
1457+
boost::hash_combine(seed, s.m_type);
1458+
if (s.m_type == DTYPE_STR) {
1459+
const char* c = s.get_char_ptr();
1460+
boost::hash_combine(seed, boost::hash_range(c, c + std::strlen(c)));
1461+
} else {
1462+
boost::hash_combine(seed, s.m_data.m_uint64);
1463+
}
14541464
} else {
1455-
boost::hash_combine(seed, s.m_data.m_uint64);
1465+
boost::hash_combine(seed, STATUS_INVALID);
1466+
boost::hash_combine(seed, s.m_type);
14561467
}
14571468

1458-
boost::hash_combine(seed, s.m_type);
1459-
boost::hash_combine(seed, s.m_status);
14601469
return seed;
14611470
}
14621471

‎rust/perspective-server/cpp/perspective/src/include/perspective/scalar.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,12 @@ t_tscalar::compare_common(const t_tscalar& rhs) const {
328328
return cmp(m_type, rhs.m_type);
329329
}
330330

331-
if (m_status != rhs.m_status) {
331+
if (m_status != STATUS_VALID && rhs.m_status != STATUS_VALID) {
332+
COMPARER_T<unsigned char> cmp;
333+
return cmp(1, 1);
334+
}
335+
336+
if (m_status != STATUS_VALID || rhs.m_status != STATUS_VALID) {
332337
COMPARER_T<unsigned char> cmp;
333338
return cmp(m_status, rhs.m_status);
334339
}

0 commit comments

Comments
 (0)