Skip to content

Fix last aggregate to preserve status #1390

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

Merged
texodus merged 1 commit into from
Apr 23, 2021
Merged

Fix last aggregate to preserve status #1390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
53 changes: 0 additions & 53 deletions cpp/perspective/src/cpp/aggregate.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,59 +211,6 @@ t_aggregate::init() {
default: { PSP_COMPLAIN_AND_ABORT("Unexpected dtype"); }
}
} break;
case AGGTYPE_LAST_VALUE: {
switch (m_icolumns[0]->get_dtype()) {
case DTYPE_TIME:
case DTYPE_INT64: {
build_aggregate<
t_aggimpl_last_value<std::int64_t, std::int64_t, std::int64_t>>();
} break;
case DTYPE_INT32: {
build_aggregate<
t_aggimpl_last_value<std::int32_t, std::int32_t, std::int32_t>>();
} break;
case DTYPE_INT16: {
build_aggregate<
t_aggimpl_last_value<std::int16_t, std::int16_t, std::int16_t>>();
} break;
case DTYPE_INT8: {
build_aggregate<
t_aggimpl_last_value<std::int8_t, std::int8_t, std::int8_t>>();
} break;
case DTYPE_UINT64: {
build_aggregate<
t_aggimpl_last_value<std::uint64_t, std::uint64_t, std::uint64_t>>();
} break;
case DTYPE_DATE:
case DTYPE_UINT32: {
build_aggregate<
t_aggimpl_last_value<std::uint32_t, std::uint32_t, std::uint32_t>>();
} break;
case DTYPE_UINT16: {
build_aggregate<
t_aggimpl_last_value<std::uint16_t, std::uint16_t, std::uint16_t>>();
} break;
case DTYPE_UINT8: {
build_aggregate<
t_aggimpl_last_value<std::uint8_t, std::uint8_t, std::uint8_t>>();
} break;
case DTYPE_FLOAT64: {
build_aggregate<t_aggimpl_last_value<double, double, double>>();
} break;
case DTYPE_FLOAT32: {
build_aggregate<t_aggimpl_last_value<float, float, float>>();
} break;
case DTYPE_BOOL: {
build_aggregate<
t_aggimpl_last_value<std::uint8_t, std::uint8_t, std::uint8_t>>();
} break;
case DTYPE_STR: {
build_aggregate<
t_aggimpl_last_value<const char*, const char*, const char*>>();
} break;
default: { PSP_COMPLAIN_AND_ABORT("Unexpected dtype"); }
}
} break;
case AGGTYPE_HIGH_WATER_MARK: {
switch (m_icolumns[0]->get_dtype()) {
case DTYPE_TIME:
Expand Down
12 changes: 12 additions & 0 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 @@ -371,6 +371,18 @@ t_gstate::get_table() const {
return m_table;
}

t_tscalar
t_gstate::read_by_pkey(const std::string& colname, t_tscalar& pkey) const {
std::shared_ptr<const t_column> col = m_table->get_const_column(colname);
const t_column* col_ = col.get();
t_mapping::const_iterator iter = m_mapping.find(pkey);
if (iter != m_mapping.end()) {
return col_->get_scalar(iter->second);
} else {
PSP_COMPLAIN_AND_ABORT("Called without pkey");
}
}

void
t_gstate::read_column(const std::string& colname, const std::vector<t_tscalar>& pkeys,
std::vector<t_tscalar>& out_data) const {
Expand Down
26 changes: 21 additions & 5 deletions cpp/perspective/src/cpp/sparse_tree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1108,13 +1108,29 @@ t_stree::update_agg_table(t_uindex nidx, t_agg_update_info& info, t_uindex src_r
dst->set_scalar(dst_ridx, new_value);
} break;
case AGGTYPE_LAST_VALUE: {
t_tscalar src_scalar = src->get_scalar(src_ridx);
t_tscalar dst_scalar = dst->get_scalar(dst_ridx);
old_value.set(dst_scalar);
t_uindex leaf;
if (is_leaf(nidx)) {
leaf = nidx;
} else {
auto iters = m_idxleaf->get<by_idx_lfidx>().equal_range(nidx);
if (iters.first != iters.second) {
leaf = (--iters.second)->m_lfidx;
} else {
dst->set_scalar(dst_ridx, mknone());
break;
}
}

old_value.set(dst_scalar);
new_value.set(src_scalar);

dst->set_scalar(dst_ridx, new_value);
auto iters = m_idxpkey->get<by_idx_pkey>().equal_range(leaf);
if (iters.first != iters.second) {
t_tscalar pkey = (--iters.second)->m_pkey;
std::vector<t_tscalar> values;
dst->set_scalar(dst_ridx, gstate.read_by_pkey(spec.get_dependencies()[0].name(), pkey));
} else {
dst->set_scalar(dst_ridx, mknone());
}
} break;
case AGGTYPE_HIGH_WATER_MARK: {
t_tscalar src_scalar = src->get_scalar(src_ridx);
Expand Down
22 changes: 0 additions & 22 deletions cpp/perspective/src/include/perspective/aggregate.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,28 +126,6 @@ class PERSPECTIVE_EXPORT t_aggimpl_mean : public t_aggimpl<RAW_DATA_T, ROLLING_T
}
};

template <typename RAW_DATA_T, typename ROLLING_T, typename RESULT_T>
class PERSPECTIVE_EXPORT t_aggimpl_last_value
: public t_aggimpl<RAW_DATA_T, ROLLING_T, RESULT_T> {
public:
RESULT_T
value(ROLLING_T rs) { return RESULT_T(rs); }

ROLLING_T
reduce(const RAW_DATA_T* biter, const RAW_DATA_T* eiter) {
if (biter >= eiter)
return ROLLING_T();
return ROLLING_T(*(eiter - 1));
}

ROLLING_T
roll_up(const ROLLING_T* biter, const ROLLING_T* eiter) {
if (biter >= eiter)
return ROLLING_T();

return ROLLING_T(*(eiter - 1));
}
};

template <typename RAW_DATA_T, typename ROLLING_T, typename RESULT_T>
class PERSPECTIVE_EXPORT t_aggimpl_hwm : public t_aggimpl<RAW_DATA_T, ROLLING_T, RESULT_T> {
Expand Down
3 changes: 3 additions & 0 deletions cpp/perspective/src/include/perspective/gnode_state.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,9 @@ class PERSPECTIVE_EXPORT t_gstate {
const std::vector<t_uindex>& master_table_indexes,
t_uindex num_rows);

t_tscalar read_by_pkey(
const std::string& colname, t_tscalar& pkey) const;

/**
* @brief Read the values with the specified `pkeys` from the column at
* `colname`, writing into `out_data`.
Expand Down
80 changes: 80 additions & 0 deletions packages/perspective/test/js/pivot_nulls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,86 @@

module.exports = perspective => {
describe("Pivotting with nulls", function() {
describe("last aggregate", function() {
it("preserves null when it is the last element in a leaf", async function() {
const DATA = {a: ["a", "a", "a", "b", "b", "b", "c", "c", "c"], b: [1, 2, null, 3, 4, 5, null, null, null]};
var table = await perspective.table(DATA);
var view = await table.view({
row_pivots: ["a"],
columns: ["b"],
aggregates: {b: "last"}
});
var answer = [
{__ROW_PATH__: [], b: null},
{__ROW_PATH__: ["a"], b: null},
{__ROW_PATH__: ["b"], b: 5},
{__ROW_PATH__: ["c"], b: null}
];
let result = await view.to_json();
expect(result).toEqual(answer);
view.delete();
table.delete();
});

it("preserves null when it is the last element in a leaf under 2 levels", async function() {
const DATA = {
a: ["a", "a", "a", "b", "b", "b", "c", "c", "c", "a", "a", "a", "b", "b", "b", "c", "c", "c"],
b: [1, 2, null, 3, 4, 5, null, null, null, 1, 2, null, null, null, null, 3, 4, 5],
c: ["x", "x", "x", "x", "x", "x", "x", "x", "x", "y", "y", "y", "y", "y", "y", "y", "y", "y"]
};
var table = await perspective.table(DATA);
var view = await table.view({
row_pivots: ["c", "a"],
columns: ["b"],
aggregates: {b: "last"}
});
var answer = [
{__ROW_PATH__: [], b: 5},
{__ROW_PATH__: ["x"], b: null},
{__ROW_PATH__: ["x", "a"], b: null},
{__ROW_PATH__: ["x", "b"], b: 5},
{__ROW_PATH__: ["x", "c"], b: null},
{__ROW_PATH__: ["y"], b: 5},
{__ROW_PATH__: ["y", "a"], b: null},
{__ROW_PATH__: ["y", "b"], b: null},
{__ROW_PATH__: ["y", "c"], b: 5}
];
let result = await view.to_json();
expect(result).toEqual(answer);
view.delete();
table.delete();
});

it("preserves null when it is the last element in a leaf under 2 levels when grand total is null", async function() {
const DATA = {
a: ["a", "a", "a", "b", "b", "b", "c", "c", "c", "a", "a", "a", "b", "b", "b", "c", "c", "c"],
b: [1, 2, null, null, null, null, 3, 4, 5, 1, 2, null, 3, 4, 5, null, null, null],
c: ["x", "x", "x", "x", "x", "x", "x", "x", "x", "y", "y", "y", "y", "y", "y", "y", "y", "y"]
};
var table = await perspective.table(DATA);
var view = await table.view({
row_pivots: ["c", "a"],
columns: ["b"],
aggregates: {b: "last"}
});
var answer = [
{__ROW_PATH__: [], b: null},
{__ROW_PATH__: ["x"], b: 5},
{__ROW_PATH__: ["x", "a"], b: null},
{__ROW_PATH__: ["x", "b"], b: null},
{__ROW_PATH__: ["x", "c"], b: 5},
{__ROW_PATH__: ["y"], b: null},
{__ROW_PATH__: ["y", "a"], b: null},
{__ROW_PATH__: ["y", "b"], b: 5},
{__ROW_PATH__: ["y", "c"], b: null}
];
let result = await view.to_json();
expect(result).toEqual(answer);
view.delete();
table.delete();
});
});

it("shows one pivot for the nulls on initial load", async function() {
const dataWithNulls = [
{name: "Homer", value: 1},
Expand Down