Skip to content

Commit 4e26c26

Browse files
authored
perf(cubeorchestrator): Drop DBResponseValue wrapper (-15% transform) (cube-js#10844)
`DBResponseValue` had three variants but only `Primitive` was ever constructed in production. `DateTime` and `Object` existed only in tests (the latter even carried `// TODO: Is this variant still used?`). The wrapper added per-cell discriminant + alignment overhead and a dead match arm in the `transform_value` hot loop. `TransformedData::transform` columnar path: | bench | before | after | Δ | |----------------------|-----------|-----------|--------| | columnar/c16_r10000 | 2.62 ms | 2.26 ms | −13.7% | | columnar/c16_r50000 | 17.13 ms | 13.89 ms | −18.9% | | columnar/c16_r100000 | 46.05 ms | 38.45 ms | −16.5% | | columnar/c32_r10000 | 5.89 ms | 4.81 ms | −18.3% | | columnar/c32_r50000 | 30.24 ms | 26.84 ms | −11.2% | | columnar/c64_r100000 | 121.24 ms | 108.39 ms | −10.6% | Real-shape scenarios (c16×100k): | scenario | before | after | Δ | |--------------------------------------------|-----------|-----------|--------| | columnar / no_time_dim | 42.13 ms | 32.12 ms | −23.8% | | columnar / one_time_dim_day | 87.41 ms | 74.33 ms | −15.0% | | columnar / one_time_dim_custom_granularity | 93.79 ms | 79.62 ms | −15.1% | | columnar / two_time_dims | 132.90 ms | 115.98 ms | −12.7% | | vanilla / one_time_dim_day | 161.03 ms | 146.46 ms | −9.1% | | vanilla / two_time_dims | 222.80 ms | 206.43 ms | −7.3% |
1 parent 9e05533 commit 4e26c26

4 files changed

Lines changed: 23 additions & 128 deletions

File tree

packages/cubejs-api-gateway/src/types/responses.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,6 @@ export type DBResponsePrimitive =
88
number |
99
string;
1010

11-
export type DBResponseValue =
12-
Date |
13-
DBResponsePrimitive |
14-
{ value: DBResponsePrimitive };
15-
1611
export type TransformDataResponse = {
1712
members: string[],
1813
dataset: DBResponsePrimitive[][]

packages/cubejs-backend-native/src/orchestrator.rs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ use crate::node_obj_deserializer::JsValueDeserializer;
22
use crate::transport::MapCubeErrExt;
33
use cubeorchestrator::query_message_parser::QueryResult;
44
use cubeorchestrator::query_result_transform::{
5-
DBResponsePrimitive, DBResponseValue, RequestResultData, RequestResultDataMulti,
6-
TransformedData,
5+
DBResponsePrimitive, RequestResultData, RequestResultDataMulti, TransformedData,
76
};
87
use cubeorchestrator::transport::{JsRawColumnarData, TransformDataRequest};
98
use cubesql::compile::engine::df::scan::{ColumnarValueObject, FieldValue, ValueObject};
@@ -339,7 +338,7 @@ pub fn get_cubestore_result(mut cx: FunctionContext) -> JsResult<JsValue> {
339338
for (key, value) in result.members.iter().zip(row.iter()) {
340339
let js_key = cx.string(key);
341340
let js_value: Handle<'_, JsValue> = match value {
342-
DBResponseValue::Primitive(DBResponsePrimitive::Null) => cx.null().upcast(),
341+
DBResponsePrimitive::Null => cx.null().upcast(),
343342
// For compatibility, we convert all primitives to strings
344343
other => cx.string(other.to_string()).upcast(),
345344
};

rust/cube/cubeorchestrator/src/query_message_parser.rs

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
1-
use crate::{
2-
query_result_transform::{DBResponsePrimitive, DBResponseValue},
3-
transport::JsRawColumnarData,
4-
};
1+
use crate::{query_result_transform::DBResponsePrimitive, transport::JsRawColumnarData};
52
use cubeshared::codegen::{root_as_http_message_with_opts, HttpCommand};
63
use cubeshared::flatbuffers::VerifierOptions;
74
use indexmap::IndexMap;
@@ -35,7 +32,7 @@ impl std::error::Error for ParseError {}
3532
#[derive(Debug, Clone)]
3633
pub struct QueryResult {
3734
pub members: Vec<String>,
38-
pub rows: Vec<Vec<DBResponseValue>>,
35+
pub rows: Vec<Vec<DBResponsePrimitive>>,
3936
pub columns_pos: IndexMap<String, usize>,
4037
}
4138

@@ -96,10 +93,8 @@ impl QueryResult {
9693
let row_obj: Vec<_> = values
9794
.iter()
9895
.map(|val| match val.string_value() {
99-
Some(s) => DBResponseValue::Primitive(DBResponsePrimitive::String(
100-
s.to_owned(),
101-
)),
102-
None => DBResponseValue::Primitive(DBResponsePrimitive::Null),
96+
Some(s) => DBResponsePrimitive::String(s.to_owned()),
97+
None => DBResponsePrimitive::Null,
10398
})
10499
.collect();
105100

@@ -134,14 +129,14 @@ impl QueryResult {
134129
// Transpose column-major input into the row-major shape `QueryResult`
135130
// expects. Rows are pre-allocated, then we drain each column into the
136131
// matching slot to avoid per-cell clones.
137-
let mut rows: Vec<Vec<DBResponseValue>> = (0..row_count)
132+
let mut rows: Vec<Vec<DBResponsePrimitive>> = (0..row_count)
138133
.map(|_| Vec::with_capacity(members.len()))
139134
.collect();
140135

141136
for column in columns.into_iter() {
142137
for (row_idx, value) in column.into_iter().enumerate() {
143138
if let Some(row) = rows.get_mut(row_idx) {
144-
row.push(DBResponseValue::Primitive(value));
139+
row.push(value);
145140
}
146141
}
147142
}

rust/cube/cubeorchestrator/src/query_result_transform.rs

Lines changed: 15 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -38,16 +38,9 @@ pub static GRANULARITY_LEVELS: LazyLock<HashMap<&'static str, u8>> = LazyLock::n
3838
const DEFAULT_LEVEL_FOR_UNKNOWN: u8 = 10;
3939

4040
/// Transform specified `value` with specified `type` to the network protocol type.
41-
pub fn transform_value(value: DBResponseValue, type_: &str) -> DBResponsePrimitive {
41+
pub fn transform_value(value: DBResponsePrimitive, type_: &str) -> DBResponsePrimitive {
4242
match value {
43-
DBResponseValue::DateTime(dt) if type_ == "time" || type_.is_empty() => {
44-
DBResponsePrimitive::String(
45-
dt.with_timezone(&Utc)
46-
.format("%Y-%m-%dT%H:%M:%S%.3f")
47-
.to_string(),
48-
)
49-
}
50-
DBResponseValue::Primitive(DBResponsePrimitive::String(ref s)) if type_ == "time" => {
43+
DBResponsePrimitive::String(ref s) if type_ == "time" => {
5144
let formatted = DateTime::parse_from_rfc3339(s)
5245
.map(|dt| dt.format("%Y-%m-%dT%H:%M:%S%.3f").to_string())
5346
.or_else(|_| {
@@ -88,9 +81,7 @@ pub fn transform_value(value: DBResponseValue, type_: &str) -> DBResponsePrimiti
8881
.unwrap_or_else(|_| s.clone());
8982
DBResponsePrimitive::String(formatted)
9083
}
91-
DBResponseValue::Primitive(p) => p,
92-
DBResponseValue::Object { value } => value,
93-
_ => DBResponsePrimitive::Null,
84+
other => other,
9485
}
9586
}
9687

@@ -396,7 +387,7 @@ pub(crate) fn build_compact_plan<'a>(
396387
/// Convert DB response row to the compact output
397388
pub fn get_compact_row(
398389
plan: &CompactPlan<'_>,
399-
db_row: &[DBResponseValue],
390+
db_row: &[DBResponsePrimitive],
400391
) -> Vec<DBResponsePrimitive> {
401392
let mut row: Vec<DBResponsePrimitive> = Vec::with_capacity(plan.entries.len());
402393

@@ -609,7 +600,7 @@ fn build_columnar_plan<'a>(
609600
/// row-major `cube_store_result.rows` matrix.
610601
fn build_columnar_columns(
611602
plan: &[ColumnarColumnPlan<'_>],
612-
rows: &[Vec<DBResponseValue>],
603+
rows: &[Vec<DBResponsePrimitive>],
613604
) -> Vec<Vec<DBResponsePrimitive>> {
614605
let row_count = rows.len();
615606
let mut columns: Vec<Vec<DBResponsePrimitive>> =
@@ -623,7 +614,7 @@ fn build_columnar_columns(
623614
let cell = row
624615
.get(*index)
625616
.cloned()
626-
.unwrap_or(DBResponseValue::Primitive(DBResponsePrimitive::Null));
617+
.unwrap_or(DBResponsePrimitive::Null);
627618
out.push(transform_value(cell, plan_entry.member_type));
628619
}
629620
}
@@ -644,7 +635,7 @@ pub fn get_vanilla_row(
644635
plan: &VanillaPlan<'_>,
645636
query_type: &QueryType,
646637
query: &NormalizedQuery,
647-
db_row: &[DBResponseValue],
638+
db_row: &[DBResponsePrimitive],
648639
) -> Result<IndexMap<String, DBResponsePrimitive>> {
649640
// +1 to cover the optional tail entry (compareDateRange / blending key).
650641
let mut row = IndexMap::with_capacity(plan.columns.len() + 1);
@@ -995,31 +986,11 @@ impl Display for DBResponsePrimitive {
995986
}
996987
}
997988

998-
#[derive(Debug, Clone, Deserialize)]
999-
pub enum DBResponseValue {
1000-
DateTime(DateTime<Utc>),
1001-
Primitive(DBResponsePrimitive),
1002-
// TODO: Is this variant still used?
1003-
Object { value: DBResponsePrimitive },
1004-
}
1005-
1006-
impl Display for DBResponseValue {
1007-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
1008-
let str = match self {
1009-
DBResponseValue::DateTime(dt) => dt.to_rfc3339(),
1010-
DBResponseValue::Primitive(p) => p.to_string(),
1011-
DBResponseValue::Object { value } => value.to_string(),
1012-
};
1013-
write!(f, "{}", str)
1014-
}
1015-
}
1016-
1017989
#[cfg(test)]
1018990
mod tests {
1019991
use super::*;
1020992
use crate::transport::JsRawColumnarData;
1021993
use anyhow::Result;
1022-
use chrono::{TimeZone, Timelike, Utc};
1023994
use serde_json::from_str;
1024995
use std::{fmt, sync::LazyLock};
1025996

@@ -1973,43 +1944,9 @@ mod tests {
19731944
}
19741945
}
19751946

1976-
#[test]
1977-
fn test_transform_value_datetime_to_time() {
1978-
let dt = Utc
1979-
.with_ymd_and_hms(2024, 1, 1, 12, 30, 15)
1980-
.unwrap()
1981-
.with_nanosecond(123_000_000)
1982-
.unwrap();
1983-
let value = DBResponseValue::DateTime(dt);
1984-
let result = transform_value(value, "time");
1985-
1986-
assert_eq!(
1987-
result,
1988-
DBResponsePrimitive::String("2024-01-01T12:30:15.123".to_string())
1989-
);
1990-
}
1991-
1992-
#[test]
1993-
fn test_transform_value_datetime_empty_type() {
1994-
let dt = Utc
1995-
.with_ymd_and_hms(2024, 1, 1, 12, 30, 15)
1996-
.unwrap()
1997-
.with_nanosecond(123_000_000)
1998-
.unwrap();
1999-
let value = DBResponseValue::DateTime(dt);
2000-
let result = transform_value(value, "");
2001-
2002-
assert_eq!(
2003-
result,
2004-
DBResponsePrimitive::String("2024-01-01T12:30:15.123".to_string())
2005-
);
2006-
}
2007-
20081947
#[test]
20091948
fn test_transform_value_string_to_time_valid_rfc3339() {
2010-
let value = DBResponseValue::Primitive(DBResponsePrimitive::String(
2011-
"2024-01-01T12:30:15.123".to_string(),
2012-
));
1949+
let value = DBResponsePrimitive::String("2024-01-01T12:30:15.123".to_string());
20131950
let result = transform_value(value, "time");
20141951

20151952
assert_eq!(
@@ -2020,9 +1957,7 @@ mod tests {
20201957

20211958
#[test]
20221959
fn test_transform_value_string_wo_t_to_time_valid_rfc3339() {
2023-
let value = DBResponseValue::Primitive(DBResponsePrimitive::String(
2024-
"2024-01-01 12:30:15.123".to_string(),
2025-
));
1960+
let value = DBResponsePrimitive::String("2024-01-01 12:30:15.123".to_string());
20261961
let result = transform_value(value, "time");
20271962

20281963
assert_eq!(
@@ -2033,9 +1968,7 @@ mod tests {
20331968

20341969
#[test]
20351970
fn test_transform_value_string_wo_mssec_to_time_valid_rfc3339() {
2036-
let value = DBResponseValue::Primitive(DBResponsePrimitive::String(
2037-
"2024-01-01 12:30:15".to_string(),
2038-
));
1971+
let value = DBResponsePrimitive::String("2024-01-01 12:30:15".to_string());
20391972
let result = transform_value(value, "time");
20401973

20411974
assert_eq!(
@@ -2046,9 +1979,7 @@ mod tests {
20461979

20471980
#[test]
20481981
fn test_transform_value_string_wo_mssec_w_t_to_time_valid_rfc3339() {
2049-
let value = DBResponseValue::Primitive(DBResponsePrimitive::String(
2050-
"2024-01-01T12:30:15".to_string(),
2051-
));
1982+
let value = DBResponsePrimitive::String("2024-01-01T12:30:15".to_string());
20521983
let result = transform_value(value, "time");
20531984

20541985
assert_eq!(
@@ -2059,9 +1990,7 @@ mod tests {
20591990

20601991
#[test]
20611992
fn test_transform_value_string_with_tz_offset_to_time_valid_rfc3339() {
2062-
let value = DBResponseValue::Primitive(DBResponsePrimitive::String(
2063-
"2024-01-01 12:30:15.123 +00:00".to_string(),
2064-
));
1993+
let value = DBResponsePrimitive::String("2024-01-01 12:30:15.123 +00:00".to_string());
20651994
let result = transform_value(value, "time");
20661995

20671996
assert_eq!(
@@ -2072,9 +2001,7 @@ mod tests {
20722001

20732002
#[test]
20742003
fn test_transform_value_string_with_tz_to_time_valid_rfc3339() {
2075-
let value = DBResponseValue::Primitive(DBResponsePrimitive::String(
2076-
"2024-01-01 12:30:15.123 UTC".to_string(),
2077-
));
2004+
let value = DBResponsePrimitive::String("2024-01-01 12:30:15.123 UTC".to_string());
20782005
let result = transform_value(value, "time");
20792006

20802007
assert_eq!(
@@ -2085,8 +2012,7 @@ mod tests {
20852012

20862013
#[test]
20872014
fn test_transform_value_string_to_time_invalid_rfc3339() {
2088-
let value =
2089-
DBResponseValue::Primitive(DBResponsePrimitive::String("invalid-date".to_string()));
2015+
let value = DBResponsePrimitive::String("invalid-date".to_string());
20902016
let result = transform_value(value, "time");
20912017

20922018
assert_eq!(
@@ -2097,8 +2023,7 @@ mod tests {
20972023

20982024
#[test]
20992025
fn test_transform_value_primitive_string_type_not_time() {
2100-
let value =
2101-
DBResponseValue::Primitive(DBResponsePrimitive::String("some-string".to_string()));
2026+
let value = DBResponsePrimitive::String("some-string".to_string());
21022027
let result = transform_value(value, "other");
21032028

21042029
assert_eq!(
@@ -2107,25 +2032,6 @@ mod tests {
21072032
);
21082033
}
21092034

2110-
#[test]
2111-
fn test_transform_value_object() {
2112-
let obj_value = DBResponsePrimitive::String("object-value".to_string());
2113-
let value = DBResponseValue::Object {
2114-
value: obj_value.clone(),
2115-
};
2116-
let result = transform_value(value, "time");
2117-
2118-
assert_eq!(result, obj_value);
2119-
}
2120-
2121-
#[test]
2122-
fn test_transform_value_fallback_to_null() {
2123-
let value = DBResponseValue::DateTime(Utc::now());
2124-
let result = transform_value(value, "unknown");
2125-
2126-
assert_eq!(result, DBResponsePrimitive::Null);
2127-
}
2128-
21292035
#[test]
21302036
fn test_get_date_range_value_valid_range() -> Result<()> {
21312037
let time_dimensions = vec![QueryTimeDimension {

0 commit comments

Comments
 (0)