Skip to content

Commit c2ea98c

Browse files
texodustexodus
committed
Fix C++ leaves_only implementation
Signed-off-by: Andrew Stein <steinlink@gmail.com>
1 parent b3608fa commit c2ea98c

18 files changed

Lines changed: 278 additions & 176 deletions

File tree

‎rust/perspective-client/perspective.proto‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -393,7 +393,6 @@ message ViewToColumnsStringReq {
393393
optional bool id = 2;
394394
optional bool index = 3;
395395
optional bool formatted = 4;
396-
optional bool leaves_only = 5;
397396
}
398397

399398
message ViewToColumnsStringResp {
@@ -405,7 +404,6 @@ message ViewToRowsStringReq {
405404
optional bool id = 2;
406405
optional bool index = 3;
407406
optional bool formatted = 4;
408-
optional bool leaves_only = 5;
409407
}
410408

411409
message ViewToRowsStringResp {
@@ -417,7 +415,6 @@ message ViewToNdjsonStringReq {
417415
optional bool id = 2;
418416
optional bool index = 3;
419417
optional bool formatted = 4;
420-
optional bool leaves_only = 5;
421418
}
422419

423420
message ViewToNdjsonStringResp {
@@ -522,6 +519,7 @@ message ViewConfig {
522519
map<string, AggList> aggregates = 7;
523520
FilterReducer filter_op = 8;
524521
optional uint32 group_by_depth = 9;
522+
optional bool leaves_only = 10;
525523

526524
message AggList {
527525
repeated string aggregations = 1;

‎rust/perspective-client/src/rust/config/view_config.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,10 @@ pub struct ViewConfig {
5353
#[serde(skip_serializing_if = "Option::is_none")]
5454
#[serde(default)]
5555
pub group_by_depth: Option<u32>,
56+
57+
#[serde(skip_serializing_if = "Option::is_none")]
58+
#[serde(default)]
59+
pub leaves_only: Option<bool>,
5660
}
5761

5862
fn is_default_value<A: Default + PartialEq>(value: &A) -> bool {
@@ -160,6 +164,11 @@ pub struct ViewConfigUpdate {
160164
#[ts(optional)]
161165
pub group_by_depth: Option<u32>,
162166

167+
#[serde(skip_serializing_if = "Option::is_none")]
168+
#[serde(default)]
169+
#[ts(optional)]
170+
pub leaves_only: Option<bool>,
171+
163172
#[serde(skip_serializing_if = "Option::is_none")]
164173
#[serde(default)]
165174
#[ts(optional)]
@@ -202,6 +211,7 @@ impl From<ViewConfigUpdate> for proto::ViewConfig {
202211
.map(|(x, y)| (x, y.into()))
203212
.collect(),
204213
group_by_depth: value.group_by_depth,
214+
leaves_only: value.leaves_only,
205215
}
206216
}
207217
}
@@ -236,6 +246,7 @@ impl From<ViewConfig> for ViewConfigUpdate {
236246
expressions: Some(value.expressions),
237247
aggregates: Some(value.aggregates),
238248
group_by_depth: value.group_by_depth,
249+
leaves_only: value.leaves_only,
239250
}
240251
}
241252
}
@@ -265,6 +276,7 @@ impl From<proto::ViewConfig> for ViewConfig {
265276
.map(|(x, y)| (x, y.into()))
266277
.collect(),
267278
group_by_depth: value.group_by_depth,
279+
leaves_only: value.leaves_only,
268280
}
269281
}
270282
}
@@ -281,6 +293,7 @@ impl From<ViewConfigUpdate> for ViewConfig {
281293
expressions: value.expressions.unwrap_or_default(),
282294
aggregates: value.aggregates.unwrap_or_default(),
283295
group_by_depth: value.group_by_depth,
296+
leaves_only: value.leaves_only,
284297
}
285298
}
286299
}
@@ -312,6 +325,7 @@ impl From<proto::ViewConfig> for ViewConfigUpdate {
312325
.collect(),
313326
),
314327
group_by_depth: value.group_by_depth,
328+
leaves_only: value.leaves_only,
315329
}
316330
}
317331
}

‎rust/perspective-client/src/rust/view.rs‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,6 @@ pub struct ViewWindow {
100100
#[serde(skip_serializing_if = "Option::is_none")]
101101
pub index: Option<bool>,
102102

103-
#[ts(optional)]
104-
#[serde(skip_serializing_if = "Option::is_none")]
105-
pub leaves_only: Option<bool>,
106-
107103
/// Only impacts [`View::to_csv`]
108104
#[ts(optional)]
109105
#[serde(skip_serializing_if = "Option::is_none")]
@@ -383,7 +379,7 @@ impl View {
383379
id: window.id,
384380
index: window.index,
385381
formatted: window.formatted,
386-
leaves_only: window.leaves_only,
382+
387383
}));
388384

