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 #1505, #998, #1225 - results after remove are correct #1528

Merged
merged 6 commits into from
Sep 2, 2021
Merged

Fix #1505, #998, #1225 - results after remove are correct #1528

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
065f495
Tests to reproduce #1505
sc1f Aug 23, 2021
eecc95b
fixes #1505 and #998` by refactoring t_gstate::get_pkeyed_table to no…
sc1f Aug 24, 2021
307b040
call_process in num_rows, add remove tests
sc1f Aug 25, 2021
43857c8
Fix #1225: table.size() correct with remove
sc1f Aug 25, 2021
28216fc
Add new removes test suite to assert correctness
sc1f Sep 2, 2021
2117e4d
eslint
sc1f Sep 2, 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
Prev Previous commit
Next Next commit
Add new removes test suite to assert correctness
  • Loading branch information
@sc1f
sc1f committed Sep 2, 2021
commit 28216fceed4edde94291aa5afbbd26176164b0e6
68 changes: 0 additions & 68 deletions cpp/perspective/src/cpp/gnode_state.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -611,81 +611,13 @@ t_gstate::get_pkeyed_table() const {
#endif
{
const std::string& colname = schema_columns[colidx];
// if (colname != "psp_op" && colname != "psp_pkey") {
rval->set_column(colname, master_table->get_const_column(colname)->clone(mask));
// }
}

#ifdef PSP_PARALLEL_FOR
);
#endif

// auto pkey_col = rval->get_column("psp_pkey").get();
// auto op_col = rval->get_column("psp_op").get();

// op_col->raw_fill<std::uint8_t>(OP_INSERT);
// op_col->valid_raw_fill();
// pkey_col->valid_raw_fill();

// std::vector<std::pair<t_tscalar, t_uindex>> order(table_size);
// std::vector<t_uindex> mapping;
// mapping.resize(mask.size());
// {
// t_uindex mapped = 0;
// for (t_uindex idx = 0; idx < mask.size(); ++idx) {
// mapping[idx] = mapped;
// if (mask.get(idx))
// ++mapped;
// }
// }

// t_uindex oidx = 0;
// for (const auto& kv : m_mapping) {
// if (mask.get(kv.second)) {
// order[oidx] = std::make_pair(kv.first, mapping[kv.second]);
// ++oidx;
// }
// }

// std::sort(order.begin(), order.end(),
// [](const std::pair<t_tscalar, t_uindex>& a, const std::pair<t_tscalar, t_uindex>& b) {
// return a.second < b.second;
// });

// if (get_pkey_dtype() == DTYPE_STR) {
// static const t_tscalar empty = get_interned_tscalar("");
// static bool const enable_pkeyed_table_vocab_reserve = true;

// t_uindex offset = has_pkey(empty) ? 0 : 1;

// size_t total_string_size = 0;

// if (enable_pkeyed_table_vocab_reserve) {
// total_string_size += offset;
// for (t_uindex idx = 0, loop_end = order.size(); idx < loop_end; ++idx) {
// total_string_size += strlen(order[idx].first.get_char_ptr()) + 1;
// }
// }

// // if the m_mapping is empty, get_pkey_dtype() may lie about our pkeys being strings
// // don't try to reserve in this case
// if (!order.size())
// total_string_size = 0;

// pkey_col->set_vocabulary(order, total_string_size);
// auto base = pkey_col->get_nth<t_uindex>(0);

// for (t_uindex idx = 0, loop_end = order.size(); idx < loop_end; ++idx) {
// base[idx] = idx + offset;
// }
// } else {
// t_uindex ridx = 0;
// for (const auto& e : order) {
// pkey_col->set_scalar(ridx, e.first);
// ++ridx;
// }
// }

return rval;
}

Expand Down
186 changes: 165 additions & 21 deletions packages/perspective/test/js/removes.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,48 @@ describe("Removes", () => {
await table.delete();
});

it("Output consistent with filter, remove before view construction, sorted", async () => {
const table = await perspective.table(SCHEMA, {
index: idx
});
const data = {
str: [1, 2, 3, 4, 5, 6].map(x => x.toString()),
int: [1, 2, 3, 4, 5, 6],
float: [1, 2, 3, 4, 5, 6].map(x => x * 0.5)
};

table.update(data);
table.remove([1, 3, 5].map(x => (string_pkey ? x.toString() : x)));

await query(
table,
{
sort: [["str", "desc"]]
},
async getter => {
expect(await getter()).toEqual({
str: ["6", "4", "2"],
int: [6, 4, 2],
float: [3, 2, 1]
});

table.update({
str: ["7", "9", "8"],
int: [7, 9, 8],
float: [3.5, 4.5, 4]
});

expect(await getter()).toEqual({
str: ["9", "8", "7", "6", "4", "2"],
int: [9, 8, 7, 6, 4, 2],
float: [4.5, 4, 3.5, 3, 2, 1]
});
}
);

await table.delete();
});

