Skip to content

Commit 9cba116

Browse files
KKouldKKould
authored
fix(parser): preserve detailed stage URI errors (#19746)
1 parent 791ba5b commit 9cba116

11 files changed

Lines changed: 110 additions & 60 deletions

File tree

‎src/query/ast/src/ast/statements/copy.rs‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,18 @@ pub struct UriLocation {
520520
}
521521

522522
impl UriLocation {
523+
fn invalid_uri_hint(uri: &str) -> Option<&'static str> {
524+
if uri.starts_with('/') {
525+
Some("local filesystem paths must use fs:///path/ instead of /path/")
526+
} else if uri.starts_with("file://") {
527+
Some("local filesystem paths must use fs:///path/ instead of file:///path/")
528+
} else if uri.starts_with("fs://") && !uri.starts_with("fs:///") {
529+
Some("fs locations must start with fs:///")
530+
} else {
531+
None
532+
}
533+
}
534+
523535
pub fn new(
524536
protocol: String,
525537
name: String,
@@ -535,14 +547,12 @@ impl UriLocation {
535547
}
536548

537549
pub fn from_uri(uri: String, conns: BTreeMap<String, String>) -> Result<Self> {
550+
if let Some(message) = Self::invalid_uri_hint(&uri) {
551+
return Err(ParseError(None, message.to_string()));
552+
}
553+
538554
// fs location is not a valid url, let's check it in advance.
539555
if let Some(path) = uri.strip_prefix("fs://") {
540-
if !path.starts_with('/') {
541-
return Err(ParseError(
542-
None,
543-
format!("Invalid uri: {}. fs location must start with 'fs:///'", uri),
544-
));
545-
}
546556
return Ok(UriLocation::new(
547557
"fs".to_string(),
548558
"".to_string(),

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

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ pub fn any_token(i: Input<'_>) -> IResult<'_, &Token<'_>> {
7878
Some(token) => Ok((i.slice(1..), token)),
7979
_ => Err(nom::Err::Error(Error::from_error_kind(
8080
i,
81-
ErrorKind::Other("expected any token but reached the end"),
81+
ErrorKind::other("expected any token but reached the end"),
8282
))),
8383
}
8484
}
@@ -160,7 +160,7 @@ fn quoted_identifier(i: Input) -> IResult<Identifier> {
160160
let QuotedIdent(ident, quote) = token.text().parse().map_err(|_| {
161161
nom::Err::Error(Error::from_error_kind(
162162
i,
163-
ErrorKind::Other("invalid identifier"),
163+
ErrorKind::other("invalid identifier"),
164164
))
165165
})?;
166166
Ok((i2, Identifier {
@@ -354,7 +354,7 @@ pub fn column_position(i: Input) -> IResult<ColumnID> {
354354
.parse::<usize>()
355355
.map_err(|e| nom::Err::Failure(e.into()))?;
356356
if pos == 0 {
357-
return Err(nom::Err::Failure(ErrorKind::Other(
357+
return Err(nom::Err::Failure(ErrorKind::other(
358358
"column position must be greater than 0",
359359
)));
360360
}
@@ -664,7 +664,7 @@ where
664664
move |input: Input| match match_error.parse(input) {
665665
Ok(_) => Err(nom::Err::Error(Error::from_error_kind(
666666
input,
667-
ErrorKind::Other(message),
667+
ErrorKind::other(message),
668668
))),
669669
Err(_) => Ok((input, ())),
670670
}
@@ -739,24 +739,24 @@ where
739739
input.backtrace.clear();
740740

741741
let err_kind = match err {
742-
PrattError::EmptyInput => ErrorKind::Other("expecting an operand"),
742+
PrattError::EmptyInput => ErrorKind::other("expecting an operand"),
743743
PrattError::UnexpectedNilfix(i) => {
744744
*span.borrow_mut() = Some(i.span);
745-
ErrorKind::Other("unable to parse the element")
745+
ErrorKind::other("unable to parse the element")
746746
}
747747
PrattError::UnexpectedPrefix(i) => {
748748
*span.borrow_mut() = Some(i.span);
749-
ErrorKind::Other("unable to parse the prefix operator")
749+
ErrorKind::other("unable to parse the prefix operator")
750750
}
751751
PrattError::UnexpectedInfix(i) => {
752752
*span.borrow_mut() = Some(i.span);
753-
ErrorKind::Other("missing lhs or rhs for the binary operator")
753+
ErrorKind::other("missing lhs or rhs for the binary operator")
754754
}
755755
PrattError::UnexpectedPostfix(i) => {
756756
*span.borrow_mut() = Some(i.span);
757-
ErrorKind::Other("unable to parse the postfix operator")
757+
ErrorKind::other("unable to parse the postfix operator")
758758
}
759-
PrattError::UserError(err) => ErrorKind::Other(err),
759+
PrattError::UserError(err) => ErrorKind::other(err),
760760
};
761761

762762
let span = span
@@ -785,7 +785,7 @@ where F: nom::Parser<Input<'a>, Output = O, Error = Error<'a>> {
785785
i.backtrace.clear();
786786
let error = Error::from_error_kind(
787787
input,
788-
ErrorKind::Other("variable is only available in SQL template"),
788+
ErrorKind::other("variable is only available in SQL template"),
789789
);
790790
Err(nom::Err::Failure(error))
791791
}
@@ -819,7 +819,7 @@ macro_rules! declare_experimental_feature {
819819
i.backtrace.clear();
820820
let error = Error::from_error_kind(
821821
input,
822-
ErrorKind::Other(
822+
ErrorKind::other(
823823
concat!(
824824
$feature_name,
825825
" only works in experimental dialect, try `set sql_dialect = 'experimental'`"

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ pub fn copy_into_table(i: Input) -> IResult<Statement> {
9595
for opt in opts {
9696
copy_stmt
9797
.apply_option(opt)
98-
.map_err(|e| nom::Err::Failure(ErrorKind::Other(e)))?;
98+
.map_err(|e| nom::Err::Failure(ErrorKind::other(e)))?;
9999
}
100100
Ok(Statement::CopyIntoTable(copy_stmt))
101101
},

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

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,20 @@ pub struct Error<'a> {
4545
}
4646

4747
/// ErrorKind is the error type returned from parser.
48-
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
48+
#[derive(Clone, Debug, PartialEq, Eq)]
4949
pub enum ErrorKind {
5050
/// Error generated by `match_token` function
5151
ExpectToken(TokenKind),
5252
/// Error generated by `match_text` function
5353
ExpectText(&'static str),
5454
/// Plain text description of an error
55-
Other(&'static str),
55+
Other(String),
56+
}
57+
58+
impl ErrorKind {
59+
pub fn other(message: impl Into<String>) -> Self {
60+
Self::Other(message.into())
61+
}
5662
}
5763

5864
/// Record the farthest position in the input before encountering an error.
@@ -137,20 +143,20 @@ impl<'a> Error<'a> {
137143
if let Some(ref mut inner) = *inner {
138144
match input.tokens[0].span.start.cmp(&inner.span.start) {
139145
Ordering::Equal => {
140-
inner.errors.push(kind);
146+
inner.errors.push(kind.clone());
141147
}
142148
Ordering::Less => (),
143149
Ordering::Greater => {
144150
*inner = BacktraceInner {
145151
span: transform_span(&input.tokens[..1]).unwrap(),
146-
errors: vec![kind],
152+
errors: vec![kind.clone()],
147153
};
148154
}
149155
}
150156
} else {
151157
*inner = Some(BacktraceInner {
152158
span: transform_span(&input.tokens[..1]).unwrap(),
153-
errors: vec![kind],
159+
errors: vec![kind.clone()],
154160
})
155161
}
156162

@@ -165,7 +171,7 @@ impl<'a> Error<'a> {
165171

166172
impl From<fast_float2::Error> for ErrorKind {
167173
fn from(_: fast_float2::Error) -> Self {
168-
ErrorKind::Other("unable to parse float number")
174+
ErrorKind::other("unable to parse float number")
169175
}
170176
}
171177

@@ -179,7 +185,7 @@ impl From<ParseIntError> for ErrorKind {
179185
IntErrorKind::NegOverflow => "unable to parse number because it negatively overflowed",
180186
_ => "unable to parse number",
181187
};
182-
ErrorKind::Other(msg)
188+
ErrorKind::other(msg)
183189
}
184190
}
185191

@@ -196,7 +202,7 @@ impl From<hex::FromHexError> for ErrorKind {
196202
"unable to parse hex literal because it has an invalid length"
197203
}
198204
};
199-
ErrorKind::Other(msg)
205+
ErrorKind::other(msg)
200206
}
201207
}
202208

@@ -229,7 +235,7 @@ pub fn display_parser_error(error: Error, source: &str) -> String {
229235
.chain(inner.errors.iter().map(|err| (inner.span, err)))
230236
{
231237
if let ErrorKind::Other(msg) = kind {
232-
labels = vec![(span, msg.to_string())];
238+
labels = vec![(span, msg.clone())];
233239
break;
234240
}
235241
}

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

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ pub fn subexpr(min_precedence: u32) -> impl FnMut(Input) -> IResult<Expr> {
6060
{
6161
Err(nom::Err::Error(Error::from_error_kind(
6262
i,
63-
ErrorKind::Other("expected more tokens for expression"),
63+
ErrorKind::other("expected more tokens for expression"),
6464
)))
6565
}
6666
_ => Ok((rest, elem)),
@@ -1822,7 +1822,7 @@ pub fn unary_op(i: Input) -> IResult<UnaryOperator> {
18221822
}
18231823
Err(nom::Err::Error(Error::from_error_kind(
18241824
i,
1825-
ErrorKind::Other("expecting `NOT`, '!', '|/', '~', '||/', '@', or more ..."),
1825+
ErrorKind::other("expecting `NOT`, '!', '|/', '~', '||/', '@', or more ..."),
18261826
)))
18271827
}
18281828

@@ -1889,7 +1889,7 @@ pub fn binary_op(i: Input) -> IResult<BinaryOperator> {
18891889
}
18901890
Err(nom::Err::Error(Error::from_error_kind(
18911891
i,
1892-
ErrorKind::Other(
1892+
ErrorKind::other(
18931893
"expecting `IS`, `IN`, `LIKE`, `EXISTS`, `BETWEEN`, `+`, `-`, `*`, `/`, `//`, `DIV`, `%`, `||`, `<=>`, `<+>`, `<->`, `>`, `<`, `>=`, `<=`, `=`, `<>`, `!=`, `^`, `AND`, `OR`, `XOR`, `NOT`, `REGEXP`, `RLIKE`, `SOUNDS`, or more ...",
18941894
),
18951895
)))
@@ -1915,7 +1915,7 @@ pub fn json_op(i: Input) -> IResult<JsonOperator> {
19151915
}
19161916
Err(nom::Err::Error(Error::from_error_kind(
19171917
i,
1918-
ErrorKind::Other(
1918+
ErrorKind::other(
19191919
"expecting `->`, '->>', '#>', '#>>', '?', '?|', '?&', '@>', '<@', '@?', '@@', '#-', or more ...",
19201920
),
19211921
)))
@@ -1961,7 +1961,7 @@ pub fn literal(i: Input) -> IResult<Literal> {
19611961

19621962
Err(nom::Err::Error(Error::from_error_kind(
19631963
i,
1964-
ErrorKind::Other(
1964+
ErrorKind::other(
19651965
"expecting `<LiteralString>`, '<LiteralCodeString>', '<LiteralInteger>', '<LiteralFloat>', 'TRUE', 'FALSE', or more ...",
19661966
),
19671967
)))
@@ -2029,7 +2029,7 @@ pub fn literal_string(i: Input) -> IResult<String> {
20292029
let quote::QuotedString(s, quote) = token
20302030
.text()
20312031
.parse()
2032-
.map_err(|_| nom::Err::Failure(ErrorKind::Other("invalid escape or unicode")))?;
2032+
.map_err(|_| nom::Err::Failure(ErrorKind::other("invalid escape or unicode")))?;
20332033

20342034
if !i.dialect.is_string_quote(quote) {
20352035
return Err(nom::Err::Error(ErrorKind::ExpectToken(LiteralString)));
@@ -2057,7 +2057,7 @@ pub fn at_string(i: Input) -> IResult<String> {
20572057
let AtString(s) = token
20582058
.text()
20592059
.parse()
2060-
.map_err(|_| nom::Err::Failure(ErrorKind::Other("invalid at string")))?;
2060+
.map_err(|_| nom::Err::Failure(ErrorKind::other("invalid at string")))?;
20612061
Ok(s)
20622062
})(i)
20632063
}
@@ -2134,10 +2134,10 @@ pub fn type_name(i: Input) -> IResult<TypeName> {
21342134
Ok(TypeName::Decimal {
21352135
precision: precision
21362136
.try_into()
2137-
.map_err(|_| nom::Err::Failure(ErrorKind::Other("precision is too large")))?,
2137+
.map_err(|_| nom::Err::Failure(ErrorKind::other("precision is too large")))?,
21382138
scale: scale
21392139
.try_into()
2140-
.map_err(|_| nom::Err::Failure(ErrorKind::Other("scale is too large")))?,
2140+
.map_err(|_| nom::Err::Failure(ErrorKind::other("scale is too large")))?,
21412141
})
21422142
},
21432143
);
@@ -2253,7 +2253,7 @@ pub fn type_name(i: Input) -> IResult<TypeName> {
22532253
Some(true) => Ok(ty.wrap_nullable()),
22542254
Some(false) => {
22552255
if matches!(ty, TypeName::Nullable(_)) {
2256-
Err(nom::Err::Failure(ErrorKind::Other(
2256+
Err(nom::Err::Failure(ErrorKind::other(
22572257
"ambiguous NOT NULL constraint",
22582258
)))
22592259
} else {
@@ -2684,7 +2684,7 @@ pub fn function_call(i: Input) -> IResult<ExprElement> {
26842684
args,
26852685
})
26862686
}
2687-
_ => Err(nom::Err::Error(ErrorKind::Other(
2687+
_ => Err(nom::Err::Error(ErrorKind::other(
26882688
"Unsupported function format",
26892689
))),
26902690
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ pub fn set_operation_element(i: Input) -> IResult<WithSpan<SetOperationElement>>
109109
},
110110
|(_from, from_block)| {
111111
if from_block.len() != 1 {
112-
return Err(nom::Err::Failure(ErrorKind::Other(
112+
return Err(nom::Err::Failure(ErrorKind::other(
113113
"FROM query only support query one table",
114114
)));
115115
}
@@ -157,7 +157,7 @@ pub fn set_operation_element(i: Input) -> IResult<WithSpan<SetOperationElement>>
157157
opt_qualify_block,
158158
)| {
159159
if opt_from_block_first.is_some() && opt_from_block_second.is_some() {
160-
return Err(nom::Err::Failure(ErrorKind::Other(
160+
return Err(nom::Err::Failure(ErrorKind::other(
161161
"duplicated FROM clause",
162162
)));
163163
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ pub fn script_stmt(i: Input) -> IResult<ScriptStatement> {
165165
if matches!(kind, END | ELSE | ELSEIF | WHEN | UNTIL) {
166166
return Err(nom::Err::Error(Error::from_error_kind(
167167
i,
168-
ErrorKind::Other("block terminator"),
168+
ErrorKind::other("block terminator"),
169169
)));
170170
}
171171
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -297,15 +297,15 @@ pub fn file_location(i: Input) -> IResult<FileLocation> {
297297
pub fn stage_location(i: Input) -> IResult<String> {
298298
map_res(file_location, |location| match location {
299299
FileLocation::Stage(s) => Ok(s),
300-
FileLocation::Uri(_) => Err(nom::Err::Failure(ErrorKind::Other(
300+
FileLocation::Uri(_) => Err(nom::Err::Failure(ErrorKind::other(
301301
"expect stage location, got uri location",
302302
))),
303303
})(i)
304304
}
305305

306306
pub fn uri_location(i: Input) -> IResult<UriLocation> {
307307
map_res(string_location, |location| match location {
308-
FileLocation::Stage(_) => Err(nom::Err::Failure(ErrorKind::Other(
308+
FileLocation::Stage(_) => Err(nom::Err::Failure(ErrorKind::other(
309309
"uri location should not start with '@'",
310310
))),
311311
FileLocation::Uri(u) => Ok(u),
@@ -323,7 +323,7 @@ pub fn string_location(i: Input) -> IResult<FileLocation> {
323323
if connection_opts.is_none() {
324324
Ok(FileLocation::Stage(stripped.to_string()))
325325
} else {
326-
Err(nom::Err::Failure(ErrorKind::Other(
326+
Err(nom::Err::Failure(ErrorKind::other(
327327
"uri location should not start with '@'",
328328
)))
329329
}
@@ -333,7 +333,7 @@ pub fn string_location(i: Input) -> IResult<FileLocation> {
333333
let conns = connection_opts.map(|v| v.2).unwrap_or_default();
334334

335335
let uri = UriLocation::from_uri(location, conns)
336-
.map_err(|_| nom::Err::Failure(ErrorKind::Other("invalid uri")))?;
336+
.map_err(|err| nom::Err::Failure(ErrorKind::other(err.1)))?;
337337
Ok(FileLocation::Uri(uri))
338338
}
339339
},

0 commit comments

Comments
 (0)