Skip to content

Commit 60d0a03

Browse files
committed
Fix DuckDB arrow type columns
Signed-off-by: Andrew Stein <steinlink@gmail.com>
1 parent d1453e6 commit 60d0a03

11 files changed

Lines changed: 86 additions & 23 deletions

File tree

.github/workflows/build.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ on:
2828
- docs/
2929
- examples/
3030
- rust/perspective-python/README.md
31-
pull_request:
31+
pull_request_target:
3232
branches:
3333
- master
3434
workflow_dispatch:

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/viewer-charts/src/ts/charts/cartesian/cartesian-render.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ export function renderCartesianFrame(
8787
const facetMode = chart._facetConfig.facet_mode;
8888
const useGrid = hasSplits && facetMode === "grid";
8989

90+
chart.computeEffectiveFacetFlags();
91+
9092
// Legend appears only when the user wired a color column with a
9193
// non-degenerate range. `split_by` alone no longer forces a
9294
// legend — faceting is the axis of splitting, not coloring.

rust/perspective-client/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ protobuf-src = { version = "2.1.1", optional = true }
6868
arrow-array = { version = "57.3.0", default-features = false }
6969
arrow-ipc = { version = "57.3.0", default-features = false }
7070
arrow-schema = { version = "57.3.0", default-features = false }
71+
arrow-select = { version = "57.3.0", default-features = false }
7172
async-lock = { version = "2.5.0" }
7273
futures = { version = "0.3.28" }
7374
indexmap = { version = "2.2.6", features = ["serde"] }

rust/perspective-client/src/rust/virtual_server/data.rs

Lines changed: 56 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,21 @@ pub enum VirtualDataCell {
6464
RowPath(Vec<Scalar>),
6565
}
6666

67+
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
68+
pub enum RowPathStyle {
69+
/// Legacy: emit a single `__ROW_PATH__` sidecar (per-row nested
70+
/// array in `render_to_rows`, array-of-arrays in
71+
/// `render_to_columns_json`). `__ROW_PATH_N__` per-level columns
72+
/// are filtered out. Matches the native engine's `to_json` /
73+
/// `to_columns` shape.
74+
Sidecar,
75+
76+
/// Native: emit per-level `__ROW_PATH_0__`, `__ROW_PATH_1__`, …
77+
/// columns directly. No `__ROW_PATH__` sidecar. Matches the native
78+
/// engine's Arrow IPC, CSV, and NDJSON shapes.
79+
PerLevel,
80+
}
81+
6782
/// Trait for types that can be written to a [`ColumnBuilder`] which
6883
/// enforces sequential construction.
6984
///
@@ -593,14 +608,16 @@ impl VirtualDataSlice {
593608
/// to Perspective-compatible types.
594609
pub fn from_arrow_ipc(&mut self, ipc: &[u8]) -> Result<(), Box<dyn Error>> {
595610
let cursor = std::io::Cursor::new(ipc);
596-
let batch = if &ipc[0..6] == "ARROW1".as_bytes() {
597-
FileReader::try_new(cursor, None)?
598-
.next()
599-
.ok_or("Arrow IPC stream contained no record batches")??
611+
let batches: Vec<RecordBatch> = if &ipc[0..6] == "ARROW1".as_bytes() {
612+
FileReader::try_new(cursor, None)?.collect::<Result<Vec<_>, _>>()?
600613
} else {
601-
StreamReader::try_new(cursor, None)?
602-
.next()
603-
.ok_or("Arrow IPC stream contained no record batches")??
614+
StreamReader::try_new(cursor, None)?.collect::<Result<Vec<_>, _>>()?
615+
};
616+
617+
let batch = match batches.len() {
618+
0 => return Err("Arrow IPC stream contained no record batches".into()),
619+
1 => batches.into_iter().next().unwrap(),
620+
_ => arrow_select::concat::concat_batches(&batches[0].schema(), &batches)?,
604621
};
605622

606623
let has_group_by = !self.config.group_by.is_empty();
@@ -764,17 +781,24 @@ impl VirtualDataSlice {
764781

765782
/// Converts the columnar data to a row-oriented representation for JSON
766783
/// serialization.
767-
pub(crate) fn render_to_rows(&mut self) -> Vec<IndexMap<String, VirtualDataCell>> {
784+
///
785+
/// `style` selects between the legacy `__ROW_PATH__` sidecar
786+
/// (`Sidecar`, used by `to_json`) and the native per-level
787+
/// `__ROW_PATH_N__` columns (`PerLevel`, used by `to_csv` /
788+
/// `to_ndjson`). See [`RowPathStyle`] for the deprecation plan.
789+
pub(crate) fn render_to_rows(
790+
&mut self,
791+
style: RowPathStyle,
792+
) -> Vec<IndexMap<String, VirtualDataCell>> {
768793
let batch = self.freeze().clone();
769794
let num_rows = batch.num_rows();
770795
let schema = batch.schema();
771796

772797
(0..num_rows)
773798
.map(|row_idx| {
774799
let mut row = IndexMap::new();
775-
776-
// Add RowPath column first if present
777-
if let Some(ref rp) = self.row_path
800+
if style == RowPathStyle::Sidecar
801+
&& let Some(ref rp) = self.row_path
778802
&& row_idx < rp.len()
779803
{
780804
row.insert(
@@ -783,8 +807,11 @@ impl VirtualDataSlice {
783807
);
784808
}
785809

786-
// Add Arrow columns
787810
for (col_idx, field) in schema.fields().iter().enumerate() {
811+
if style == RowPathStyle::Sidecar && field.name().starts_with("__ROW_PATH_") {
812+
continue;
813+
}
814+
788815
let col = batch.column(col_idx);
789816
let cell = if col.is_null(row_idx) {
790817
match field.data_type() {
@@ -869,17 +896,31 @@ impl VirtualDataSlice {
869896
}
870897

871898
/// Serializes the data to a column-oriented JSON string.
872-
pub fn render_to_columns_json(&mut self) -> Result<String, Box<dyn Error>> {
899+
///
900+
/// `style` selects between the legacy `__ROW_PATH__` sidecar
901+
/// (`Sidecar`, used by `to_columns`) and the native per-level
902+
/// `__ROW_PATH_N__` columns (`PerLevel`, currently unused — reserved
903+
/// for the future deprecation of `__ROW_PATH__`). See
904+
/// [`RowPathStyle`] for context.
905+
pub fn render_to_columns_json(
906+
&mut self,
907+
style: RowPathStyle,
908+
) -> Result<String, Box<dyn Error>> {
873909
let batch = self.freeze().clone();
874910
let schema = batch.schema();
875911
let mut map = serde_json::Map::new();
876912

877-
// Add RowPath if present
878-
if let Some(ref rp) = self.row_path {
913+
if style == RowPathStyle::Sidecar
914+
&& let Some(ref rp) = self.row_path
915+
{
879916
map.insert("__ROW_PATH__".to_string(), serde_json::to_value(rp)?);
880917
}
881918

882919
for (col_idx, field) in schema.fields().iter().enumerate() {
920+
if style == RowPathStyle::Sidecar && field.name().starts_with("__ROW_PATH_") {
921+
continue;
922+
}
923+
883924
let col = batch.column(col_idx);
884925
let num_rows = col.len();
885926
let values: serde_json::Value = match field.data_type() {

rust/perspective-client/src/rust/virtual_server/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ mod generic_sql_model;
2222
mod handler;
2323
mod server;
2424

25-
pub use data::{SetVirtualDataColumn, VirtualDataCell, VirtualDataSlice};
25+
pub use data::{RowPathStyle, SetVirtualDataColumn, VirtualDataCell, VirtualDataSlice};
2626
pub use error::{ResultExt, VirtualServerError};
2727
pub use features::{AggSpec, Features};
2828
pub use generic_sql_model::{

rust/perspective-client/src/rust/virtual_server/server.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use indexmap::IndexMap;
1616
use prost::Message as ProstMessage;
1717
use prost::bytes::{Bytes, BytesMut};
1818

19+
use super::data::RowPathStyle;
1920
use super::error::VirtualServerError;
2021
use super::handler::VirtualServerHandler;
2122
use crate::config::{ViewConfig, ViewConfigUpdate};
@@ -322,7 +323,7 @@ impl<T: VirtualServerHandler> VirtualServer<T> {
322323
.view_get_data(msg.entity_id.as_str(), config, &schema, &viewport)
323324
.await?;
324325

325-
let rows = cols.render_to_rows();
326+
let rows = cols.render_to_rows(RowPathStyle::PerLevel);
326327
let mut csv = String::new();
327328
if let Some(first_row) = rows.first() {
328329
let headers: Vec<&str> = first_row.keys().map(|k| k.as_str()).collect();
@@ -350,7 +351,7 @@ impl<T: VirtualServerHandler> VirtualServer<T> {
350351
.view_get_data(msg.entity_id.as_str(), config, &schema, &viewport)
351352
.await?;
352353

353-
let rows = cols.render_to_rows();
354+
let rows = cols.render_to_rows(RowPathStyle::PerLevel);
354355
let ndjson_string = rows
355356
.iter()
356357
.map(serde_json::to_string)
@@ -369,7 +370,7 @@ impl<T: VirtualServerHandler> VirtualServer<T> {
369370
.view_get_data(msg.entity_id.as_str(), config, &schema, &viewport)
370371
.await?;
371372

372-
let rows = cols.render_to_rows();
373+
let rows = cols.render_to_rows(RowPathStyle::Sidecar);
373374
let json_string = serde_json::to_string(&rows)
374375
.map_err(|e| VirtualServerError::InvalidJSON(std::sync::Arc::new(e)))?;
375376

@@ -385,7 +386,7 @@ impl<T: VirtualServerHandler> VirtualServer<T> {
385386
.await?;
386387

387388
let json_string = cols
388-
.render_to_columns_json()
389+
.render_to_columns_json(RowPathStyle::Sidecar)
389390
.map_err(|e| VirtualServerError::Other(e.to_string()))?;
390391

391392
respond!(msg, ViewToColumnsStringResp { json_string })

rust/perspective-python/src/server/virtual_server_sync.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ use chrono::{DateTime, TimeZone, Utc};
1717
use indexmap::IndexMap;
1818
use perspective_client::proto::{ColumnType, HostedTable};
1919
use perspective_client::virtual_server::{
20-
Features, ResultExt, VirtualDataSlice, VirtualServer, VirtualServerFuture, VirtualServerHandler,
20+
Features, ResultExt, RowPathStyle, VirtualDataSlice, VirtualServer, VirtualServerFuture,
21+
VirtualServerHandler,
2122
};
2223
use pyo3::exceptions::PyValueError;
2324
use pyo3::types::{
@@ -415,7 +416,7 @@ impl PyVirtualDataSlice {
415416
self.0
416417
.lock()
417418
.unwrap()
418-
.render_to_columns_json()
419+
.render_to_columns_json(RowPathStyle::Sidecar)
419420
.map_err(|e| PyValueError::new_err(e.to_string()))
420421
}
421422

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,9 @@ convert_type(const std::string& src) {
185185
if (src == "timestamp") {
186186
return DTYPE_TIME;
187187
}
188+
if (src == "time32" || src == "time64" || src == "time32[s]" ) {
189+
return DTYPE_UINT32;
190+
}
188191
if (src == "date32" || src == "date64") {
189192
return DTYPE_DATE;
190193
}
@@ -776,6 +779,17 @@ copy_array(
776779
dest->set_valid(i, false);
777780
}
778781
} break;
782+
case arrow::Time32Type::type_id: {
783+
auto scol = std::static_pointer_cast<arrow::Time32Array>(src);
784+
std::memcpy(
785+
dest->get_nth<std::uint64_t>(offset),
786+
(void*)scol->raw_values(),
787+
len * 8
788+
);
789+
} break;
790+
// case arrow::Type {
791+
792+
// } break;
779793
default: {
780794
std::stringstream ss;
781795
std::string arrow_type = src->type()->ToString();

tools/test/playwright.config.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,8 @@ const DEVICE_OPTIONS = {
8585
// 2× perf hit is under budget for the test suite.
8686
"--use-gl=swiftshader",
8787
"--use-angle=swiftshader",
88+
"--force-color-profile=srgb",
89+
"--disable-lcd-text",
8890
],
8991
},
9092
},

0 commit comments

Comments
 (0)