Skip to content

Commit 466bf3f

Browse files
mcheshkovmcheshkov
committed
perf(cubesql): Improve LogicalPlanLanguage parsing
* Add separate cheaper error type, without backtrace and input part * Avoid run-time formatting to add : * Avoid calls to replace when strip_prefix + to_string is enough
1 parent fa6d524 commit 466bf3f

File tree

2 files changed

+105
-74
lines changed

2 files changed

+105
-74
lines changed

‎rust/cubesql/cubesql/src/compile/rewrite/language.rs‎

Lines changed: 102 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,34 @@
1+
use std::{
2+
num::{ParseFloatError, ParseIntError},
3+
str::ParseBoolError,
4+
};
5+
6+
// `from_op` is generated as if-else chain from op.parse()
7+
// Because of this alot of instances of FromStr::Err wil lbe constructed just as a "take other branch" marker
8+
// This type should be very cheap to construct new instances, at least while we use FromStr chains
9+
// Don't store any input in here, FromStr is not designed for this, and it requires allocation
10+
// Instead rely on egg::FromOpError for now, it will return allocated `op`, but only once in the end
11+
// TODO make parser dispatch stricter, and return both input and LanguageParseError from `from_op`
12+
#[derive(thiserror::Error, Debug)]
13+
pub enum LanguageParseError {
14+
#[error("Should start with '{0}'")]
15+
ShouldStartWith(&'static str),
16+
#[error("Can't be matched against {0}")]
17+
ShouldMatch(&'static str),
18+
#[error("Should contain a valid type")]
19+
InvalidType,
20+
#[error("Should contains a valid scalar type")]
21+
InvalidScalarType,
22+
#[error("Can't parse boolean scalar value with error: {0}")]
23+
InvalidBoolValue(#[source] ParseBoolError),
24+
#[error("Can't parse i64 scalar value with error: {0}")]
25+
InvalidIntValue(#[source] ParseIntError),
26+
#[error("Can't parse f64 scalar value with error: {0}")]
27+
InvalidFloatValue(#[source] ParseFloatError),
28+
#[error("Conversion from string is not supported")]
29+
NotSupported,
30+
}
31+
132
#[macro_export]
233
macro_rules! plan_to_language {
334
($(#[$meta:meta])* $vis:vis enum $name:ident $variants:tt) => {
@@ -13,13 +44,13 @@ macro_rules! variant_field_struct {
1344
pub struct [<$variant $var_field:camel>](String);
1445

1546
impl FromStr for [<$variant $var_field:camel>] {
16-
type Err = CubeError;
47+
type Err = $crate::compile::rewrite::language::LanguageParseError;
1748
fn from_str(s: &str) -> Result<Self, Self::Err> {
18-
let prefix = format!("{}:", std::stringify!([<$variant $var_field:camel>]));
19-
if s.starts_with(&prefix) {
20-
return Ok([<$variant $var_field:camel>](s.replace(&prefix, "")));
49+
const PREFIX: &'static str = concat!(std::stringify!([<$variant $var_field:camel>]), ":");
50+
if let Some(suffix) = s.strip_prefix(PREFIX) {
51+
return Ok([<$variant $var_field:camel>](suffix.to_string()));
2152
}
22-
Err(CubeError::internal(format!("Can't convert {}. Should start with '{}'", s, prefix)))
53+
Err(Self::Err::ShouldStartWith(PREFIX))
2354
}
2455
}
2556

@@ -37,13 +68,13 @@ macro_rules! variant_field_struct {
3768
pub struct [<$variant $var_field:camel>](usize);
3869

3970
impl FromStr for [<$variant $var_field:camel>] {
40-
type Err = CubeError;
71+
type Err = $crate::compile::rewrite::language::LanguageParseError;
4172
fn from_str(s: &str) -> Result<Self, Self::Err> {
42-
let prefix = format!("{}:", std::stringify!([<$variant $var_field:camel>]));
43-
if s.starts_with(&prefix) {
44-
return Ok([<$variant $var_field:camel>](s.replace(&prefix, "").parse().unwrap()));
73+
const PREFIX: &'static str = concat!(std::stringify!([<$variant $var_field:camel>]), ":");
74+
if let Some(suffix) = s.strip_prefix(PREFIX) {
75+
return Ok([<$variant $var_field:camel>](suffix.parse().unwrap()));
4576
}
46-
Err(CubeError::internal(format!("Can't convert {}. Should start with '{}'", s, prefix)))
77+
Err(Self::Err::ShouldStartWith(PREFIX))
4778
}
4879
}
4980

@@ -61,13 +92,13 @@ macro_rules! variant_field_struct {
6192
pub struct [<$variant $var_field:camel>](bool);
6293

6394
impl FromStr for [<$variant $var_field:camel>] {
64-
type Err = CubeError;
95+
type Err = $crate::compile::rewrite::language::LanguageParseError;
6596
fn from_str(s: &str) -> Result<Self, Self::Err> {
66-
let prefix = format!("{}:", std::stringify!([<$variant $var_field:camel>]));
67-
if s.starts_with(&prefix) {
68-
return Ok([<$variant $var_field:camel>](s.replace(&prefix, "").parse().unwrap()));
97+
const PREFIX: &'static str = concat!(std::stringify!([<$variant $var_field:camel>]), ":");
98+
if let Some(suffix) = s.strip_prefix(PREFIX) {
99+
return Ok([<$variant $var_field:camel>](suffix.parse().unwrap()));
69100
}
70-
Err(CubeError::internal(format!("Can't convert {}. Should start with '{}'", s, prefix)))
101+
Err(Self::Err::ShouldStartWith(PREFIX))
71102
}
72103
}
73104

@@ -85,18 +116,17 @@ macro_rules! variant_field_struct {
85116
pub struct [<$variant $var_field:camel>](Option<usize>);
86117

87118
impl FromStr for [<$variant $var_field:camel>] {
88-
type Err = CubeError;
119+
type Err = $crate::compile::rewrite::language::LanguageParseError;
89120
fn from_str(s: &str) -> Result<Self, Self::Err> {
90-
let prefix = format!("{}:", std::stringify!([<$variant $var_field:camel>]));
91-
if s.starts_with(&prefix) {
92-
let replaced = s.replace(&prefix, "");
93-
if &replaced == "None" {
121+
const PREFIX: &'static str = concat!(std::stringify!([<$variant $var_field:camel>]), ":");
122+
if let Some(suffix) = s.strip_prefix(PREFIX) {
123+
if suffix == "None" {
94124
return Ok([<$variant $var_field:camel>](None));
95125
} else {
96-
return Ok([<$variant $var_field:camel>](Some(s.replace(&prefix, "").parse().unwrap())));
126+
return Ok([<$variant $var_field:camel>](Some(suffix.parse().unwrap())));
97127
}
98128
}
99-
Err(CubeError::internal(format!("Can't convert {}. Should start with '{}'", s, prefix)))
129+
Err(Self::Err::ShouldStartWith(PREFIX))
100130
}
101131
}
102132

@@ -114,18 +144,17 @@ macro_rules! variant_field_struct {
114144
pub struct [<$variant $var_field:camel>](Option<Vec<String>>);
115145

116146
impl FromStr for [<$variant $var_field:camel>] {
117-
type Err = CubeError;
147+
type Err = $crate::compile::rewrite::language::LanguageParseError;
118148
fn from_str(s: &str) -> Result<Self, Self::Err> {
119-
let prefix = format!("{}:", std::stringify!([<$variant $var_field:camel>]));
120-
if s.starts_with(&prefix) {
121-
let replaced = s.replace(&prefix, "");
122-
if &replaced == "None" {
149+
const PREFIX: &'static str = concat!(std::stringify!([<$variant $var_field:camel>]), ":");
150+
if let Some(suffix) = s.strip_prefix(PREFIX) {
151+
if suffix == "None" {
123152
return Ok([<$variant $var_field:camel>](None));
124153
} else {
125-
return Ok([<$variant $var_field:camel>](Some(s.split(',').map(|s| s.to_string()).collect::<Vec<_>>())));
154+
return Ok([<$variant $var_field:camel>](Some(suffix.split(',').map(|s| s.to_string()).collect::<Vec<_>>())));
126155
}
127156
}
128-
Err(CubeError::internal(format!("Can't convert {}. Should start with '{}'", s, prefix)))
157+
Err(Self::Err::ShouldStartWith(PREFIX))
129158
}
130159
}
131160

@@ -143,18 +172,17 @@ macro_rules! variant_field_struct {
143172
pub struct [<$variant $var_field:camel>](Option<String>);
144173

145174
impl FromStr for [<$variant $var_field:camel>] {
146-
type Err = CubeError;
175+
type Err = $crate::compile::rewrite::language::LanguageParseError;
147176
fn from_str(s: &str) -> Result<Self, Self::Err> {
148-
let prefix = format!("{}:", std::stringify!([<$variant $var_field:camel>]));
149-
if s.starts_with(&prefix) {
150-
let replaced = s.replace(&prefix, "");
151-
if &replaced == "None" {
177+
const PREFIX: &'static str = concat!(std::stringify!([<$variant $var_field:camel>]), ":");
178+
if let Some(suffix) = s.strip_prefix(PREFIX) {
179+
if suffix == "None" {
152180
return Ok([<$variant $var_field:camel>](None));
153181
} else {
154-
return Ok([<$variant $var_field:camel>](Some(s.to_string())));
182+
return Ok([<$variant $var_field:camel>](Some(suffix.to_string())));
155183
}
156184
}
157-
Err(CubeError::internal("Conversion from string is not supported".to_string()))
185+
Err(Self::Err::NotSupported)
158186
}
159187
}
160188

@@ -172,9 +200,9 @@ macro_rules! variant_field_struct {
172200
pub struct [<$variant $var_field:camel>](Column);
173201

174202
impl FromStr for [<$variant $var_field:camel>] {
175-
type Err = CubeError;
203+
type Err = $crate::compile::rewrite::language::LanguageParseError;
176204
fn from_str(_s: &str) -> Result<Self, Self::Err> {
177-
Err(CubeError::internal("Conversion from string is not supported".to_string()))
205+
Err(Self::Err::NotSupported)
178206
}
179207
}
180208

@@ -192,9 +220,9 @@ macro_rules! variant_field_struct {
192220
pub struct [<$variant $var_field:camel>](Vec<Column>);
193221

194222
impl FromStr for [<$variant $var_field:camel>] {
195-
type Err = CubeError;
223+
type Err = $crate::compile::rewrite::language::LanguageParseError;
196224
fn from_str(_s: &str) -> Result<Self, Self::Err> {
197-
Err(CubeError::internal("Conversion from string is not supported".to_string()))
225+
Err(Self::Err::NotSupported)
198226
}
199227
}
200228

@@ -212,9 +240,9 @@ macro_rules! variant_field_struct {
212240
pub struct [<$variant $var_field:camel>](Vec<Column>);
213241

214242
impl FromStr for [<$variant $var_field:camel>] {
215-
type Err = CubeError;
243+
type Err = $crate::compile::rewrite::language::LanguageParseError;
216244
fn from_str(_s: &str) -> Result<Self, Self::Err> {
217-
Err(CubeError::internal("Conversion from string is not supported".to_string()))
245+
Err(Self::Err::NotSupported)
218246
}
219247
}
220248

@@ -232,9 +260,9 @@ macro_rules! variant_field_struct {
232260
pub struct [<$variant $var_field:camel>](String);
233261

234262
impl FromStr for [<$variant $var_field:camel>] {
235-
type Err = CubeError;
263+
type Err = $crate::compile::rewrite::language::LanguageParseError;
236264
fn from_str(_s: &str) -> Result<Self, Self::Err> {
237-
Err(CubeError::internal("Conversion from string is not supported".to_string()))
265+
Err(Self::Err::NotSupported)
238266
}
239267
}
240268

@@ -252,9 +280,9 @@ macro_rules! variant_field_struct {
252280
pub struct [<$variant $var_field:camel>](String);
253281

254282
impl FromStr for [<$variant $var_field:camel>] {
255-
type Err = CubeError;
283+
type Err = $crate::compile::rewrite::language::LanguageParseError;
256284
fn from_str(_s: &str) -> Result<Self, Self::Err> {
257-
Err(CubeError::internal("Conversion from string is not supported".to_string()))
285+
Err(Self::Err::NotSupported)
258286
}
259287
}
260288

@@ -272,13 +300,13 @@ macro_rules! variant_field_struct {
272300
pub struct [<$variant $var_field:camel>](String);
273301

274302
impl FromStr for [<$variant $var_field:camel>] {
275-
type Err = CubeError;
303+
type Err = $crate::compile::rewrite::language::LanguageParseError;
276304
fn from_str(s: &str) -> Result<Self, Self::Err> {
277-
let prefix = format!("{}:", std::stringify!([<$variant $var_field:camel>]));
278-
if s.starts_with(&prefix) {
279-
return Ok([<$variant $var_field:camel>](s.replace(&prefix, "")));
305+
const PREFIX: &'static str = concat!(std::stringify!([<$variant $var_field:camel>]), ":");
306+
if let Some(suffix) = s.strip_prefix(PREFIX) {
307+
return Ok([<$variant $var_field:camel>](suffix.to_string()));
280308
}
281-
Err(CubeError::internal(format!("Can't convert {}. Should start with '{}'", s, prefix)))
309+
Err(Self::Err::ShouldStartWith(PREFIX))
282310
}
283311
}
284312

@@ -484,14 +512,16 @@ macro_rules! variant_field_struct {
484512
pub struct [<$variant $var_field:camel>]($var_field_type);
485513

486514
impl FromStr for [<$variant $var_field:camel>] {
487-
type Err = CubeError;
515+
type Err = $crate::compile::rewrite::language::LanguageParseError;
488516
fn from_str(s: &str) -> Result<Self, Self::Err> {
489-
let prefix = format!("{}:", std::stringify!([<$variant $var_field:camel>]));
490-
let name = s.strip_prefix(&prefix).ok_or(CubeError::internal(format!("Can't convert {}. Should start with '{}'", s, prefix)))?;
517+
const PREFIX: &'static str = concat!(std::stringify!([<$variant $var_field:camel>]), ":");
518+
let name = s
519+
.strip_prefix(PREFIX)
520+
.ok_or(Self::Err::ShouldStartWith(PREFIX))?;
491521

492522
match name {
493523
$($name => Ok([<$variant $var_field:camel>]($variant_type)),)*
494-
x => Err(CubeError::internal(format!("{} can't be matched against {}", x, std::stringify!($var_field_type))))
524+
_ => Err(Self::Err::ShouldMatch(std::stringify!($var_field_type)))
495525
}
496526
}
497527
}
@@ -557,10 +587,12 @@ macro_rules! variant_field_struct {
557587
pub struct [<$variant $var_field:camel>](DataType);
558588

559589
impl FromStr for [<$variant $var_field:camel>] {
560-
type Err = CubeError;
590+
type Err = $crate::compile::rewrite::language::LanguageParseError;
561591
fn from_str(s: &str) -> Result<Self, Self::Err> {
562-
let prefix = format!("{}:", std::stringify!([<$variant $var_field:camel>]));
563-
let typed_str = s.strip_prefix(&prefix).ok_or(CubeError::internal(format!("Can't convert {}. Should start with '{}'", s, prefix)))?;
592+
const PREFIX: &'static str = concat!(std::stringify!([<$variant $var_field:camel>]), ":");
593+
let typed_str = s
594+
.strip_prefix(PREFIX)
595+
.ok_or(Self::Err::ShouldStartWith(PREFIX))?;
564596

565597
match typed_str {
566598
"Float32" => Ok([<$variant $var_field:camel>](DataType::Float32)),
@@ -571,7 +603,7 @@ macro_rules! variant_field_struct {
571603
"Utf8" => Ok([<$variant $var_field:camel>](DataType::Utf8)),
572604
"Date32" => Ok([<$variant $var_field:camel>](DataType::Date32)),
573605
"Date64" => Ok([<$variant $var_field:camel>](DataType::Date64)),
574-
_ => Err(CubeError::internal(format!("Can't convert {}. Should contain a valid type, actual: {}", s, typed_str))),
606+
_ => Err(Self::Err::InvalidType),
575607
}
576608
}
577609
}
@@ -590,24 +622,26 @@ macro_rules! variant_field_struct {
590622
pub struct [<$variant $var_field:camel>](ScalarValue);
591623

592624
impl FromStr for [<$variant $var_field:camel>] {
593-
type Err = CubeError;
625+
type Err = $crate::compile::rewrite::language::LanguageParseError;
594626
fn from_str(s: &str) -> Result<Self, Self::Err> {
595-
let prefix = format!("{}:", std::stringify!([<$variant $var_field:camel>]));
596-
let typed_str = s.strip_prefix(&prefix).ok_or(CubeError::internal(format!("Can't convert {}. Should start with '{}'", s, prefix)))?;
627+
const PREFIX: &'static str = concat!(std::stringify!([<$variant $var_field:camel>]), ":");
628+
let typed_str = s
629+
.strip_prefix(PREFIX)
630+
.ok_or(Self::Err::ShouldStartWith(PREFIX))?;
597631

598632
if let Some(value) = typed_str.strip_prefix("s:") {
599633
Ok([<$variant $var_field:camel>](ScalarValue::Utf8(Some(value.to_string()))))
600634
} else if let Some(value) = typed_str.strip_prefix("b:") {
601-
let n: bool = value.parse().map_err(|err| CubeError::internal(format!("Can't parse boolean scalar value from '{}' with error: {}", typed_str, err)))?;
635+
let n: bool = value.parse().map_err(|err| Self::Err::InvalidBoolValue(err))?;
602636
Ok([<$variant $var_field:camel>](ScalarValue::Boolean(Some(n))))
603637
} else if let Some(value) = typed_str.strip_prefix("i:") {
604-
let n: i64 = value.parse().map_err(|err| CubeError::internal(format!("Can't parse i64 scalar value from '{}' with error: {}", typed_str, err)))?;
638+
let n: i64 = value.parse().map_err(|err| Self::Err::InvalidIntValue(err))?;
605639
Ok([<$variant $var_field:camel>](ScalarValue::Int64(Some(n))))
606640
} else if let Some(value) = typed_str.strip_prefix("f:") {
607-
let n: f64 = value.parse().map_err(|err| CubeError::internal(format!("Can't parse f64 scalar value from '{}' with error: {}", typed_str, err)))?;
641+
let n: f64 = value.parse().map_err(|err| Self::Err::InvalidFloatValue(err))?;
608642
Ok([<$variant $var_field:camel>](ScalarValue::Float64(Some(n))))
609643
} else {
610-
Err(CubeError::internal(format!("Can't convert {}. Should contains type type, actual: {}", s, typed_str)))
644+
Err(Self::Err::InvalidScalarType)
611645
}
612646
}
613647
}
@@ -657,9 +691,9 @@ macro_rules! variant_field_struct {
657691
pub struct [<$variant $var_field:camel>]($var_field_type);
658692

659693
impl FromStr for [<$variant $var_field:camel>] {
660-
type Err = CubeError;
694+
type Err = $crate::compile::rewrite::language::LanguageParseError;
661695
fn from_str(_s: &str) -> Result<Self, Self::Err> {
662-
Err(CubeError::internal("Conversion from string is not supported".to_string()))
696+
Err(Self::Err::NotSupported)
663697
}
664698
}
665699

‎rust/cubesql/cubesql/src/compile/rewrite/mod.rs‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,9 @@ pub mod rewriter;
66
pub mod rules;
77

88
use self::analysis::{LogicalPlanData, MemberNameToExpr};
9-
use crate::{
10-
compile::rewrite::{
11-
analysis::{LogicalPlanAnalysis, OriginalExpr},
12-
rewriter::{CubeEGraph, CubeRewrite},
13-
},
14-
CubeError,
9+
use crate::compile::rewrite::{
10+
analysis::{LogicalPlanAnalysis, OriginalExpr},
11+
rewriter::{CubeEGraph, CubeRewrite},
1512
};
1613
use analysis::MemberNamesToExpr;
1714
use datafusion::{

0 commit comments

Comments
 (0)