Skip to content

Commit 33bcd7b

Browse files
texodustexodus
committed
Fix flat hierarchial sort for SQL model
Signed-off-by: Andrew Stein <steinlink@gmail.com>
1 parent 92eb162 commit 33bcd7b

3 files changed

Lines changed: 172 additions & 11 deletions

File tree

‎rust/perspective-client/src/rust/virtual_server/generic_sql_model/table_make_view.rs‎

Lines changed: 76 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -455,14 +455,64 @@ impl<'a> ViewQueryContext<'a> {
455455
fn order_by_clauses(&self) -> Vec<String> {
456456
let mut clauses = Vec::new();
457457
if !self.config.group_by.is_empty() && self.is_flat_mode() {
458-
for (sidx, Sort(sort_col, sort_dir)) in self.config.sort.iter().enumerate() {
459-
if *sort_dir != SortDir::None && !is_col_sort(sort_dir) {
460-
let dir = sort_dir_to_string(sort_dir);
461-
if !self.config.split_by.is_empty() {
462-
clauses.push(format!("__SORT_{}__ {}", sidx, dir));
463-
} else {
464-
let agg = self.get_aggregate(sort_col);
465-
clauses.push(format!("{}({}) {}", agg, self.col_name(sort_col), dir));
458+
let has_row_sort = self
459+
.config
460+
.sort
461+
.iter()
462+
.any(|Sort(_, dir)| *dir != SortDir::None && !is_col_sort(dir));
463+
if self.config.group_by.len() > 1 && has_row_sort {
464+
// Hierarchical flat sort — mirrors rollup logic but without GROUPING_ID
465+
for gidx in 0..self.config.group_by.len() {
466+
let is_leaf = gidx >= self.config.group_by.len() - 1;
467+
for (sidx, Sort(sort_col, sort_dir)) in self.config.sort.iter().enumerate() {
468+
if *sort_dir == SortDir::None || is_col_sort(sort_dir) {
469+
continue;
470+
}
471+
472+
let dir = sort_dir_to_string(sort_dir);
473+
if !self.config.split_by.is_empty() {
474+
if is_leaf {
475+
clauses.push(format!("__SORT_{}__ {}", sidx, dir));
476+
} else {
477+
clauses.push(format!(
478+
"first(__SORT_{}__) OVER __WINDOW_{}__ {}",
479+
sidx, gidx, dir
480+
));
481+
}
482+
} else {
483+
let agg = self.get_aggregate(sort_col);
484+
if is_leaf {
485+
clauses.push(format!(
486+
"{}({}) {}",
487+
agg,
488+
self.col_name(sort_col),
489+
dir
490+
));
491+
} else {
492+
clauses.push(format!(
493+
"first({}({})) OVER __WINDOW_{}__ {}",
494+
agg,
495+
self.col_name(sort_col),
496+
gidx,
497+
dir
498+
));
499+
}
500+
}
501+
}
502+
503+
clauses.push(format!("{} ASC", self.row_path_aliases[gidx]));
504+
}
505+
} else {
506+
// Single group level — simple sort, no window needed
507+
for (sidx, Sort(sort_col, sort_dir)) in self.config.sort.iter().enumerate() {
508+
if *sort_dir != SortDir::None && !is_col_sort(sort_dir) {
509+
let dir = sort_dir_to_string(sort_dir);
510+
if !self.config.split_by.is_empty() {
511+
clauses.push(format!("__SORT_{}__ {}", sidx, dir));
512+
} else {
513+
let agg = self.get_aggregate(sort_col);
514+
clauses.push(format!("{}({}) {}", agg, self.col_name(sort_col), dir));
515+
}
466516
}
467517
}
468518
}
@@ -531,14 +581,30 @@ impl<'a> ViewQueryContext<'a> {
531581
}
532582

533583
fn window_clauses(&self) -> Vec<String> {
534-
if self.is_flat_mode() || self.config.sort.is_empty() || self.config.group_by.len() <= 1 {
584+
if self.config.sort.is_empty() || self.config.group_by.len() <= 1 {
535585
return Vec::new();
536586
}
537587

538588
let mut clauses = Vec::new();
539589
for gidx in 0..(self.config.group_by.len() - 1) {
540590
let partition = self.row_path_aliases[..=gidx].join(", ");
541-
if !self.config.split_by.is_empty() {
591+
if self.is_flat_mode() {
592+
// Flat mode: partition by row path only (no GROUPING_ID)
593+
if !self.config.split_by.is_empty() {
594+
let order = self.row_path_aliases.join(", ");
595+
clauses.push(format!(
596+
"__WINDOW_{}__ AS (PARTITION BY {} ORDER BY {})",
597+
gidx, partition, order,
598+
));
599+
} else {
600+
clauses.push(format!(
601+
"__WINDOW_{}__ AS (PARTITION BY {} ORDER BY {})",
602+
gidx,
603+
partition,
604+
self.group_col_names.join(", ")
605+
));
606+
}
607+
} else if !self.config.split_by.is_empty() {
542608
let shift = self.config.group_by.len() - 1 - gidx;
543609
let grouping_expr = if shift > 0 {
544610
format!("(__GROUPING_ID__ >> {})", shift)

‎rust/perspective-js/src/ts/virtual_servers/duckdb.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,11 @@ export class DuckDBHandler implements perspective.VirtualServerHandler {
290290
await runQuery(this.db, query);
291291
}
292292

293-
async viewGetMinMax(viewId: string, columnName: string, config: ViewConfig) {
293+
async viewGetMinMax(
294+
viewId: string,
295+
columnName: string,
296+
config: ViewConfig,
297+
) {
294298
const query = this.sqlBuilder.viewGetMinMax(viewId, columnName, config);
295299
const results = await runQuery(this.db, query);
296300
const row = results[0].toJSON();

‎rust/perspective-js/test/js/duckdb/combined.spec.js‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ describeDuckDB("combined operations", (getClient) => {
9494
"West|Sales": 165134.77900000007,
9595
},
9696
]);
97+
9798
await view.delete();
9899
});
99100

@@ -124,6 +125,7 @@ describeDuckDB("combined operations", (getClient) => {
124125
"West|Sales": null,
125126
},
126127
]);
128+
127129
await view.delete();
128130
});
129131

@@ -155,6 +157,93 @@ describeDuckDB("combined operations", (getClient) => {
155157
"West|Sales": null,
156158
},
157159
]);
160+
161+
await view.delete();
162+
});
163+
164+
test("flat + multi group_by + sort", async function () {
165+
const table = await getClient().open_table("memory.superstore");
166+
const view = await table.view({
167+
columns: ["Sales"],
168+
group_by: ["Region", "Category"],
169+
sort: [["Sales", "desc"]],
170+
aggregates: { Sales: "sum" },
171+
group_rollup_mode: "flat",
172+
});
173+
const json = await view.to_json({ start_row: 0, end_row: 5 });
174+
expect(json).toEqual([
175+
{
176+
__ROW_PATH__: ["West", "Furniture"],
177+
Sales: 252612.7435000003,
178+
},
179+
{
180+
__ROW_PATH__: ["West", "Technology"],
181+
Sales: 251991.83199999997,
182+
},
183+
{
184+
__ROW_PATH__: ["West", "Office Supplies"],
185+
Sales: 220853.24900000007,
186+
},
187+
{
188+
__ROW_PATH__: ["East", "Technology"],
189+
Sales: 264973.9810000003,
190+
},
191+
{
192+
__ROW_PATH__: ["East", "Furniture"],
193+
Sales: 208291.20400000009,
194+
},
195+
]);
196+
await view.delete();
197+
});
198+
199+
test("flat + multi group_by + split_by + sort", async function () {
200+
const table = await getClient().open_table("memory.superstore");
201+
const view = await table.view({
202+
columns: ["Sales"],
203+
group_by: ["Region", "Category"],
204+
split_by: ["Ship Mode"],
205+
sort: [["Sales", "desc"]],
206+
aggregates: { Sales: "sum" },
207+
group_rollup_mode: "flat",
208+
});
209+
const json = await view.to_json({ start_row: 0, end_row: 5 });
210+
expect(json).toEqual([
211+
{
212+
__ROW_PATH__: ["West", "Furniture"],
213+
"First Class|Sales": 40018.829499999985,
214+
"Same Day|Sales": 14527.978000000001,
215+
"Second Class|Sales": 54155.6555,
216+
"Standard Class|Sales": 143910.28049999996,
217+
},
218+
{
219+
__ROW_PATH__: ["West", "Technology"],
220+
"First Class|Sales": 61107.98900000001,
221+
"Same Day|Sales": 19218.053999999993,
222+
"Second Class|Sales": 38610.979999999996,
223+
"Standard Class|Sales": 133054.809,
224+
},
225+
{
226+
__ROW_PATH__: ["West", "Office Supplies"],
227+
"First Class|Sales": 28635.06999999996,
228+
"Same Day|Sales": 9857.678000000002,
229+
"Second Class|Sales": 52572.79199999999,
230+
"Standard Class|Sales": 129787.70900000003,
231+
},
232+
{
233+
__ROW_PATH__: ["East", "Technology"],
234+
"First Class|Sales": 47693.312999999995,
235+
"Same Day|Sales": 21349.464999999997,
236+
"Second Class|Sales": 29304.490000000005,
237+
"Standard Class|Sales": 166626.71300000005,
238+
},
239+
{
240+
__ROW_PATH__: ["East", "Furniture"],
241+
"First Class|Sales": 29410.643999999997,
242+
"Same Day|Sales": 12852.570999999996,
243+
"Second Class|Sales": 44035.937000000005,
244+
"Standard Class|Sales": 121992.05199999997,
245+
},
246+
]);
158247
await view.delete();
159248
});
160249

@@ -167,6 +256,7 @@ describeDuckDB("combined operations", (getClient) => {
167256
sort: [["profitmargin", "desc"]],
168257
aggregates: { profitmargin: "avg" },
169258
});
259+
170260
const json = await view.to_json();
171261
expect(json).toEqual([
172262
{
@@ -190,6 +280,7 @@ describeDuckDB("combined operations", (getClient) => {
190280
profitmargin: -10.407293926323575,
191281
},
192282
]);
283+
193284
await view.delete();
194285
});
195286
});

0 commit comments

Comments
 (0)