Skip to content

Commit 0759822

Browse files
committed
chore: error display
1 parent f9bcc4a commit 0759822

7 files changed

Lines changed: 101 additions & 35 deletions

File tree

src/binder/insert.rs

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
use crate::binder::{lower_case_name, Binder};
15+
use crate::binder::{attach_span_if_absent, lower_case_name, Binder};
1616
use crate::errors::DatabaseError;
1717
use crate::expression::simplify::ConstantCalculator;
1818
use crate::expression::visitor_mut::VisitorMut;
@@ -96,13 +96,25 @@ impl<T: Transaction, A: AsRef<[(&'static str, DataValue)]>> Binder<'_, '_, T, A>
9696
}
9797
// Check if the value length is too long
9898
value.check_len(ty)?;
99+
if value.is_null() && !schema_ref[i].nullable() {
100+
return Err(attach_span_if_absent(
101+
DatabaseError::not_null_column(schema_ref[i].name().to_string()),
102+
expr,
103+
));
104+
}
99105

100106
row.push(value);
101107
}
102108
ScalarExpression::Empty => {
103109
let default_value = schema_ref[i]
104110
.default_value()?
105111
.ok_or(DatabaseError::DefaultNotExist)?;
112+
if default_value.is_null() && !schema_ref[i].nullable() {
113+
return Err(attach_span_if_absent(
114+
DatabaseError::not_null_column(schema_ref[i].name().to_string()),
115+
expr,
116+
));
117+
}
106118
row.push(default_value);
107119
}
108120
_ => return Err(DatabaseError::UnsupportedStmt(expr.to_string())),

src/binder/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ pub(crate) fn sqlparser_span_to_sql_error_span(span: Span) -> Option<SqlErrorSpa
105105
start,
106106
end,
107107
line: span.start.line as usize,
108-
near: None,
108+
highlight: None,
109109
})
110110
}
111111

src/db.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -739,7 +739,10 @@ pub(crate) mod test {
739739
assert_eq!(span.line, 1);
740740
assert!(span.start >= 12);
741741
assert!(span.end > span.start);
742-
assert_eq!(span.near.as_deref(), Some("missing_col"));
742+
assert!(span
743+
.highlight
744+
.as_deref()
745+
.is_some_and(|h| h.contains("missing_col")));
743746
}
744747
other => panic!("unexpected error type: {other:?}"),
745748
}
@@ -768,7 +771,10 @@ pub(crate) mod test {
768771
assert_eq!(span.line, 1);
769772
assert!(span.start >= 8);
770773
assert!(span.end > span.start);
771-
assert_eq!(span.near.as_deref(), Some("missing_fn(id)"));
774+
assert!(span
775+
.highlight
776+
.as_deref()
777+
.is_some_and(|h| h.contains("missing_fn(id)")));
772778
}
773779
other => panic!("unexpected error type: {other:?}"),
774780
}

src/errors.rs

Lines changed: 72 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -26,21 +26,31 @@ pub struct SqlErrorSpan {
2626
pub start: usize,
2727
pub end: usize,
2828
pub line: usize,
29-
pub near: Option<String>,
29+
pub highlight: Option<String>,
3030
}
3131

