Skip to content

Commit 2b313c7

Browse files
BohuTANGBohuTANG
authored
Merge pull request #10612 from andylokandy/span
feat(planner): pretty display more errors with span
2 parents bc898b3 + cbcc59b commit 2b313c7

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

56 files changed

+874
-340
lines changed

‎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.

‎src/common/exception/src/exception.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,6 @@ where E: Display + Send + Sync + 'static
298298

299299
impl Clone for ErrorCode {
300300
fn clone(&self) -> Self {
301-
ErrorCode::create(self.code(), self.message(), None, self.backtrace())
301+
ErrorCode::create(self.code(), self.message(), None, self.backtrace()).set_span(self.span())
302302
}
303303
}

‎src/common/exception/src/exception_flight.rs‎

Lines changed: 29 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -12,78 +12,58 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
use std::mem::size_of;
1615
use std::sync::Arc;
1716

1817
use common_arrow::arrow_format::flight::data::FlightData;
1918

2019
use crate::exception::ErrorCodeBacktrace;
2120
use crate::ErrorCode;
2221
use crate::Result;
22+
use crate::SerializedError;
2323

2424
impl From<ErrorCode> for FlightData {
2525
fn from(error: ErrorCode) -> Self {
26-
let error_message = error.message();
27-
let error_backtrace = error.backtrace_str();
28-
29-
let message = error_message.as_bytes();
30-
let backtrace = error_backtrace.as_bytes();
31-
32-
let mut data_body = Vec::with_capacity(16 + message.len() + backtrace.len());
33-
34-
data_body.extend((message.len() as u64).to_be_bytes());
35-
data_body.extend_from_slice(message);
36-
data_body.extend((backtrace.len() as u64).to_be_bytes());
37-
data_body.extend_from_slice(backtrace);
26+
let serialized_error = serde_json::to_vec::<SerializedError>(&SerializedError {
27+
code: error.code(),
28+
message: error.message(),
29+
span: error.span(),
30+
backtrace: error.backtrace_str(),
31+
})
32+
.unwrap();
3833

3934
FlightData {
40-
data_body,
35+
data_body: serialized_error,
4136
app_metadata: vec![0x02],
4237
data_header: error.code().to_be_bytes().to_vec(),
4338
flight_descriptor: None,
4439
}
4540
}
4641
}
4742