389385
match self.client.oneshot(&msg).await? {
@@ -402,7 +398,7 @@ impl View {
402398
id: window.id,
403399
index: window.index,
404400
formatted: window.formatted,
405-
leaves_only: window.leaves_only,
401+
406402
}));
407403

408404
match self.client.oneshot(&msg).await? {
@@ -422,7 +418,7 @@ impl View {
422418
id: window.id,
423419
index: window.index,
424420
formatted: window.formatted,
425-
leaves_only: window.leaves_only,
421+
426422
}));
427423

428424
match self.client.oneshot(&msg).await? {

‎rust/perspective-js/test/js/to_format.spec.js‎

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -525,8 +525,9 @@ const pivoted_output = [
525525
let table = await perspective.table(int_float_string_data);
526526
let view = await table.view({
527527
group_by: ["int"],
528+
leaves_only: true,
528529
});
529-
let json = await view.to_json({ leaves_only: true });
530+
let json = await view.to_json();
530531
expect(json).toEqual([
531532
{
532533
__ROW_PATH__: [1],
@@ -560,6 +561,79 @@ const pivoted_output = [
560561
view.delete();
561562
table.delete();
562563
});
564+
565+
test("num_rows returns correct leaf count", async function () {
566+
let table = await perspective.table(int_float_string_data);
567+
let view = await table.view({
568+
group_by: ["int"],
569+
leaves_only: true,
570+
});
571+
let num_rows = await view.num_rows();
572+
expect(num_rows).toEqual(4);
573+
view.delete();
574+
table.delete();
575+
});
576+
577+
test("viewport pagination returns exact page sizes", async function () {
578+
let table = await perspective.table(int_float_string_data);
579+
let view = await table.view({
580+
group_by: ["int"],
581+
leaves_only: true,
582+
});
583+
let json = await view.to_json({
584+
start_row: 0,
585+
end_row: 2,
586+
});
587+
expect(json.length).toEqual(2);
588+
expect(json[0].__ROW_PATH__).toEqual([1]);
589+
expect(json[1].__ROW_PATH__).toEqual([2]);
590+
view.delete();
591+
table.delete();
592+
});
593+
594+
test("to_columns works with leaves_only", async function () {
595+
let table = await perspective.table(int_float_string_data);
596+
let view = await table.view({
597+
group_by: ["int"],
598+
leaves_only: true,
599+
});
600+
let cols = await view.to_columns();
601+
expect(cols["__ROW_PATH__"]).toEqual([[1], [2], [3], [4]]);
602+
expect(cols["int"]).toEqual([1, 2, 3, 4]);
603+
view.delete();
604+
table.delete();
605+
});
606+
607+
test("leaves_only with multi-level group_by", async function () {
608+
let table = await perspective.table(int_float_string_data);
609+
let view = await table.view({
610+
group_by: ["string", "int"],
611+
leaves_only: true,
612+
});
613+
let num_rows = await view.num_rows();
614+
expect(num_rows).toEqual(4);
615+
let json = await view.to_json();
616+
for (let row of json) {
617+
expect(row.__ROW_PATH__.length).toEqual(2);
618+
}
619+
view.delete();
620+
table.delete();
621+
});
622+
623+
test("leaves_only updates after table.update()", async function () {
624+
let table = await perspective.table(int_float_string_data);
625+
let view = await table.view({
626+
group_by: ["int"],
627+
leaves_only: true,
628+
});
629+
let num_rows = await view.num_rows();
630+
expect(num_rows).toEqual(4);
631+
table.update([{ int: 5, float: 6.0, string: "e", datetime: STD_DATE }]);
632+
let num_rows2 = await view.num_rows();
633+
expect(num_rows2).toEqual(5);
634+
view.delete();
635+
table.delete();
636+
});
563637
});
564638

565639
test.describe("to_arrow()", function () {

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -346,9 +346,13 @@ void
346346
t_ctx1::step_end() {
347347
PSP_TRACE_SENTINEL();
348348
PSP_VERBOSE_ASSERT(m_init, "touching uninited object");
349-
sort_by(m_sortby);
350-
if (m_depth_set) {
351-
set_depth(m_depth);
349+
if (m_leaves_only) {
350+
m_traversal->rebuild_from_leaves(m_sortby);
351+
} else {
352+
sort_by(m_sortby);
353+
if (m_depth_set) {
354+
set_depth(m_depth);
355+
}
352356
}
353357
}
354358

@@ -424,6 +428,14 @@ t_ctx1::set_depth(t_depth depth) {
424428
m_depth_set = true;
425429
}
426430

431+
void
432+
t_ctx1::set_leaves_only(bool enabled) {
433+
PSP_TRACE_SENTINEL();
434+
PSP_VERBOSE_ASSERT(m_init, "touching uninited object");
435+
m_leaves_only = enabled;
436+
m_traversal->set_leaves_only(enabled, m_config.get_num_rpivots());
437+
}
438+
427439
std::vector<t_tscalar>
428440
t_ctx1::get_pkeys(const std::vector<std::pair<t_uindex, t_uindex>>& cells
429441
) const {

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,12 @@ t_ctx2::step_begin() {
110110

111111
void
112112
t_ctx2::step_end() {
113-
if (m_row_depth_set) {
114-
set_depth(HEADER_ROW, m_row_depth);
113+
if (m_leaves_only) {
114+
m_rtraversal->rebuild_from_leaves(m_sortby);
115+
} else {
116+
if (m_row_depth_set) {
117+
set_depth(HEADER_ROW, m_row_depth);
118+
}
115119
}
116120
if (m_column_depth_set) {
117121
set_depth(HEADER_COLUMN, m_column_depth);
@@ -935,6 +939,12 @@ t_ctx2::set_depth(t_header header, t_depth depth) {
935939
}
936940
}
937941

942+
void
943+
t_ctx2::set_leaves_only(bool enabled) {
944+
m_leaves_only = enabled;
945+
m_rtraversal->set_leaves_only(enabled, m_config.get_num_rpivots());
946+
}
947+
938948
std::vector<t_tscalar>
939949
t_ctx2::get_pkeys(const std::vector<std::pair<t_uindex, t_uindex>>& cells
940950
) const {

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,9 @@ make_context(
130130

131131
auto pool = table->get_pool();
132132
auto gnode = table->get_gnode();
133-
if (row_pivot_depth > -1) {
133+
if (view_config->is_leaves_only()) {
134+
ctx1->set_leaves_only(true);
135+
} else if (row_pivot_depth > -1) {
134136
ctx1->set_depth(row_pivot_depth - 1);
135137
} else {
136138
ctx1->set_depth(row_pivots.size());
@@ -193,7 +195,9 @@ make_context(
193195
ctx2->column_sort_by(col_sortspec);
194196
}
195197

196-
if (row_pivot_depth > -1) {
198+
if (view_config->is_leaves_only()) {
199+
ctx2->set_leaves_only(true);
200+
} else if (row_pivot_depth > -1) {
197201
ctx2->set_depth(t_header::HEADER_ROW, row_pivot_depth - 1);
198202
} else {
199203
ctx2->set_depth(t_header::HEADER_ROW, row_pivots.size());
@@ -2091,6 +2095,9 @@ ProtoServer::_handle_request(std::uint32_t client_id, Request&& req) {
20912095

20922096
LOG_DEBUG("FILTER_OP: " << filter_op);
20932097

2098+
bool leaves_only =
2099+
cfg.has_leaves_only() ? cfg.leaves_only() : false;
2100+
20942101
auto config = std::make_shared<t_view_config>(
20952102
vocab,
20962103
row_pivots,
@@ -2101,7 +2108,8 @@ ProtoServer::_handle_request(std::uint32_t client_id, Request&& req) {
21012108
sort_str,
21022109
expressions,
21032110
filter_op,
2104-
column_only
2111+
column_only,
2112+
leaves_only
21052113
);
21062114
config->init(schema);
21072115

@@ -2486,7 +2494,6 @@ ProtoServer::_handle_request(std::uint32_t client_id, Request&& req) {
24862494
r.formatted(),
24872495
r.index(),
24882496
r.id(),
2489-
r.leaves_only(),
24902497
view->sides(),
24912498
view->sides() > 0 && !config->is_column_only(),
24922499
nidx,
@@ -2525,7 +2532,6 @@ ProtoServer::_handle_request(std::uint32_t client_id, Request&& req) {
25252532
r.formatted(),
25262533
r.index(),
25272534
r.id(),
2528-
r.leaves_only(),
25292535
view->sides(),
25302536
view->sides() > 0 && !config->is_column_only(),
25312537
nidx,
@@ -2565,7 +2571,6 @@ ProtoServer::_handle_request(std::uint32_t client_id, Request&& req) {
25652571
r.formatted(),
25662572
r.index(),
25672573
r.id(),
2568-
r.leaves_only(),
25692574
view->sides(),
25702575
view->sides() > 0 && !config->is_column_only(),
25712576
nidx,

0 commit comments

Comments
 (0)