3232
fn format_sql_error_loc(span: &Option<SqlErrorSpan>) -> String {
3333
span.as_ref()
3434
.map(|s| {
35-
if let Some(near) = &s.near {
36-
format!(" near '{near}' at line {}", s.line)
35+
if let Some(highlight) = &s.highlight {
36+
format!("\n{highlight}")
3737
} else {
3838
format!(" at line {}, range {}..{}", s.line, s.start, s.end)
3939
}
4040
})
4141
.unwrap_or_default()
4242
}
4343

44+
fn format_not_null_message(column: &Option<String>, span: &Option<SqlErrorSpan>) -> String {
45+
match column {
46+
Some(column) => format!(
47+
"column: {column} cannot be null{}",
48+
format_sql_error_loc(span)
49+
),
50+
None => format!("cannot be null{}", format_sql_error_loc(span)),
51+
}
52+
}
53+
4454
#[derive(thiserror::Error, Debug)]
4555
pub enum DatabaseError {
4656
#[error("agg miss: {0}")]
@@ -160,8 +170,11 @@ pub enum DatabaseError {
160170
},
161171
#[error("no transaction begin")]
162172
NoTransactionBegin,
163-
#[error("cannot be null")]
164-
NotNull,
173+
#[error("{msg}", msg = format_not_null_message(column, span))]
174+
NotNull {
175+
column: Option<String>,
176+
span: Option<SqlErrorSpan>,
177+
},
165178
#[error("over flow")]
166179
OverFlow,
167180
#[error("parser bool: {0}")]
@@ -291,6 +304,20 @@ impl DatabaseError {
291304
}
292305
}
293306

307+
pub fn not_null() -> Self {
308+
Self::NotNull {
309+
column: None,
310+
span: None,
311+
}
312+
}
313+
314+
pub fn not_null_column(name: impl Into<String>) -> Self {
315+
Self::NotNull {
316+
column: Some(name.into()),
317+
span: None,
318+
}
319+
}
320+
294321
pub fn with_span(self, span: SqlErrorSpan) -> Self {
295322
match self {
296323
Self::CastFail { from, to, .. } => Self::CastFail {
@@ -318,15 +345,19 @@ impl DatabaseError {
318345
name,
319346
span: Some(span),
320347
},
348+
Self::NotNull { column, .. } => Self::NotNull {
349+
column,
350+
span: Some(span),
351+
},
321352
other => other,
322353
}
323354
}
324355

325356
pub fn with_sql_context(self, sql: &str) -> Self {
326357
let annotate = |span: Option<SqlErrorSpan>| -> Option<SqlErrorSpan> {
327358
span.map(|mut span| {
328-
if span.near.is_none() {
329-
span.near = extract_sql_near(sql, &span);
359+
if span.highlight.is_none() {
360+
span.highlight = build_sql_highlight(sql, &span);
330361
}
331362
span
332363
})
@@ -358,6 +389,10 @@ impl DatabaseError {
358389
name,
359390
span: annotate(span),
360391
},
392+
Self::NotNull { column, span } => Self::NotNull {
393+
column,
394+
span: annotate(span),
395+
},
361396
other => other,
362397
}
363398
}
@@ -369,38 +404,48 @@ impl DatabaseError {
369404
| DatabaseError::ColumnNotFound { span, .. }
370405
| DatabaseError::InvalidTable { span, .. }
371406
| DatabaseError::FunctionNotFound { span, .. }
372-
| DatabaseError::ParametersNotFound { span, .. } => span.as_ref(),
407+
| DatabaseError::ParametersNotFound { span, .. }
408+
| DatabaseError::NotNull { span, .. } => span.as_ref(),
373409
_ => None,
374410
}
375411
}
376412
}
377413

378-
fn extract_sql_near(sql: &str, span: &SqlErrorSpan) -> Option<String> {
414+
fn build_sql_highlight(sql: &str, span: &SqlErrorSpan) -> Option<String> {
379415
if span.line == 0 || span.start == 0 {
380416
return None;
381417
}
382418

383-
let line = sql.lines().nth(span.line.saturating_sub(1))?;
384-
let line = line.trim_end_matches('\r');
419+
let lines = sql
420+
.lines()
421+
.map(|line| line.trim_end_matches('\r').to_string())
422+
.collect::<Vec<_>>();
423+
if lines.is_empty() || span.line > lines.len() {
424+
return None;
425+
}
385426

386-
let line_char_count = line.chars().count();
387-
let start_idx = span.start.saturating_sub(1).min(line_char_count);
388-
let end_idx = span.end.min(line_char_count).max(start_idx + 1);
427+
let width = lines.len().to_string().len();
428+
let mut out = String::new();
429+
out.push_str(&format!("--> line {}\n", span.line));
389430

390-
let start_byte = char_to_byte_offset(line, start_idx)?;
391-
let end_byte = char_to_byte_offset(line, end_idx)?;
392-
let near = line.get(start_byte..end_byte)?.trim();
431+
for (i, line) in lines.iter().enumerate() {
432+
let line_no = i + 1;
433+
out.push_str(&format!("{line_no:>width$} | {line}\n", width = width));
393434

394-
if near.is_empty() {
395-
None
396-
} else {
397-
Some(near.to_string())
435+
if line_no == span.line {
436+
let char_len = line.chars().count();
437+
let start = span.start.saturating_sub(1).min(char_len);
438+
let end = span.end.min(char_len).max(start + 1);
439+
let marker_len = end.saturating_sub(start).max(1);
440+
out.push_str(&format!(
441+
"{:>width$} | {}{}\n",
442+
"",
443+
" ".repeat(start),
444+
"^".repeat(marker_len),
445+
width = width
446+
));
447+
}
398448
}
399-
}
400449

401-
fn char_to_byte_offset(s: &str, char_index: usize) -> Option<usize> {
402-
if char_index == s.chars().count() {
403-
return Some(s.len());
404-
}
405-
s.char_indices().nth(char_index).map(|(idx, _)| idx)
450+
Some(out.trim_end().to_string())
406451
}

src/execution/dml/insert.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ impl<'a, T: Transaction + 'a> WriteExecutor<'a, T> for Insert {
9696
.map(|(_, col)| col.key(is_mapping_by_name))
9797
.collect_vec();
9898
if primary_keys.is_empty() {
99-
throw!(co, Err(DatabaseError::NotNull))
99+
throw!(co, Err(DatabaseError::not_null()))
100100
}
101101

102102
if let Some(table_catalog) = throw!(
@@ -152,7 +152,8 @@ impl<'a, T: Transaction + 'a> WriteExecutor<'a, T> for Insert {
152152
value.unwrap_or(DataValue::Null)
153153
};
154154
if value.is_null() && !col.nullable() {
155-
co.yield_(Err(DatabaseError::NotNull)).await;
155+
co.yield_(Err(DatabaseError::not_null_column(col.name().to_string())))
156+
.await;
156157
return;
157158
}
158159
values.push(value)

src/function/numbers.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,9 @@ impl TableFunctionImpl for Numbers {
7171
if value.logical_type() != LogicalType::Integer {
7272
value = value.cast(&LogicalType::Integer)?;
7373
}
74-
let num = value.i32().ok_or(DatabaseError::NotNull)?;
74+
let num = value
75+
.i32()
76+
.ok_or_else(|| DatabaseError::not_null_column("numbers() arg"))?;
7577

7678
Ok(
7779
Box::new((0..num).map(|i| Ok(Tuple::new(None, vec![DataValue::Int32(i)]))))

src/storage/table_codec.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ impl TableCodec {
7575
return Err(DatabaseError::PrimaryKeyTooManyLayers);
7676
}
7777
if value.is_null() {
78-
return Err(DatabaseError::NotNull);
78+
return Err(DatabaseError::not_null_column("primary key"));
7979
}
8080

8181
if let DataValue::Tuple(values, _) = &value {

0 commit comments

Comments
 (0)