48-
fn read_be_u64(bytes: &[u8]) -> u64 {
49-
u64::from_be_bytes(bytes[0..size_of::<u64>()].try_into().unwrap())
50-
}
51-
5243
impl TryFrom<FlightData> for ErrorCode {
5344
type Error = ErrorCode;
5445

5546
fn try_from(flight_data: FlightData) -> Result<Self> {
56-
match flight_data.data_header.try_into() {
57-
Err(_) => Err(ErrorCode::BadBytes("Cannot parse inf usize.")),
58-
Ok(slice) => {
59-
let code = u16::from_be_bytes(slice);
60-
let data_body = flight_data.data_body;
61-
62-
let mut data_offset = 0;
63-
let message_len = read_be_u64(&data_body) as usize;
64-
data_offset += size_of::<u64>();
65-
let message = &data_body[data_offset..data_offset + message_len];
66-
data_offset += message_len;
67-
data_offset += size_of::<u64>();
68-
let backtrace = &data_body[data_offset..];
69-
70-
match backtrace.is_empty() {
71-
true => Ok(ErrorCode::create(
72-
code,
73-
String::from_utf8(message.to_vec())?,
74-
None,
75-
None,
76-
)),
77-
false => Ok(ErrorCode::create(
78-
code,
79-
String::from_utf8(message.to_vec())?,
80-
None,
81-
Some(ErrorCodeBacktrace::Serialized(Arc::new(String::from_utf8(
82-
backtrace.to_vec(),
83-
)?))),
84-
)),
85-
}
86-
}
47+
match serde_json::from_slice::<SerializedError>(&flight_data.data_body) {
48+
Err(error) => Ok(ErrorCode::from(error)),
49+
Ok(serialized_error) => match serialized_error.backtrace.len() {
50+
0 => Ok(ErrorCode::create(
51+
serialized_error.code,
52+
serialized_error.message,
53+
None,
54+
None,
55+
)
56+
.set_span(serialized_error.span)),
57+
_ => Ok(ErrorCode::create(
58+
serialized_error.code,
59+
serialized_error.message,
60+
None,
61+
Some(ErrorCodeBacktrace::Serialized(Arc::new(
62+
serialized_error.backtrace,
63+
))),
64+
)
65+
.set_span(serialized_error.span)),
66+
},
8767
}
8868
}
8969
}

‎src/common/exception/src/exception_into.rs‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ use common_meta_types::MetaError;
2525

2626
use crate::exception::ErrorCodeBacktrace;
2727
use crate::ErrorCode;
28+
use crate::Span;
2829

2930
#[derive(thiserror::Error)]
3031
enum OtherErrors {
@@ -210,9 +211,10 @@ impl From<prost::EncodeError> for ErrorCode {
210211
// === ser/de to/from tonic::Status ===
211212
#[derive(thiserror::Error, serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq, Eq)]
212213
pub struct SerializedError {
213-
code: u16,
214-
message: String,
215-
backtrace: String,
214+
pub code: u16,
215+
pub message: String,
216+
pub span: Span,
217+
pub backtrace: String,
216218
}
217219

218220
impl Display for SerializedError {
@@ -226,6 +228,7 @@ impl From<ErrorCode> for SerializedError {
226228
SerializedError {
227229
code: e.code(),
228230
message: e.message(),
231+
span: e.span(),
229232
backtrace: e.backtrace_str(),
230233
}
231234
}
@@ -239,6 +242,7 @@ impl From<SerializedError> for ErrorCode {
239242
None,
240243
Some(ErrorCodeBacktrace::Serialized(Arc::new(se.backtrace))),
241244
)
245+
.set_span(se.span)
242246
}
243247
}
244248

@@ -262,15 +266,17 @@ impl From<tonic::Status> for ErrorCode {
262266
serialized_error.message,
263267
None,
264268
None,
265-
),
269+
)
270+
.set_span(serialized_error.span),
266271
_ => ErrorCode::create(
267272
serialized_error.code,
268273
serialized_error.message,
269274
None,
270275
Some(ErrorCodeBacktrace::Serialized(Arc::new(
271276
serialized_error.backtrace,
272277
))),
273-
),
278+
)
279+
.set_span(serialized_error.span),
274280
},
275281
}
276282
}
@@ -299,17 +305,18 @@ impl From<MetaStorageError> for ErrorCode {
299305

300306
impl From<ErrorCode> for tonic::Status {
301307
fn from(err: ErrorCode) -> Self {
302-
let rst_json = serde_json::to_vec::<SerializedError>(&SerializedError {
308+
let error_json = serde_json::to_vec::<SerializedError>(&SerializedError {
303309
code: err.code(),
304310
message: err.message(),
311+
span: err.span(),
305312
backtrace: {
306313
let mut str = err.backtrace_str();
307314
str.truncate(2 * 1024);
308315
str
309316
},
310317
});
311318

312-
match rst_json {
319+
match error_json {
313320
Ok(serialized_error_json) => {
314321
// Code::Internal will be used by h2, if something goes wrong internally.
315322
// To distinguish from that, we use Code::Unknown here

‎src/common/exception/tests/it/exception_flight.rs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,13 @@ fn test_serialize() -> Result<()> {
2727
String::from("test_message"),
2828
None,
2929
Some(ErrorCodeBacktrace::Origin(Arc::new(Backtrace::capture()))),
30-
);
30+
)
31+
.set_span(Some((0..1).into()));
3132
let backtrace_str = error_code.backtrace_str();
3233
let error_code = ErrorCode::try_from(FlightData::from(error_code))?;
3334
assert_eq!(1, error_code.code());
3435
assert_eq!(String::from("test_message"), error_code.message());
3536
assert_eq!(backtrace_str, error_code.backtrace_str());
37+
assert_eq!(error_code.span(), Some((0..1).into()));
3638
Ok(())
3739
}

‎src/query/ast/src/ast/query.rs‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ pub enum Indirection {
154154
// Field name
155155
Identifier(Identifier),
156156
// Wildcard star
157-
Star,
157+
Star(Span),
158158
}
159159

160160
/// Time Travel specification
@@ -239,6 +239,16 @@ pub enum JoinCondition {
239239
None,
240240
}
241241

242+
impl SetExpr {
243+
pub fn span(&self) -> Span {
244+
match self {
245+
SetExpr::Select(stmt) => stmt.span,
246+
SetExpr::Query(query) => query.span,
247+
SetExpr::SetOperation(op) => op.span,
248+
}
249+
}
250+
}
251+
242252
impl Display for OrderByExpr {
243253
fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
244254
write!(f, "{}", self.expr)?;
@@ -403,7 +413,7 @@ impl Display for Indirection {
403413
Indirection::Identifier(ident) => {
404414
write!(f, "{ident}")?;
405415
}
406-
Indirection::Star => {
416+
Indirection::Star(_) => {
407417
write!(f, "*")?;
408418
}
409419
}

‎src/query/ast/src/parser/expr.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1050,11 +1050,11 @@ pub fn expr_element(i: Input) -> IResult<WithSpan<ExprElement>> {
10501050

10511051
let date_expr = map(
10521052
rule! {
1053-
DATE ~ #literal_string
1053+
DATE ~ #consumed(literal_string)
10541054
},
1055-
|(_, date)| ExprElement::Cast {
1055+
|(_, (span, date))| ExprElement::Cast {
10561056
expr: Box::new(Expr::Literal {
1057-
span: None,
1057+
span: transform_span(span.0),
10581058
lit: Literal::String(date),
10591059
}),
10601060
target_type: TypeName::Date,
@@ -1063,11 +1063,11 @@ pub fn expr_element(i: Input) -> IResult<WithSpan<ExprElement>> {
10631063

10641064
let timestamp_expr = map(
10651065
rule! {
1066-
TIMESTAMP ~ #literal_string
1066+
TIMESTAMP ~ #consumed(literal_string)
10671067
},
1068-
|(_, date)| ExprElement::Cast {
1068+
|(_, (span, date))| ExprElement::Cast {
10691069
expr: Box::new(Expr::Literal {
1070-
span: None,
1070+
span: transform_span(span.0),
10711071
lit: Literal::String(date),
10721072
}),
10731073
target_type: TypeName::Timestamp,

‎src/query/ast/src/parser/query.rs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,23 +110,26 @@ pub fn select_target(i: Input) -> IResult<SelectTarget> {
110110
rule! {
111111
( #ident ~ "." ~ ( #ident ~ "." )? )? ~ "*" ~ ( EXCLUDE ~ #exclude_col )?
112112
},
113-
|(res, _, opt_exclude)| {
113+
|(res, star, opt_exclude)| {
114114
let exclude = opt_exclude.map(|(_, exclude)| exclude);
115115
match res {
116116
Some((fst, _, Some((snd, _)))) => SelectTarget::QualifiedName {
117117
qualified: vec![
118118
Indirection::Identifier(fst),
119119
Indirection::Identifier(snd),
120-
Indirection::Star,
120+
Indirection::Star(Some(star.span)),
121121
],
122122
exclude,
123123
},
124124
Some((fst, _, None)) => SelectTarget::QualifiedName {
125-
qualified: vec![Indirection::Identifier(fst), Indirection::Star],
125+
qualified: vec![
126+
Indirection::Identifier(fst),
127+
Indirection::Star(Some(star.span)),
128+
],
126129
exclude,
127130
},
128131
None => SelectTarget::QualifiedName {
129-
qualified: vec![Indirection::Star],
132+
qualified: vec![Indirection::Star(Some(star.span))],
130133
exclude,
131134
},
132135
}

‎src/query/ast/src/visitors/walk.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ pub fn walk_select_target<'a, V: Visitor<'a>>(visitor: &mut V, target: &'a Selec
202202
Indirection::Identifier(ident) => {
203203
visitor.visit_identifier(ident);
204204
}
205-
Indirection::Star => {}
205+
Indirection::Star(_) => {}
206206
}
207207
}
208208
if let Some(cols) = exclude {

‎src/query/ast/src/visitors/walk_mut.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ pub fn walk_select_target_mut<V: VisitorMut>(visitor: &mut V, target: &mut Selec
202202
Indirection::Identifier(ident) => {
203203
visitor.visit_identifier(ident);
204204
}
205-
Indirection::Star => {}
205+
Indirection::Star(_) => {}
206206
}
207207
}
208208
if let Some(cols) = exclude {

0 commit comments

Comments
 (0)