it("Output consistent with filter, remove and add identical", async () => {
const table = await perspective.table(SCHEMA, {
index: idx
Expand Down Expand Up @@ -212,7 +254,111 @@ describe("Removes", () => {
await table.delete();
});

it("Output consistent with filter, remove and add null", async () => {
it("Output consistent with filter, add null and remove other", async () => {
const table = await perspective.table(SCHEMA, {
index: idx
});
const data = {
str: [1, 2, 3, 4, 5, 6].map(x => x.toString()),
int: [1, 2, 3, 4, 5, 6],
float: [1, 2, 3, 4, 5, 6].map(x => x * 0.5)
};

table.update(data);

await query(table, {}, async getter => {
expect(await getter()).toEqual(data);

table.update({
str: [string_pkey ? null : "7"],
int: [string_pkey ? 7 : null],
float: [3.5]
});

if (string_pkey) {
expect(await getter()).toEqual({
str: [null, 1, 2, 3, 4, 5, 6].map(x => (x ? x.toString() : x)),
int: [7, 1, 2, 3, 4, 5, 6],
float: [7, 1, 2, 3, 4, 5, 6].map(x => x * 0.5)
});
} else {
expect(await getter()).toEqual({
str: [7, 1, 2, 3, 4, 5, 6].map(x => x.toString()),
int: [null, 1, 2, 3, 4, 5, 6],
float: [7, 1, 2, 3, 4, 5, 6].map(x => x * 0.5)
});
}

table.remove([string_pkey ? "3" : 3]);

if (string_pkey) {
expect(await getter()).toEqual({
str: [null, 1, 2, 4, 5, 6].map(x => (x ? x.toString() : x)),
int: [7, 1, 2, 4, 5, 6],
float: [7, 1, 2, 4, 5, 6].map(x => x * 0.5)
});
} else {
expect(await getter()).toEqual({
str: [7, 1, 2, 4, 5, 6].map(x => x.toString()),
int: [null, 1, 2, 4, 5, 6],
float: [7, 1, 2, 4, 5, 6].map(x => x * 0.5)
});
}
});

await table.delete();
});

it("Output consistent with filter, remove and add new dataset", async () => {
const table = await perspective.table(SCHEMA, {
index: idx
});

const data = {
str: [1, 2, 3, 4, 5, 6].map(x => x.toString()),
int: [1, 2, 3, 4, 5, 6],
float: [1, 2, 3, 4, 5, 6].map(x => x * 0.5)
};

table.update(data);

await query(
table,
{
filter: [["int", "!=", 3]]
},
async getter => {
table.remove([1, 2, 3, 4, 5, 6].map(x => (string_pkey ? x.toString() : x)));

expect(await getter()).toEqual({});

table.update({
str: ["def", "abc", "deff"],
int: [1, 2, 4],
float: [100.5, 200.5, 300.5]
});

if (string_pkey) {
expect(await getter()).toEqual({
str: ["abc", "def", "deff"],
int: [2, 1, 4],
float: [200.5, 100.5, 300.5]
});
} else {
expect(await getter()).toEqual({
str: ["def", "abc", "deff"],
int: [1, 2, 4],
float: [100.5, 200.5, 300.5]
});
}
}
);

await table.delete();
});

it.skip("Output consistent with filter, remove and add null", async () => {
// FIXME
const table = await perspective.table(SCHEMA, {
index: idx
});
Expand Down Expand Up @@ -259,40 +405,38 @@ describe("Removes", () => {
await table.delete();
});

it("Output consistent with filter, remove and add new dataset", async () => {
it.skip("Output consistent with filter, update null", async () => {
const table = await perspective.table(SCHEMA, {
index: idx
});

const data = {
str: [1, 2, 3, 4, 5, 6].map(x => x.toString()),
int: [1, 2, 3, 4, 5, 6],
float: [1, 2, 3, 4, 5, 6].map(x => x * 0.5)
};

const splice_idx = Math.floor(Math.random() * 6);
data.str.splice(splice_idx, 0, string_pkey ? null : "7");
data.int.splice(splice_idx, 0, string_pkey ? 7 : null);
data.float.splice(splice_idx, 0, 3.5);

table.update(data);

await query(
table,
{
filter: [["int", "!=", 3]]
},
async getter => {
table.remove([1, 2, 3, 4, 5, 6].map(x => (string_pkey ? x.toString() : x)));
await query(table, {}, async getter => {
const expected = {
str: data.str.slice(),
int: data.int.slice(),
float: data.float.slice()
};

expect(await getter()).toEqual({});
expect(await getter()).toEqual(expected);

for (let i = 5; i >= 0; i--) {
table.update([{str: data.str[i], int: data.int[i], float: data.float[i]}]);
}
table.update([{str: string_pkey ? null : "7", int: string_pkey ? 7 : null, float: 4.5}]);

expect(await getter()).toEqual({
str: ["1", "2", "4", "5", "6"],
int: [1, 2, 4, 5, 6],
float: [0.5, 1, 2, 2.5, 3]
});
}
);
expected.float[splice_idx] = 4.5;

expect(await getter()).toEqual(expected);
});

await table.delete();
});
Expand Down
28 changes: 28 additions & 0 deletions packages/perspective/test/js/updates.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1380,6 +1380,34 @@ module.exports = perspective => {
table.delete();
});

it.skip("multiple updates on str {index: 'y'} with new, old, null in dataset", async function() {
// FIXME: this is an engine bug
const table = await perspective.table(
{
x: [1, 2, 3],
y: ["a", null, "c"]
},
{index: "y"}
);

const view = await table.view();

table.update([{x: 12345, y: "a"}]);
table.update([{x: 100, y: null}]);
table.update([{x: 123, y: "abc"}]);

const result = await view.to_json();
expect(result).toEqual([
{x: 100, y: null},
{x: 12345, y: "a"},
{x: 123, y: "abc"},
{x: 3, y: "c"}
]);

await view.delete();
await table.delete();
});

it("{index: 'x'} with overlap", async function() {
var table = await perspective.table(data, {index: "x"});
var view = await table.view();
Expand Down